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.

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("<L", self.crc & 0xffffffffL))
-                self.fileobj.write(struct.pack("<L", self.pos & 0xffffFFFFL))
-
-        if not self._extfileobj:
-            self.fileobj.close()
-
         self.closed = True
+        try:
+            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("<L", self.crc & 0xffffffffL))
+                    self.fileobj.write(struct.pack("<L", self.pos & 0xffffFFFFL))
+        finally:
+            if not self._extfileobj:
+                self.fileobj.close()
 
     def _init_read_gz(self):
         """Initialize for reading a gzip compressed fileobj.
@@ -1796,18 +1796,19 @@ class TarFile(object):
         if self.closed:
             return
 
-        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))
-
-        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.