diff --git a/Lib/aifc.py b/Lib/aifc.py index 9e64de9c441..0b4f85a3050 100644 --- a/Lib/aifc.py +++ b/Lib/aifc.py @@ -356,7 +356,10 @@ class Aifc_read: self._soundpos = 0 def close(self): - self._file.close() + file = self._file + if file is not None: + self._file = None + file.close() def tell(self): return self._soundpos diff --git a/Lib/binhex.py b/Lib/binhex.py index 14badb71aa5..56b5f852c00 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 @@ -213,16 +214,21 @@ 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): """binhex(infilename, outfilename): create binhex-encoded copy of a file""" @@ -435,11 +441,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): """hexbin(infilename, outfilename) - Decode binhexed file""" diff --git a/Lib/chunk.py b/Lib/chunk.py index dc90a7522e3..98187b2c43a 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/dbm/dumb.py b/Lib/dbm/dumb.py index 34240968e72..7777a7cae67 100644 --- a/Lib/dbm/dumb.py +++ b/Lib/dbm/dumb.py @@ -258,8 +258,10 @@ class _Database(collections.MutableMapping): raise error('DBM object has already been closed') from None 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/distutils/text_file.py b/Lib/distutils/text_file.py index 40b8484a685..478336f0d2b 100644 --- a/Lib/distutils/text_file.py +++ b/Lib/distutils/text_file.py @@ -118,10 +118,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): outmsg = [] diff --git a/Lib/fileinput.py b/Lib/fileinput.py index 87758ad82b3..af810d1d732 100644 --- a/Lib/fileinput.py +++ b/Lib/fileinput.py @@ -238,8 +238,10 @@ class FileInput: self.close() def close(self): - self.nextfile() - self._files = () + try: + self.nextfile() + finally: + self._files = () def __enter__(self): return self @@ -275,29 +277,31 @@ class FileInput: def nextfile(self): savestdout = self._savestdout - self._savestdout = 0 + self._savestdout = None if savestdout: sys.stdout = savestdout output = self._output - self._output = 0 - if output: - output.close() + self._output = None + try: + if output: + output.close() + finally: + file = self._file + self._file = None + try: + if file and not self._isstdin: + file.close() + finally: + backupfilename = self._backupfilename + self._backupfilename = None + if backupfilename and not self._backup: + try: os.unlink(backupfilename) + except OSError: pass - 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 + self._isstdin = False + self._buffer = [] + self._bufindex = 0 def readline(self): try: diff --git a/Lib/ftplib.py b/Lib/ftplib.py index 135ec9c1dff..54b0e2cfcb3 100644 --- a/Lib/ftplib.py +++ b/Lib/ftplib.py @@ -667,11 +667,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 21d83e6414d..deaf15db4f9 100644 --- a/Lib/gzip.py +++ b/Lib/gzip.py @@ -503,19 +503,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 & 0xffffffff) - 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 & 0xffffffff) + 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/http/client.py b/Lib/http/client.py index ac120e796c4..80c80cf576e 100644 --- a/Lib/http/client.py +++ b/Lib/http/client.py @@ -388,9 +388,11 @@ class HTTPResponse(io.BufferedIOBase): fp.close() def close(self): - super().close() # set "closed" flag - if self.fp: - self._close_conn() + try: + super().close() # set "closed" flag + finally: + if self.fp: + self._close_conn() # These implementations are for the benefit of io.BufferedReader. @@ -829,13 +831,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 4942147055b..104b0be8d07 100644 --- a/Lib/logging/__init__.py +++ b/Lib/logging/__init__.py @@ -1013,14 +1013,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 d4f8aef6e70..02a5fc1283d 100644 --- a/Lib/logging/handlers.py +++ b/Lib/logging/handlers.py @@ -627,9 +627,10 @@ class SocketHandler(logging.Handler): """ self.acquire() try: - if self.sock: - self.sock.close() + sock = self.sock + if sock: self.sock = None + sock.close() logging.Handler.close(self) finally: self.release() @@ -1213,8 +1214,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): """ @@ -1268,13 +1271,15 @@ 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() class QueueHandler(logging.Handler): diff --git a/Lib/mailbox.py b/Lib/mailbox.py index e7f31df1e53..24d4aec8a48 100644 --- a/Lib/mailbox.py +++ b/Lib/mailbox.py @@ -722,10 +722,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.""" @@ -1966,9 +1970,11 @@ class _ProxyFile: def close(self): """Close the file.""" if hasattr(self, '_file'): - if hasattr(self._file, 'close'): - self._file.close() - del self._file + try: + if hasattr(self._file, 'close'): + self._file.close() + finally: + del self._file def _read(self, size, read_method): """Read size bytes using read_method.""" diff --git a/Lib/multiprocessing/connection.py b/Lib/multiprocessing/connection.py index 07d19de7e33..4c32237f14c 100644 --- a/Lib/multiprocessing/connection.py +++ b/Lib/multiprocessing/connection.py @@ -460,9 +460,10 @@ class Listener(object): ''' Close the bound socket or named pipe of `self`. ''' - if self._listener is not None: - self._listener.close() + listener = self._listener + if listener is not None: self._listener = None + listener.close() address = property(lambda self: self._listener._address) last_accepted = property(lambda self: self._listener._last_accepted) @@ -594,9 +595,13 @@ class SocketListener(object): return Connection(s.detach()) 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 b004a6acfae..786a303b337 100644 --- a/Lib/multiprocessing/queues.py +++ b/Lib/multiprocessing/queues.py @@ -130,9 +130,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/poplib.py b/Lib/poplib.py index 1224eacbefc..4915628b03f 100644 --- a/Lib/poplib.py +++ b/Lib/poplib.py @@ -276,18 +276,23 @@ class POP3: 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: - try: - self.sock.shutdown(socket.SHUT_RDWR) - except OSError as e: - # The server might already have closed the connection - if e.errno != errno.ENOTCONN: - raise - finally: - 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: + try: + sock.shutdown(socket.SHUT_RDWR) + except OSError as e: + # The server might already have closed the connection + if e.errno != errno.ENOTCONN: + raise + finally: + sock.close() #__del__ = quit diff --git a/Lib/selectors.py b/Lib/selectors.py index 44a61508feb..e17ea363c8b 100644 --- a/Lib/selectors.py +++ b/Lib/selectors.py @@ -439,8 +439,10 @@ if hasattr(select, 'epoll'): return ready def close(self): - self._epoll.close() - super().close() + try: + self._epoll.close() + finally: + super().close() if hasattr(select, 'devpoll'): @@ -496,8 +498,10 @@ if hasattr(select, 'devpoll'): return ready def close(self): - self._devpoll.close() - super().close() + try: + self._devpoll.close() + finally: + super().close() if hasattr(select, 'kqueue'): @@ -566,8 +570,10 @@ if hasattr(select, 'kqueue'): return ready def close(self): - self._kqueue.close() - super().close() + try: + self._kqueue.close() + finally: + super().close() # Choose the best implementation, roughly: diff --git a/Lib/shelve.py b/Lib/shelve.py index cef580e5cdc..581baf1e6f3 100644 --- a/Lib/shelve.py +++ b/Lib/shelve.py @@ -138,17 +138,21 @@ class Shelf(collections.MutableMapping): self.close() def close(self): - self.sync() + if self.dict is None: + return try: - self.dict.close() - except AttributeError: - pass - # Catch errors that may happen when close is called from __del__ - # because CPython is in interpreter shutdown. - 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 24237289d16..5c8dd722d69 100755 --- a/Lib/smtplib.py +++ b/Lib/smtplib.py @@ -880,12 +880,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): """Terminate the SMTP session.""" diff --git a/Lib/sunau.py b/Lib/sunau.py index 3da41b75e4e..e760093dd2a 100644 --- a/Lib/sunau.py +++ b/Lib/sunau.py @@ -295,9 +295,11 @@ class Au_read: self._soundpos = pos def close(self): - if self._opened and self._file: - self._file.close() - self._file = None + file = self._file + if file: + self._file = None + if self._opened: + file.close() class Au_write: @@ -438,9 +440,10 @@ class Au_write: self._patchheader() self._file.flush() finally: - if self._opened and self._file: - self._file.close() + file = self._file self._file = None + if self._opened: + file.close() # # private methods diff --git a/Lib/tarfile.py b/Lib/tarfile.py index 720bbf7d134..43909f0df16 100755 --- a/Lib/tarfile.py +++ b/Lib/tarfile.py @@ -449,26 +449,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 = b"" - 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 eebb9529027..72dabc76e0c 100644 --- a/Lib/telnetlib.py +++ b/Lib/telnetlib.py @@ -261,12 +261,13 @@ class Telnet: def close(self): """Close the connection.""" - if self.sock: - self.sock.close() - self.sock = 0 - self.eof = 1 + sock = self.sock + self.sock = None + self.eof = True self.iacseq = b'' 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 4543d3f6911..81c289ad6fc 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -357,9 +357,11 @@ class _TemporaryFileCloser: def close(self, unlink=_os.unlink): if not self.close_called and self.file is not None: self.close_called = True - self.file.close() - if self.delete: - unlink(self.name) + try: + self.file.close() + finally: + if self.delete: + unlink(self.name) # Need to ensure the file is deleted on __del__ def __del__(self): diff --git a/Lib/urllib/response.py b/Lib/urllib/response.py index 4a143b03e67..4778118dbb1 100644 --- a/Lib/urllib/response.py +++ b/Lib/urllib/response.py @@ -43,11 +43,15 @@ class addclosehook(addbase): self.hookargs = hookargs def close(self): - if self.closehook: - self.closehook(*self.hookargs) - self.closehook = None - self.hookargs = None - super(addclosehook, self).close() + try: + closehook = self.closehook + hookargs = self.hookargs + if closehook: + self.closehook = None + self.hookargs = None + closehook(*hookargs) + finally: + super(addclosehook, self).close() class addinfo(addbase): diff --git a/Lib/wave.py b/Lib/wave.py index b56395ead70..8fba70f02e3 100644 --- a/Lib/wave.py +++ b/Lib/wave.py @@ -186,10 +186,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 @@ -428,17 +429,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 65ac7e30434..1795b23af60 100644 --- a/Lib/xml/sax/expatreader.py +++ b/Lib/xml/sax/expatreader.py @@ -211,22 +211,24 @@ 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: - file = self._source.getCharacterStream() - if file is not None: - file.close() + self.feed("", isFinal = 1) + self._cont_handler.endDocument() finally: - file = self._source.getByteStream() - if file is not None: - file.close() + self._parsing = 0 + # break cycle created by expat handlers pointing to our methods + self._parser = None + try: + file = self._source.getCharacterStream() + if file is not None: + file.close() + finally: + file = self._source.getByteStream() + if file is not None: + file.close() def _reset_cont_handler(self): self._parser.ProcessingInstructionHandler = \ diff --git a/Lib/xmlrpc/client.py b/Lib/xmlrpc/client.py index da089a2f039..acb81421500 100644 --- a/Lib/xmlrpc/client.py +++ b/Lib/xmlrpc/client.py @@ -438,8 +438,13 @@ class ExpatParser: 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(b"", True) # end of data # -------------------------------------------------------------------- # XML-RPC marshalling and unmarshalling code @@ -1065,8 +1070,10 @@ class GzipDecodedResponse(gzip.GzipFile if gzip else object): gzip.GzipFile.__init__(self, mode="rb", fileobj=self.io) def close(self): - gzip.GzipFile.close(self) - self.io.close() + try: + gzip.GzipFile.close(self) + finally: + self.io.close() # -------------------------------------------------------------------- @@ -1221,9 +1228,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 HTTP request. diff --git a/Misc/NEWS b/Misc/NEWS index b62e72aeea5..7f913494cfc 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -19,6 +19,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 #23400: Raise same exception on both Python 2 and 3 if sem_open is not available. Patch by Davin Potts.