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.

Author: vxgmichel

Lib/asyncio/streams.py

@@ -227,9 +227,6 @@ def _on_reader_gc(self, wr):
             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 @@ def can_write_eof(self):
         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):

Lib/asyncio/subprocess.py

@@ -36,11 +36,6 @@ def __repr__(self):
             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
 

Lib/test/test_asyncio/test_streams.py

@@ -589,6 +589,7 @@ async def handle_client(self, client_reader, client_writer):
                 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 @@ async def client(addr):
             # read it back
             msgback = await reader.readline()
             writer.close()
+            await writer.wait_closed()
             return msgback
 
         messages = []
@@ -666,6 +668,7 @@ async def handle_client(self, client_reader, client_writer):
                 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 @@ async def client(path):
             # read it back
             msgback = await reader.readline()
             writer.close()
+            await writer.wait_closed()
             return msgback
 
         messages = []
@@ -987,6 +991,25 @@ def test_async_writer_api(self):
 
         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()

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.