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

@@ -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

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,6 +214,9 @@ class BinHex:
         self._write(data)
 
     def close(self):
+        if self.state is None:
+            return
+        try:
             if self.state < _DID_DATA:
                 self.close_data()
             if self.state != _DID_DATA:
@@ -220,9 +224,11 @@ class BinHex:
             if self.rlen != 0:
                 raise Error("Incorrect resource-datasize, diff=%r" % (self.rlen,))
             self._writecrc()
-        self.ofp.close()
+        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,10 +441,14 @@ class HexBin:
         return self._read(n)
 
     def close(self):
+        if self.state is None:
+            return
+        try:
             if self.rlen:
                 dummy = self.read_rsrc(self.rlen)
             self._checkcrc()
-        self.state = _DID_RSRC
+        finally:
+            self.state = None
             self.ifp.close()
 
 def hexbin(inp, out):

Lib/chunk.py

@@ -85,7 +85,9 @@ class Chunk:
 
     def close(self):
         if not self.closed:
+            try:
                 self.skip()
+            finally:
                 self.closed = True
 
     def isatty(self):

Lib/dbm/dumb.py

@@ -258,7 +258,9 @@ class _Database(collections.MutableMapping):
             raise error('DBM object has already been closed') from None
 
     def close(self):
+        try:
             self._commit()
+        finally:
             self._index = self._datfile = self._dirfile = self._bakfile = None
 
     __del__ = close

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 = []

Lib/fileinput.py

@@ -238,7 +238,9 @@ class FileInput:
         self.close()
 
     def close(self):
+        try:
             self.nextfile()
+        finally:
             self._files = ()
 
     def __enter__(self):
@@ -275,22 +277,24 @@ 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
+        self._output = None
+        try:
             if output:
                 output.close()
-
+        finally:
             file = self._file
-        self._file = 0
+            self._file = None
+            try:
                 if file and not self._isstdin:
                     file.close()
-
+            finally:
                 backupfilename = self._backupfilename
-        self._backupfilename = 0
+                self._backupfilename = None
                 if backupfilename and not self._backup:
                     try: os.unlink(backupfilename)
                     except OSError: pass

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

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
+        self.fileobj = None
+        try:
             if self.mode == WRITE:
-            self.fileobj.write(self.compress.flush())
-            write32u(self.fileobj, self.crc)
+                fileobj.write(self.compress.flush())
+                write32u(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()
+                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()

Lib/http/client.py

@@ -388,7 +388,9 @@ class HTTPResponse(io.BufferedIOBase):
         fp.close()
 
     def close(self):
+        try:
             super().close() # set "closed" flag
+        finally:
             if self.fp:
                 self._close_conn()
 
@@ -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.

Lib/logging/__init__.py

@@ -1012,12 +1012,17 @@ class FileHandler(StreamHandler):
         Closes the stream.
         """
         self.acquire()
+        try:
             try:
                 if self.stream:
+                    try:
                         self.flush()
-                if hasattr(self.stream, "close"):
-                    self.stream.close()
+                    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)

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,7 +1214,9 @@ class BufferingHandler(logging.Handler):
 
         This version just flushes and chains to the parent class' close().
         """
+        try:
             self.flush()
+        finally:
             logging.Handler.close(self)
 
 class MemoryHandler(BufferingHandler):
@@ -1268,7 +1271,9 @@ class MemoryHandler(BufferingHandler):
         """
         Flush, set the target to None and lose the buffer.
         """
+        try:
             self.flush()
+        finally:
             self.acquire()
             try:
                 self.target = None

Lib/mailbox.py

@@ -722,9 +722,13 @@ class _singlefileMailbox(Mailbox):
 
     def close(self):
         """Flush and close the mailbox."""
+        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):
@@ -1966,8 +1970,10 @@ class _ProxyFile:
     def close(self):
         """Close the file."""
         if hasattr(self, '_file'):
+            try:
                 if hasattr(self._file, 'close'):
                     self._file.close()
+            finally:
                 del self._file
 
     def _read(self, size, read_method):

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):
+        try:
             self._socket.close()
-        if self._unlink is not None:
-            self._unlink()
+        finally:
+            unlink = self._unlink
+            if unlink is not None:
+                self._unlink = None
+                unlink()
 
 
 def SocketClient(address):

Lib/multiprocessing/queues.py

@@ -130,9 +130,13 @@ class Queue(object):
 
     def close(self):
         self._closed = True
+        try:
             self._reader.close()
-        if self._close:
-            self._close()
+        finally:
+            close = self._close
+            if close:
+                self._close = None
+                close()
 
     def join_thread(self):
         debug('Queue.join_thread()')

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)
+            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:
-                self.sock.close()
-        self.file = self.sock = None
+                    sock.close()
 
     #__del__ = quit
 

Lib/selectors.py

@@ -439,7 +439,9 @@ if hasattr(select, 'epoll'):
             return ready
 
         def close(self):
+            try:
                 self._epoll.close()
+            finally:
                 super().close()
 
 
@@ -496,7 +498,9 @@ if hasattr(select, 'devpoll'):
             return ready
 
         def close(self):
+            try:
                 self._devpoll.close()
+            finally:
                 super().close()
 
 
@@ -566,7 +570,9 @@ if hasattr(select, 'kqueue'):
             return ready
 
         def close(self):
+            try:
                 self._kqueue.close()
+            finally:
                 super().close()
 
 

Lib/shelve.py

@@ -138,16 +138,20 @@ class Shelf(collections.MutableMapping):
         self.close()
 
     def close(self):
+        if self.dict is None:
+            return
+        try:
             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 (NameError, TypeError):
+            except:
                 self.dict = None
 
     def __del__(self):

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()
+        try:
+            file = self.file
             self.file = None
-        if self.sock:
-            self.sock.close()
+            if file:
+                file.close()
+        finally:
+            sock = self.sock
             self.sock = None
+            if sock:
+                sock.close()
 
     def quit(self):
         """Terminate the SMTP session."""

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()
+        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

Lib/tarfile.py

@@ -449,6 +449,8 @@ class _Stream:
         if self.closed:
             return
 
+        self.closed = True
+        try:
             if self.mode == "w" and self.comptype != "tar":
                 self.buf += self.cmp.flush()
 
@@ -464,12 +466,10 @@ class _Stream:
                     # it to look positive on all boxes.
                     self.fileobj.write(struct.pack("<L", self.crc & 0xffffffff))
                     self.fileobj.write(struct.pack("<L", self.pos & 0xffffFFFF))
-
+        finally:
             if not self._extfileobj:
                 self.fileobj.close()
 
-        self.closed = True
-
     def _init_read_gz(self):
         """Initialize for reading a gzip compressed fileobj.
         """
@@ -1714,6 +1714,8 @@ class TarFile(object):
         if self.closed:
             return
 
+        self.closed = True
+        try:
             if self.mode in "aw":
                 self.fileobj.write(NUL * (BLOCKSIZE * 2))
                 self.offset += (BLOCKSIZE * 2)
@@ -1722,10 +1724,9 @@ class TarFile(object):
                 blocks, remainder = divmod(self.offset, RECORDSIZE)
                 if remainder > 0:
                     self.fileobj.write(NUL * (RECORDSIZE - remainder))
-
+        finally:
             if not self._extfileobj:
                 self.fileobj.close()
-        self.closed = True
 
     def getmember(self, name):
         """Return a TarInfo object for member `name'. If `name' can not be

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."""

Lib/tempfile.py

@@ -357,7 +357,9 @@ class _TemporaryFileCloser:
         def close(self, unlink=_os.unlink):
             if not self.close_called and self.file is not None:
                 self.close_called = True
+                try:
                     self.file.close()
+                finally:
                     if self.delete:
                         unlink(self.name)
 

Lib/urllib/response.py

@@ -43,10 +43,14 @@ class addclosehook(addbase):
         self.hookargs = hookargs
 
     def close(self):
-        if self.closehook:
-            self.closehook(*self.hookargs)
+        try:
+            closehook = self.closehook
+            hookargs = self.hookargs
+            if closehook:
                 self.closehook = None
                 self.hookargs = None
+                closehook(*hookargs)
+        finally:
             super(addclosehook, self).close()
 
 

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:
+            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()
+            file = self._i_opened_the_file
+            if file:
                 self._i_opened_the_file = None
+                file.close()
 
     #
     # Internal methods.

Lib/xml/sax/expatreader.py

@@ -211,11 +211,13 @@ 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
+        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

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
+        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,7 +1070,9 @@ class GzipDecodedResponse(gzip.GzipFile if gzip else object):
         gzip.GzipFile.__init__(self, mode="rb", fileobj=self.io)
 
     def close(self):
+        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.

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.