From a4a760e24552aafc51de6cbe8b969790bcdd68f3 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Fri, 21 Jul 2017 02:04:33 +0300 Subject: [PATCH 1/3] 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. --- Lib/test/test_asyncore.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_asyncore.py b/Lib/test/test_asyncore.py index dc2f716e0bb828..4930cb3a16a00a 100644 --- a/Lib/test/test_asyncore.py +++ b/Lib/test/test_asyncore.py @@ -431,9 +431,11 @@ def test_resource_warning(self): def test_close_twice(self): fd = os.open(support.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() - f.close() self.assertEqual(f.fd, -1) # calling close twice should not fail f.close() From bd4b398d9991735dc75c33c05ddc3a24eda5deea Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Fri, 21 Jul 2017 02:33:55 +0300 Subject: [PATCH 2/3] bpo-30980: Fix double close protection Invalidated self.fd before closing, handling correctly the case when os.close raises. --- Lib/asyncore.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/asyncore.py b/Lib/asyncore.py index 705e4068130325..03d16838b739f9 100644 --- a/Lib/asyncore.py +++ b/Lib/asyncore.py @@ -619,8 +619,9 @@ def getsockopt(self, level, optname, buflen=None): def close(self): if self.fd < 0: return - os.close(self.fd) + fd = self.fd self.fd = -1 + os.close(fd) def fileno(self): return self.fd From e4f25d5bf88aafb3b969a3a6937f2a0130ff54f7 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Fri, 21 Jul 2017 03:14:23 +0300 Subject: [PATCH 3/3] bpo-30980: Fix fd leak introduced in the fixed test --- Lib/test/test_asyncore.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_asyncore.py b/Lib/test/test_asyncore.py index 4930cb3a16a00a..07edf2275bea0e 100644 --- a/Lib/test/test_asyncore.py +++ b/Lib/test/test_asyncore.py @@ -431,8 +431,9 @@ def test_resource_warning(self): def test_close_twice(self): fd = os.open(support.TESTFN, os.O_RDONLY) f = asyncore.file_wrapper(fd) - os.close(f.fd) # file_wrapper dupped fd + os.close(fd) + os.close(f.fd) # file_wrapper dupped fd with self.assertRaises(OSError): f.close()