asyncio: read pipe transport tries to resume reading after loop is gone

[mwfrojdman]

BPO	23837
Nosy	@gvanrossum, @vstinner, @asvetlov, @1st1, @mwfrojdman
Files	child_reader.py: Script to reproduce bug read_pipe_pause_reading.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-09-13.12:15:15.039>
created_at = <Date 2015-04-01.10:06:02.650>
labels = ['type-bug', 'expert-asyncio']
title = 'asyncio: read pipe transport tries to resume reading after loop is gone'
updated_at = <Date 2019-09-13.12:15:15.037>
user = 'https://github.com/mwfrojdman'

bugs.python.org fields:

activity = <Date 2019-09-13.12:15:15.037>
actor = 'asvetlov'
assignee = 'none'
closed = True
closed_date = <Date 2019-09-13.12:15:15.039>
closer = 'asvetlov'
components = ['asyncio']
creation = <Date 2015-04-01.10:06:02.650>
creator = 'mwf'
dependencies = []
files = ['38778', '38784']
hgrepos = []
issue_num = 23837
keywords = ['patch']
message_count = 3.0
messages = ['239779', '239810', '352305']
nosy_count = 6.0
nosy_names = ['gvanrossum', 'vstinner', 'sascha_silbe', 'asvetlov', 'yselivanov', 'mwf']
pr_nums = []
priority = 'normal'
resolution = 'out of date'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue23837'
versions = ['Python 3.4']

[mwfrojdman]

Script attached which reproduces the issue.

Steps to reproduce:
0) Use python 3.4.3 on Linux. Does not repro with 3.4.0 or 3.4.2.

1. Create a child process with asyncio.create_child_exec and stdout=PIPE
2. loop yield from child.read(n) (i used n=2048, any other positive value < StreamRead._limit ought to work too)
3. Write >= StreamRead._limit * 2 + 1 (by default 2**17+1) bytes from child process and exit

File referenced below: asyncio/streams.py

feed_data is called when data arrives from the child process. Having more than 2 * self._limit bytes in self._buffer will lead to StreamReader pausing reading on line 372.

feed_eof sets self._eof to True, but that value is not checked in relevant sections later.

Child process exits, which will lead to self._loop = None being set apparently on line 405 of _UnixReadPipeTransport._call_connection_lost in asyncio/unix_events.py (could not find any other location where it would be set to None).

After a number of read()s, self._maybe_resume_transport() is called when len(self._buffer) <= self._limit (ie. <= 64k left in buffer). self._paused will evaluate true too, so self._transport.resume_reading() is called on line 349.

That will call self._loop.add_reader(self._fileno, self._read_ready) on line 364 of asyncio/unix_events.py. self._loop is None, so and AttributeError is raised when None has no add_reader attribute:

Traceback (most recent call last):
  File "child_reader.py", line 29, in <module>
    print('read {} bytes from child'.format(loop.run_until_complete(fail())))
  File "/home/frojma/python-3.4.3/lib/python3.4/asyncio/base_events.py", line 316, in run_until_complete
    return future.result()
  File "/home/frojma/python-3.4.3/lib/python3.4/asyncio/futures.py", line 275, in result
    raise self._exception
  File "/home/frojma/python-3.4.3/lib/python3.4/asyncio/tasks.py", line 238, in _step
    result = next(coro)
  File "child_reader.py", line 15, in fail
    chunk = yield from child.stdout.read(2048)
  File "/home/frojma/python-3.4.3/lib/python3.4/asyncio/streams.py", line 478, in read
    self._maybe_resume_transport()
  File "/home/frojma/python-3.4.3/lib/python3.4/asyncio/streams.py", line 354, in _maybe_resume_transport
    self._transport.resume_reading()
  File "/home/frojma/python-3.4.3/lib/python3.4/asyncio/unix_events.py", line 364, in resume_reading
    self._loop.add_reader(self._fileno, self._read_ready)

[vstinner]

Oh, it looks like all pause_reading() and resume_reading() methods of transports check the status the transport (especially the _closing attribute), except two transports: _UnixReadPipeTransport and _SSLProtocolTransport.

For _SSLProtocolTransport, I don't think that it matters because this transport is not used directly, but through _SelectorSslTransport or _ProactorSocketTransport and these transports already check the status.

Here is a patch for _UnixReadPipeTransport, without patch yet.

[asvetlov]

The issue can be closed, loop.add_reader() / loop.remove_reader() calls correctly handle closed loop now