]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
Issue #23865: close() methods in multiple modules now are idempotent and more
authorSerhiy Storchaka <storchaka@gmail.com>
Fri, 10 Apr 2015 10:24:10 +0000 (13:24 +0300)
committerSerhiy Storchaka <storchaka@gmail.com>
Fri, 10 Apr 2015 10:24:10 +0000 (13:24 +0300)
robust at shutdown. If needs to release multiple resources, they are released
even if errors are occured.

24 files changed:
Lib/aifc.py
Lib/binhex.py
Lib/chunk.py
Lib/distutils/text_file.py
Lib/dumbdbm.py
Lib/fileinput.py
Lib/ftplib.py
Lib/gzip.py
Lib/httplib.py
Lib/logging/__init__.py
Lib/logging/handlers.py
Lib/mailbox.py
Lib/multiprocessing/connection.py
Lib/multiprocessing/queues.py
Lib/shelve.py
Lib/smtplib.py
Lib/tarfile.py
Lib/telnetlib.py
Lib/tempfile.py
Lib/urllib.py
Lib/wave.py
Lib/xml/sax/expatreader.py
Lib/xmlrpclib.py
Misc/NEWS

index 9ac710fbde5f98e1b6a0bb49670ab925001d83b6..c9a021ee9d0309c3680794fc00d6781028151b7f 100644 (file)
@@ -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
index 8abc9f3e14c17a2f7421e8a7472993bb686047f4..14ec23375219d89611356b6b1bb0de8569d7b756 100644 (file)
@@ -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"""
index a8fbc1051f3e44794427408fa0c4dbc89b3c9103..3e3b5a4ea7b18939f9842a9ef1528d1e53946e7b 100644 (file)
@@ -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:
index 09a798b1902d773ea98765f3dd58dba3ca64b878..690cb80f3ccbb20480636ff9d815fa421b60e923 100644 (file)
@@ -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):
index 46d543df0c3a4a11650e6dfd40296c171949c06d..8ef7e79582c6f59a28872ff4a02dd7523d531543 100644 (file)
@@ -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
 
index 21c2d1f9bb7604780aab22973fd4e1cff3bd3f4e..02dc4c61273f284ab9989f92dc80ec1a95e071ef 100644 (file)
@@ -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:
index e1c3d99d1e3973bf29f70c859456a9ec11cc2292..449ce718cfb31b2c9c4a14c14860ec76577a10c5 100644 (file)
@@ -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
index 83cc0773d779de099c559450d30bf10eabdf1a41..de0dab1037436d446bb626d711ea3cf85cdb1a8e 100644 (file)
@@ -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()
index 5406e77d4d190d314b9881e118e329e23ed37673..9f1e088e8ed75c10cb025379e9bbb5dc8fc7d244 100644 (file)
@@ -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."""
index b2a7711dc36878538739db9d7b630bb27689a15d..bd4afeb1aa34dcafc45ed3a3c01a84be46393787 100644 (file)
@@ -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()
 
index a458529684f214ed7cc44baac5bd108609b9cada..e430ab7b9b6588199116b3cf9e5234438e6ce501 100644 (file)
@@ -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()
index ba497530872db06178cf3a5c073c9af2e50b667b..3866953ec584c643d9de388c5209cf61691782e0 100644 (file)
@@ -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."""
index 463d4723effed80b3b088558e797964c28049d58..645a26f069eaa2f28ab102117271effa8cd06c0b 100644 (file)
@@ -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):
index 487e0afb6a76a35ddea79bc9f662337e2914ba16..a88e298973ca8782779451eff5345f48310a184a 100644 (file)
@@ -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()')
index c8cba8582d6ff1ed7a4c7af6e42492fdfca3d13d..4f1e49dc3b22e11fcfaa6c0c8f177fcf9b311be6 100644 (file)
@@ -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'):
index d1c280618f548680a9a16b5814e02e25b30b394c..8388b984a11aca88face0055480e8620a275e43f 100755 (executable)
@@ -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):
index bc0b09fa88cd29e0c2884008e96070eaa3d373ce..082f36117902c0265631f96d58431444f9496fa2 100644 (file)
@@ -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
index 88aa482d014db49ba7c7ed77f51e309cf5292d2e..2eaa8e3709861acfda572b12810b9b4bdd3678f0 100644 (file)
@@ -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."""
index a4083f75997ea89e3e855ac77717e5b23399a7e1..fbda8ebeb732ad4a1423647da16cee8e79ea4a68 100644 (file)
@@ -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()
index 9f972e3c1d40957bcbe71f8f4e52ba4f93d41681..ccb05749355dfaa3c32cf1c9a25c76585cdd5580 100644 (file)
@@ -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."""
index 8ff93c37f2025129d573ca9c2c3807bb1c80cd4b..97f21468ef671fb642af8e5c7fc211f083207c26 100644 (file)
@@ -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.
index 9de3e72307d8ff43cadf9dc339b90618a0b4b009..cd243e62051e76713fed54421606c728367ecf94 100644 (file)
@@ -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 = \
index 340dc51eb6aebd3fe55627c261ef9df32b2a67a5..db185a6a1214fa0e740ba02101c9de804199307b 100644 (file)
@@ -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.
index 5aca0a8cb686cc838a4ac7b4dd2127c769323ed8..90a14390cc7102d2b1224a3deab59332f2d62eb3 100644 (file)
--- 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.