Fixes issue #3826 and #4791:

Have SocketIO objects update their reference count in the underlying
socket object on close() so that the underlying socket object is
closed immediately when the last user is done rather than at an
unknown later time when garbage collection can do it.

diff --git a/Lib/socket.py b/Lib/socket.py
index 045987c086013f2e7d4efb369b9a1a61299c976d..737930d0032d97a09b3e23179a183f6e7a9a4d11 100644 (file)
--- a/Lib/socket.py
+++ b/Lib/socket.py
@@ -225,11 +225,12 @@ class SocketIO(io.RawIOBase):
         return self._writing and not self.closed
 
     def fileno(self):
+        self._checkClosed()
         return self._sock.fileno()
 
     @property
     def name(self):
-        return self._sock.fileno()
+        return self.fileno()
 
     @property
     def mode(self):
@@ -239,9 +240,12 @@ class SocketIO(io.RawIOBase):
         if self.closed:
             return
         io.RawIOBase.close(self)
+        self._sock._decref_socketios()
+        self._sock = None
 
     def __del__(self):
-        self._sock._decref_socketios()
+        if not self.closed:
+            self._sock._decref_socketios()
 
 
 def getfqdn(name=''):

diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py
index 51f41a9f2efa62904c377229f2058cf4c635b6cc..b0fe0577717e8c274250f3e86adfd9303fb72a8c 100644 (file)
--- a/Lib/test/test_socket.py
+++ b/Lib/test/test_socket.py
@@ -856,6 +856,16 @@ class FileObjectClassTestCase(SocketConnectedTest):
         self.assertEqual(self.cli_file.mode, 'wb')
         self.assertEqual(self.cli_file.name, self.serv_conn.fileno())
 
+    def testRealClose(self):
+        self.serv_file.close()
+        self.assertRaises(ValueError, self.serv_file.fileno)
+        self.cli_conn.close()
+        self.assertRaises(socket.error, self.cli_conn.getsockname)
+
+    def _testRealClose(self):
+        pass
+
+
 class UnbufferedFileObjectClassTestCase(FileObjectClassTestCase):
 
     """Repeat the tests from FileObjectClassTestCase with bufsize==0.
@@ -881,6 +891,29 @@ class UnbufferedFileObjectClassTestCase(FileObjectClassTestCase):
         self.cli_file.write(b"B. " + MSG)
         self.cli_file.flush()
 
+    def testMakefileClose(self):
+        # The file returned by makefile should keep the socket open...
+        self.cli_conn.close()
+        msg = self.cli_conn.recv(1024)
+        self.assertEqual(msg, MSG)
+        # ...until the file is itself closed
+        self.serv_file.close()
+        self.assertRaises(socket.error, self.cli_conn.recv, 1024)
+
+    def _testMakefileClose(self):
+        self.cli_file.write(MSG)
+        self.cli_file.flush()
+
+    def testMakefileCloseSocketDestroy(self):
+        refcount_before = sys.getrefcount(self.cli_conn)
+        self.serv_file.close()
+        refcount_after = sys.getrefcount(self.cli_conn)
+        self.assertEqual(refcount_before - 1, refcount_after)
+
+    def _testMakefileCloseSocketDestroy(self):
+        pass
+
+
 class LineBufferedFileObjectClassTestCase(FileObjectClassTestCase):
 
     bufsize = 1 # Default-buffered for reading; line-buffered for writing

diff --git a/Misc/NEWS b/Misc/NEWS
index 42637ddf5c4db6df23da59ef42a8c664dbc06373..442cc1f8962efb02d75f0a57c3ee67ba5648e693 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -111,6 +111,10 @@ Core and Builtins
 Library
 -------
 
+- Issue #3826 and #4791: The socket module now closes the underlying socket
+  appropriately when it is being used via socket.makefile() objects
+  rather than delaying the close by waiting for garbage collection to do it.
+
 - Issue #3860: GzipFile and BZ2File now support the context manager protocol.
 
 - Issue #4867: Fixed a crash in ctypes when passing a string to a