bpo-30980: Fix double close in asyncore.file_wrapper (#2789) (#2900)

* bpo-30980: Fix close test to fail

test_close_twice was not considering the fact that file_wrapper is
duping the file descriptor. Closing the original descriptor left the
duped one open, hiding the fact that close protection is not effective.

* bpo-30980: Fix double close protection

Invalidated self.fd before closing, handling correctly the case when
os.close raises.

* bpo-30980: Fix fd leak introduced in the fixed test

diff --git a/Lib/asyncore.py b/Lib/asyncore.py
index 29099bdf5c0bb24a4e06809b2167ad18222f1963..105982f790cd1b29d0aedd8d32fccb40c10e39f9 100644 (file)
--- a/Lib/asyncore.py
+++ b/Lib/asyncore.py
@@ -633,7 +633,11 @@ if os.name == 'posix':
         write = send
 
         def close(self):
-            os.close(self.fd)
+            if self.fd < 0:
+                return
+            fd = self.fd
+            self.fd = -1
+            os.close(fd)
 
         def fileno(self):
             return self.fd

diff --git a/Lib/test/test_asyncore.py b/Lib/test/test_asyncore.py
index dc2b626f1e73ed3620a5ce08e973d1372dfe2775..693d67cd8a3d6089e797ae12678513c73a1b1ce5 100644 (file)
--- a/Lib/test/test_asyncore.py
+++ b/Lib/test/test_asyncore.py
@@ -442,6 +442,19 @@ class FileWrapperTest(unittest.TestCase):
         asyncore.loop(timeout=0.01, use_poll=True, count=2)
         self.assertEqual(b"".join(data), self.d)
 
+    def test_close_twice(self):
+        fd = os.open(TESTFN, os.O_RDONLY)
+        f = asyncore.file_wrapper(fd)
+        os.close(fd)
+
+        os.close(f.fd)  # file_wrapper dupped fd
+        with self.assertRaises(OSError):
+            f.close()
+
+        self.assertEqual(f.fd, -1)
+        # calling close twice should not fail
+        f.close()
+
 
 class BaseTestHandler(asyncore.dispatcher):