[Python-checkins] bpo-33263: Fix FD leak in _SelectorSocketTransport (GH-6450) (#7022)

[Andrew Svetlov]

https://github.com/python/cpython/commit/b8b800090ff0954117a26ffcb501307823f3d33a
commit: b8b800090ff0954117a26ffcb501307823f3d33a
branch: 3.7
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: Andrew Svetlov <andrew.svetlov at gmail.com>
date: 2018-05-21T11:35:25+03:00
summary:

bpo-33263: Fix FD leak in _SelectorSocketTransport (GH-6450) (#7022)

* bpo-33263 Fix FD leak in _SelectorSocketTransport. (GH-6450)

Under particular circumstances _SelectorSocketTransport can try to add a reader
even the transport is already being closed. This can lead to FD leak and
invalid stated of the following connections. Fixed the SelectorSocketTransport
to add the reader only if the trasport is still active.
(cherry picked from commit a84d0b361a26c05c6fadc6640591ec3feee5bfb5)

Co-authored-by: Vlad Starostin <drtyrsa at yandex.ru>

files:
A Misc/NEWS.d/next/Library/2018-04-11-20-29-19.bpo-33263.B56Hc1.rst
M Lib/asyncio/selector_events.py
M Lib/test/test_asyncio/test_selector_events.py

diff --git a/Lib/asyncio/selector_events.py b/Lib/asyncio/selector_events.py
index 354bf9d1c2de..f9533a1d77be 100644
--- a/Lib/asyncio/selector_events.py
+++ b/Lib/asyncio/selector_events.py
@@ -706,6 +706,12 @@ def _call_connection_lost(self, exc):
     def get_write_buffer_size(self):
         return len(self._buffer)
 
+    def _add_reader(self, fd, callback, *args):
+        if self._closing:
+            return
+
+        self._loop._add_reader(fd, callback, *args)
+
 
 class _SelectorSocketTransport(_SelectorTransport):
 
@@ -732,7 +738,7 @@ def __init__(self, loop, sock, protocol, waiter=None,
 
         self._loop.call_soon(self._protocol.connection_made, self)
         # only start reading when connection_made() has been called
-        self._loop.call_soon(self._loop._add_reader,
+        self._loop.call_soon(self._add_reader,
                              self._sock_fd, self._read_ready)
         if waiter is not None:
             # only wake up the waiter when connection_made() has been called
@@ -754,7 +760,7 @@ def resume_reading(self):
         if self._closing or not self._paused:
             return
         self._paused = False
-        self._loop._add_reader(self._sock_fd, self._read_ready)
+        self._add_reader(self._sock_fd, self._read_ready)
         if self._loop.get_debug():
             logger.debug("%r resumes reading", self)
 
@@ -930,7 +936,7 @@ def __init__(self, loop, sock, protocol, address=None,
         self._address = address
         self._loop.call_soon(self._protocol.connection_made, self)
         # only start reading when connection_made() has been called
-        self._loop.call_soon(self._loop._add_reader,
+        self._loop.call_soon(self._add_reader,
                              self._sock_fd, self._read_ready)
         if waiter is not None:
             # only wake up the waiter when connection_made() has been called
diff --git a/Lib/test/test_asyncio/test_selector_events.py b/Lib/test/test_asyncio/test_selector_events.py
index 219ab0eb5b8b..684c29dec3e2 100644
--- a/Lib/test/test_asyncio/test_selector_events.py
+++ b/Lib/test/test_asyncio/test_selector_events.py
@@ -871,6 +871,21 @@ def test_connection_lost(self):
         self.assertIsNone(tr._protocol)
         self.assertIsNone(tr._loop)
 
+    def test__add_reader(self):
+        tr = self.create_transport()
+        tr._buffer.extend(b'1')
+        tr._add_reader(7, mock.sentinel)
+        self.assertTrue(self.loop.readers)
+
+        tr._force_close(None)
+
+        self.assertTrue(tr.is_closing())
+        self.assertFalse(self.loop.readers)
+
+        # can not add readers after closing
+        tr._add_reader(7, mock.sentinel)
+        self.assertFalse(self.loop.readers)
+
 
 class SelectorSocketTransportTests(test_utils.TestCase):
 
diff --git a/Misc/NEWS.d/next/Library/2018-04-11-20-29-19.bpo-33263.B56Hc1.rst b/Misc/NEWS.d/next/Library/2018-04-11-20-29-19.bpo-33263.B56Hc1.rst
new file mode 100644
index 000000000000..77994f6a5986
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2018-04-11-20-29-19.bpo-33263.B56Hc1.rst
@@ -0,0 +1 @@
+Fix FD leak in `_SelectorSocketTransport`  Patch by Vlad Starostin.