From 1aa2c0f073bdbed4fa824591d53e20bbf3d01add Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 10 Apr 2015 13:24:10 +0300 Subject: Issue #23865: close() methods in multiple modules now are idempotent and more robust at shutdown. If needs to release multiple resources, they are released even if errors are occured. --- Lib/aifc.py | 11 ++++--- Lib/binhex.py | 44 +++++++++++++++++----------- Lib/chunk.py | 6 ++-- Lib/distutils/text_file.py | 4 +-- Lib/dumbdbm.py | 6 ++-- Lib/fileinput.py | 42 +++++++++++++++------------ Lib/ftplib.py | 15 ++++++---- Lib/gzip.py | 26 +++++++++-------- Lib/httplib.py | 21 +++++++++----- Lib/logging/__init__.py | 21 +++++++++----- Lib/logging/handlers.py | 23 +++++++++------ Lib/mailbox.py | 12 +++++--- Lib/multiprocessing/connection.py | 10 +++++-- Lib/multiprocessing/queues.py | 10 +++++-- Lib/shelve.py | 24 ++++++++------- Lib/smtplib.py | 16 ++++++---- Lib/tarfile.py | 61 ++++++++++++++++++++------------------- Lib/telnetlib.py | 5 ++-- Lib/tempfile.py | 8 +++-- Lib/urllib.py | 15 ++++++---- Lib/wave.py | 22 +++++++------- Lib/xml/sax/expatreader.py | 14 +++++---- Lib/xmlrpclib.py | 20 +++++++++---- Misc/NEWS | 4 +++ 24 files changed, 264 insertions(+), 176 deletions(-) diff --git a/Lib/aifc.py b/Lib/aifc.py index 9ac710f..c9a021e 100644 --- a/Lib/aifc.py +++ b/Lib/aifc.py @@ -357,10 +357,13 @@ class Aifc_read: self._soundpos = 0 def close(self): - if self._decomp: - self._decomp.CloseDecompressor() - self._decomp = None - self._file.close() + decomp = self._decomp + try: + if decomp: + self._decomp = None + decomp.CloseDecompressor() + finally: + self._file.close() def tell(self): return self._soundpos diff --git a/Lib/binhex.py b/Lib/binhex.py index 8abc9f3..14ec233 100644 --- a/Lib/binhex.py +++ b/Lib/binhex.py @@ -32,7 +32,8 @@ class Error(Exception): pass # States (what have we written) -[_DID_HEADER, _DID_DATA, _DID_RSRC] = range(3) +_DID_HEADER = 0 +_DID_DATA = 1 # Various constants REASONABLY_LARGE=32768 # Minimal amount we pass the rle-coder @@ -235,17 +236,22 @@ class BinHex: self._write(data) def close(self): - if self.state < _DID_DATA: - self.close_data() - if self.state != _DID_DATA: - raise Error, 'Close at the wrong time' - if self.rlen != 0: - raise Error, \ - "Incorrect resource-datasize, diff=%r" % (self.rlen,) - self._writecrc() - self.ofp.close() - self.state = None - del self.ofp + if self.state is None: + return + try: + if self.state < _DID_DATA: + self.close_data() + if self.state != _DID_DATA: + raise Error, 'Close at the wrong time' + if self.rlen != 0: + raise Error, \ + "Incorrect resource-datasize, diff=%r" % (self.rlen,) + self._writecrc() + finally: + self.state = None + ofp = self.ofp + del self.ofp + ofp.close() def binhex(inp, out): """(infilename, outfilename) - Create binhex-encoded copy of a file""" @@ -463,11 +469,15 @@ class HexBin: return self._read(n) def close(self): - if self.rlen: - dummy = self.read_rsrc(self.rlen) - self._checkcrc() - self.state = _DID_RSRC - self.ifp.close() + if self.state is None: + return + try: + if self.rlen: + dummy = self.read_rsrc(self.rlen) + self._checkcrc() + finally: + self.state = None + self.ifp.close() def hexbin(inp, out): """(infilename, outfilename) - Decode binhexed file""" diff --git a/Lib/chunk.py b/Lib/chunk.py index a8fbc10..3e3b5a4 100644 --- a/Lib/chunk.py +++ b/Lib/chunk.py @@ -85,8 +85,10 @@ class Chunk: def close(self): if not self.closed: - self.skip() - self.closed = True + try: + self.skip() + finally: + self.closed = True def isatty(self): if self.closed: diff --git a/Lib/distutils/text_file.py b/Lib/distutils/text_file.py index 09a798b..690cb80 100644 --- a/Lib/distutils/text_file.py +++ b/Lib/distutils/text_file.py @@ -124,11 +124,11 @@ class TextFile: def close (self): """Close the current file and forget everything we know about it (filename, current line number).""" - - self.file.close () + file = self.file self.file = None self.filename = None self.current_line = None + file.close() def gen_error (self, msg, line=None): diff --git a/Lib/dumbdbm.py b/Lib/dumbdbm.py index 46d543d..8ef7e79 100644 --- a/Lib/dumbdbm.py +++ b/Lib/dumbdbm.py @@ -209,8 +209,10 @@ class _Database(UserDict.DictMixin): return len(self._index) def close(self): - self._commit() - self._index = self._datfile = self._dirfile = self._bakfile = None + try: + self._commit() + finally: + self._index = self._datfile = self._dirfile = self._bakfile = None __del__ = close diff --git a/Lib/fileinput.py b/Lib/fileinput.py index 21c2d1f..02dc4c6 100644 --- a/Lib/fileinput.py +++ b/Lib/fileinput.py @@ -233,8 +233,10 @@ class FileInput: self.close() def close(self): - self.nextfile() - self._files = () + try: + self.nextfile() + finally: + self._files = () def __iter__(self): return self @@ -270,23 +272,25 @@ class FileInput: output = self._output self._output = 0 - if output: - output.close() - - file = self._file - self._file = 0 - if file and not self._isstdin: - file.close() - - backupfilename = self._backupfilename - self._backupfilename = 0 - if backupfilename and not self._backup: - try: os.unlink(backupfilename) - except OSError: pass - - self._isstdin = False - self._buffer = [] - self._bufindex = 0 + try: + if output: + output.close() + finally: + file = self._file + self._file = 0 + try: + if file and not self._isstdin: + file.close() + finally: + backupfilename = self._backupfilename + self._backupfilename = 0 + if backupfilename and not self._backup: + try: os.unlink(backupfilename) + except OSError: pass + + self._isstdin = False + self._buffer = [] + self._bufindex = 0 def readline(self): try: diff --git a/Lib/ftplib.py b/Lib/ftplib.py index e1c3d99..449ce71 100644 --- a/Lib/ftplib.py +++ b/Lib/ftplib.py @@ -594,11 +594,16 @@ class FTP: def close(self): '''Close the connection without assuming anything about it.''' - if self.file is not None: - self.file.close() - if self.sock is not None: - self.sock.close() - self.file = self.sock = None + try: + file = self.file + self.file = None + if file is not None: + file.close() + finally: + sock = self.sock + self.sock = None + if sock is not None: + sock.close() try: import ssl diff --git a/Lib/gzip.py b/Lib/gzip.py index 83cc077..de0dab1 100644 --- a/Lib/gzip.py +++ b/Lib/gzip.py @@ -369,19 +369,21 @@ class GzipFile(io.BufferedIOBase): return self.fileobj is None def close(self): - if self.fileobj is None: + fileobj = self.fileobj + if fileobj is None: return - if self.mode == WRITE: - self.fileobj.write(self.compress.flush()) - write32u(self.fileobj, self.crc) - # self.size may exceed 2GB, or even 4GB - write32u(self.fileobj, self.size & 0xffffffffL) - self.fileobj = None - elif self.mode == READ: - self.fileobj = None - if self.myfileobj: - self.myfileobj.close() - self.myfileobj = None + self.fileobj = None + try: + if self.mode == WRITE: + fileobj.write(self.compress.flush()) + write32u(fileobj, self.crc) + # self.size may exceed 2GB, or even 4GB + write32u(fileobj, self.size & 0xffffffffL) + finally: + myfileobj = self.myfileobj + if myfileobj: + self.myfileobj = None + myfileobj.close() def flush(self,zlib_mode=zlib.Z_SYNC_FLUSH): self._check_closed() diff --git a/Lib/httplib.py b/Lib/httplib.py index 5406e77..9f1e088 100644 --- a/Lib/httplib.py +++ b/Lib/httplib.py @@ -560,9 +560,10 @@ class HTTPResponse: return True def close(self): - if self.fp: - self.fp.close() + fp = self.fp + if fp: self.fp = None + fp.close() def isclosed(self): # NOTE: it is possible that we will not ever call self.close(). This @@ -835,13 +836,17 @@ class HTTPConnection: def close(self): """Close the connection to the HTTP server.""" - if self.sock: - self.sock.close() # close it manually... there may be other refs - self.sock = None - if self.__response: - self.__response.close() - self.__response = None self.__state = _CS_IDLE + try: + sock = self.sock + if sock: + self.sock = None + sock.close() # close it manually... there may be other refs + finally: + response = self.__response + if response: + self.__response = None + response.close() def send(self, data): """Send `data' to the server.""" diff --git a/Lib/logging/__init__.py b/Lib/logging/__init__.py index b2a7711..bd4afeb 100644 --- a/Lib/logging/__init__.py +++ b/Lib/logging/__init__.py @@ -916,14 +916,19 @@ class FileHandler(StreamHandler): """ self.acquire() try: - if self.stream: - self.flush() - if hasattr(self.stream, "close"): - self.stream.close() - self.stream = None - # Issue #19523: call unconditionally to - # prevent a handler leak when delay is set - StreamHandler.close(self) + try: + if self.stream: + try: + self.flush() + finally: + stream = self.stream + self.stream = None + if hasattr(stream, "close"): + stream.close() + finally: + # Issue #19523: call unconditionally to + # prevent a handler leak when delay is set + StreamHandler.close(self) finally: self.release() diff --git a/Lib/logging/handlers.py b/Lib/logging/handlers.py index a458529..e430ab7 100644 --- a/Lib/logging/handlers.py +++ b/Lib/logging/handlers.py @@ -588,9 +588,10 @@ class SocketHandler(logging.Handler): """ self.acquire() try: - if self.sock: - self.sock.close() + sock = self.sock + if sock: self.sock = None + sock.close() finally: self.release() logging.Handler.close(self) @@ -1160,8 +1161,10 @@ class BufferingHandler(logging.Handler): This version just flushes and chains to the parent class' close(). """ - self.flush() - logging.Handler.close(self) + try: + self.flush() + finally: + logging.Handler.close(self) class MemoryHandler(BufferingHandler): """ @@ -1213,10 +1216,12 @@ class MemoryHandler(BufferingHandler): """ Flush, set the target to None and lose the buffer. """ - self.flush() - self.acquire() try: - self.target = None - BufferingHandler.close(self) + self.flush() finally: - self.release() + self.acquire() + try: + self.target = None + BufferingHandler.close(self) + finally: + self.release() diff --git a/Lib/mailbox.py b/Lib/mailbox.py index ba49753..3866953 100644 --- a/Lib/mailbox.py +++ b/Lib/mailbox.py @@ -719,10 +719,14 @@ class _singlefileMailbox(Mailbox): def close(self): """Flush and close the mailbox.""" - self.flush() - if self._locked: - self.unlock() - self._file.close() # Sync has been done by self.flush() above. + try: + self.flush() + finally: + try: + if self._locked: + self.unlock() + finally: + self._file.close() # Sync has been done by self.flush() above. def _lookup(self, key=None): """Return (start, stop) or raise KeyError.""" diff --git a/Lib/multiprocessing/connection.py b/Lib/multiprocessing/connection.py index 463d472..645a26f 100644 --- a/Lib/multiprocessing/connection.py +++ b/Lib/multiprocessing/connection.py @@ -285,9 +285,13 @@ class SocketListener(object): return conn def close(self): - self._socket.close() - if self._unlink is not None: - self._unlink() + try: + self._socket.close() + finally: + unlink = self._unlink + if unlink is not None: + self._unlink = None + unlink() def SocketClient(address): diff --git a/Lib/multiprocessing/queues.py b/Lib/multiprocessing/queues.py index 487e0af..a88e298 100644 --- a/Lib/multiprocessing/queues.py +++ b/Lib/multiprocessing/queues.py @@ -156,9 +156,13 @@ class Queue(object): def close(self): self._closed = True - self._reader.close() - if self._close: - self._close() + try: + self._reader.close() + finally: + close = self._close + if close: + self._close = None + close() def join_thread(self): debug('Queue.join_thread()') diff --git a/Lib/shelve.py b/Lib/shelve.py index c8cba85..4f1e49d 100644 --- a/Lib/shelve.py +++ b/Lib/shelve.py @@ -140,17 +140,21 @@ class Shelf(UserDict.DictMixin): pass def close(self): - self.sync() - try: - self.dict.close() - except AttributeError: - pass - # Catch errors that may happen when close is called from __del__ - # because CPython is in interpreter shutdown. + if self.dict is None: + return try: - self.dict = _ClosedDict() - except (NameError, TypeError): - self.dict = None + self.sync() + try: + self.dict.close() + except AttributeError: + pass + finally: + # Catch errors that may happen when close is called from __del__ + # because CPython is in interpreter shutdown. + try: + self.dict = _ClosedDict() + except: + self.dict = None def __del__(self): if not hasattr(self, 'writeback'): diff --git a/Lib/smtplib.py b/Lib/smtplib.py index d1c2806..8388b98 100755 --- a/Lib/smtplib.py +++ b/Lib/smtplib.py @@ -750,12 +750,16 @@ class SMTP: def close(self): """Close the connection to the SMTP server.""" - if self.file: - self.file.close() - self.file = None - if self.sock: - self.sock.close() - self.sock = None + try: + file = self.file + self.file = None + if file: + file.close() + finally: + sock = self.sock + self.sock = None + if sock: + sock.close() def quit(self): diff --git a/Lib/tarfile.py b/Lib/tarfile.py index bc0b09f..082f361 100644 --- a/Lib/tarfile.py +++ b/Lib/tarfile.py @@ -492,26 +492,26 @@ class _Stream: if self.closed: return - if self.mode == "w" and self.comptype != "tar": - self.buf += self.cmp.flush() - - if self.mode == "w" and self.buf: - self.fileobj.write(self.buf) - self.buf = "" - if self.comptype == "gz": - # The native zlib crc is an unsigned 32-bit integer, but - # the Python wrapper implicitly casts that to a signed C - # long. So, on a 32-bit box self.crc may "look negative", - # while the same crc on a 64-bit box may "look positive". - # To avoid irksome warnings from the `struct` module, force - # it to look positive on all boxes. - self.fileobj.write(struct.pack(" 0: - self.fileobj.write(NUL * (RECORDSIZE - remainder)) - - if not self._extfileobj: - self.fileobj.close() self.closed = True + try: + if self.mode in "aw": + self.fileobj.write(NUL * (BLOCKSIZE * 2)) + self.offset += (BLOCKSIZE * 2) + # fill up the end with zero-blocks + # (like option -b20 for tar does) + blocks, remainder = divmod(self.offset, RECORDSIZE) + if remainder > 0: + self.fileobj.write(NUL * (RECORDSIZE - remainder)) + finally: + if not self._extfileobj: + self.fileobj.close() def getmember(self, name): """Return a TarInfo object for member `name'. If `name' can not be diff --git a/Lib/telnetlib.py b/Lib/telnetlib.py index 88aa482..2eaa8e3 100644 --- a/Lib/telnetlib.py +++ b/Lib/telnetlib.py @@ -254,12 +254,13 @@ class Telnet: def close(self): """Close the connection.""" - if self.sock: - self.sock.close() + sock = self.sock self.sock = 0 self.eof = 1 self.iacseq = '' self.sb = 0 + if sock: + sock.close() def get_socket(self): """Return the socket object used internally.""" diff --git a/Lib/tempfile.py b/Lib/tempfile.py index a4083f7..fbda8eb 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -413,9 +413,11 @@ class _TemporaryFileWrapper: def close(self): if not self.close_called: self.close_called = True - self.file.close() - if self.delete: - self.unlink(self.name) + try: + self.file.close() + finally: + if self.delete: + self.unlink(self.name) def __del__(self): self.close() diff --git a/Lib/urllib.py b/Lib/urllib.py index 9f972e3..ccb0574 100644 --- a/Lib/urllib.py +++ b/Lib/urllib.py @@ -994,11 +994,16 @@ class addclosehook(addbase): self.hookargs = hookargs def close(self): - if self.closehook: - self.closehook(*self.hookargs) - self.closehook = None - self.hookargs = None - addbase.close(self) + try: + closehook = self.closehook + hookargs = self.hookargs + if closehook: + self.closehook = None + self.hookargs = None + closehook(*hookargs) + finally: + addbase.close(self) + class addinfo(addbase): """class to add an info() method to an open file.""" diff --git a/Lib/wave.py b/Lib/wave.py index 8ff93c3..97f2146 100644 --- a/Lib/wave.py +++ b/Lib/wave.py @@ -180,10 +180,11 @@ class Wave_read: self._soundpos = 0 def close(self): - if self._i_opened_the_file: - self._i_opened_the_file.close() - self._i_opened_the_file = None self._file = None + file = self._i_opened_the_file + if file: + self._i_opened_the_file = None + file.close() def tell(self): return self._soundpos @@ -444,17 +445,18 @@ class Wave_write: self._patchheader() def close(self): - if self._file: - try: + try: + if self._file: self._ensure_header_written(0) if self._datalength != self._datawritten: self._patchheader() self._file.flush() - finally: - self._file = None - if self._i_opened_the_file: - self._i_opened_the_file.close() - self._i_opened_the_file = None + finally: + self._file = None + file = self._i_opened_the_file + if file: + self._i_opened_the_file = None + file.close() # # Internal methods. diff --git a/Lib/xml/sax/expatreader.py b/Lib/xml/sax/expatreader.py index 9de3e72..cd243e6 100644 --- a/Lib/xml/sax/expatreader.py +++ b/Lib/xml/sax/expatreader.py @@ -214,14 +214,16 @@ class ExpatParser(xmlreader.IncrementalParser, xmlreader.Locator): self._err_handler.fatalError(exc) def close(self): - if self._entity_stack: + if self._entity_stack or self._parser is None: # If we are completing an external entity, do nothing here return - self.feed("", isFinal = 1) - self._cont_handler.endDocument() - self._parsing = 0 - # break cycle created by expat handlers pointing to our methods - self._parser = None + try: + self.feed("", isFinal = 1) + self._cont_handler.endDocument() + finally: + self._parsing = 0 + # break cycle created by expat handlers pointing to our methods + self._parser = None def _reset_cont_handler(self): self._parser.ProcessingInstructionHandler = \ diff --git a/Lib/xmlrpclib.py b/Lib/xmlrpclib.py index 340dc51..db185a6 100644 --- a/Lib/xmlrpclib.py +++ b/Lib/xmlrpclib.py @@ -558,8 +558,13 @@ else: self._parser.Parse(data, 0) def close(self): - self._parser.Parse("", 1) # end of data - del self._target, self._parser # get rid of circular references + try: + parser = self._parser + except AttributeError: + pass + else: + del self._target, self._parser # get rid of circular references + parser.Parse("", 1) # end of data class SlowParser: """Default XML parser (based on xmllib.XMLParser).""" @@ -1214,8 +1219,10 @@ class GzipDecodedResponse(gzip.GzipFile if gzip else object): gzip.GzipFile.__init__(self, mode="rb", fileobj=self.stringio) def close(self): - gzip.GzipFile.close(self) - self.stringio.close() + try: + gzip.GzipFile.close(self) + finally: + self.stringio.close() # -------------------------------------------------------------------- @@ -1384,9 +1391,10 @@ class Transport: # Used in the event of socket errors. # def close(self): - if self._connection[1]: - self._connection[1].close() + host, connection = self._connection + if connection: self._connection = (None, None) + connection.close() ## # Send request header. diff --git a/Misc/NEWS b/Misc/NEWS index 5aca0a8..90a1439 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -21,6 +21,10 @@ Core and Builtins Library ------- +- Issue #23865: close() methods in multiple modules now are idempotent and more + robust at shutdown. If needs to release multiple resources, they are released + even if errors are occured. + - Issue #23881: urllib.ftpwrapper constructor now closes the socket if the FTP connection failed. -- cgit v0.12