bpo-35065: Remove `StreamReaderProtocol._untrack_reader` (#10212)

The call to `_untrack_reader` is performed too soon, causing the protocol
to forget about the reader before `connection_lost` can run and feed the
EOF to the reader. See bpo-35065.

4 files changed, 26 insertions, 11 deletions

diff --git a/Lib/asyncio/streams.py b/Lib/asyncio/streams.py
index 0afc66a..33fc303 100644
--- a/Lib/asyncio/streams.py
+++ b/Lib/asyncio/streams.py
@@ -227,9 +227,6 @@ class StreamReaderProtocol(FlowControlMixin, protocols.Protocol):
             self._reject_connection = True
         self._stream_reader_wr = None
 
-    def _untrack_reader(self):
-        self._stream_reader_wr = None
-
     @property
     def _stream_reader(self):
         if self._stream_reader_wr is None:
@@ -345,9 +342,6 @@ class StreamWriter:
         return self._transport.can_write_eof()
 
     def close(self):
-        # a reader can be garbage collected
-        # after connection closing
-        self._protocol._untrack_reader()
         self._transport.close()
 
     def is_closing(self):
diff --git a/Lib/asyncio/subprocess.py b/Lib/asyncio/subprocess.py
index c86de3d..90fc00d 100644
--- a/Lib/asyncio/subprocess.py
+++ b/Lib/asyncio/subprocess.py
@@ -36,11 +36,6 @@ class SubprocessStreamProtocol(streams.FlowControlMixin,
             info.append(f'stderr={self.stderr!r}')
         return '<{}>'.format(' '.join(info))
 
-    def _untrack_reader(self):
-        # StreamWriter.close() expects the protocol
-        # to have this method defined.
-        pass
-
     def connection_made(self, transport):
         self._transport = transport
 
diff --git a/Lib/test/test_asyncio/test_streams.py b/Lib/test/test_asyncio/test_streams.py
index 0141df7..043fac7 100644
--- a/Lib/test/test_asyncio/test_streams.py
+++ b/Lib/test/test_asyncio/test_streams.py
@@ -589,6 +589,7 @@ class StreamTests(test_utils.TestCase):
                 client_writer.write(data)
                 await client_writer.drain()
                 client_writer.close()
+                await client_writer.wait_closed()
 
             def start(self):
                 sock = socket.socket()
@@ -628,6 +629,7 @@ class StreamTests(test_utils.TestCase):
             # read it back
             msgback = await reader.readline()
             writer.close()
+            await writer.wait_closed()
             return msgback
 
         messages = []
@@ -666,6 +668,7 @@ class StreamTests(test_utils.TestCase):
                 client_writer.write(data)
                 await client_writer.drain()
                 client_writer.close()
+                await client_writer.wait_closed()
 
             def start(self):
                 self.server = self.loop.run_until_complete(
@@ -697,6 +700,7 @@ class StreamTests(test_utils.TestCase):
             # read it back
             msgback = await reader.readline()
             writer.close()
+            await writer.wait_closed()
             return msgback
 
         messages = []
@@ -987,6 +991,25 @@ os.close(fd)
 
         self.assertEqual(messages, [])
 
+    def test_eof_feed_when_closing_writer(self):
+        # See http://bugs.python.org/issue35065
+        messages = []
+        self.loop.set_exception_handler(lambda loop, ctx: messages.append(ctx))
+
+        with test_utils.run_test_server() as httpd:
+            rd, wr = self.loop.run_until_complete(
+                asyncio.open_connection(*httpd.address,
+                                        loop=self.loop))
+
+            f = wr.aclose()
+            self.loop.run_until_complete(f)
+            assert rd.at_eof()
+            f = rd.read()
+            data = self.loop.run_until_complete(f)
+            assert data == b''
+
+        self.assertEqual(messages, [])
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/Misc/NEWS.d/next/Library/2018-10-29-10-18-31.bpo-35065.CulMN8.rst b/Misc/NEWS.d/next/Library/2018-10-29-10-18-31.bpo-35065.CulMN8.rst
new file mode 100644
index 0000000..9d5d612
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2018-10-29-10-18-31.bpo-35065.CulMN8.rst
@@ -0,0 +1,3 @@
+Remove `StreamReaderProtocol._untrack_reader`. The call to `_untrack_reader`
+is currently performed too soon, causing the protocol to forget about the
+reader before `connection_lost` can run and feed the EOF to the reader.