classification
Title: asyncio: read pipe transport tries to resume reading after loop is gone
Type: behavior Stage:
Components: asyncio Versions: Python 3.4
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: gvanrossum, mwf, vstinner, yselivanov
Priority: normal Keywords: patch

Created on 2015-04-01 10:06 by mwf, last changed 2018-07-11 10:19 by vstinner.

Files
File name Uploaded Description Edit
child_reader.py mwf, 2015-04-01 10:06 Script to reproduce bug
read_pipe_pause_reading.patch vstinner, 2015-04-01 13:53 review
Messages (2)
msg239779 - (view) Author: Mathias Fröjdman (mwf) Date: 2015-04-01 10:06
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)
msg239810 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-04-01 13:53
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.
History
Date User Action Args
2018-07-11 10:19:04vstinnersettitle: read pipe transport tries to resume reading after loop is gone -> asyncio: read pipe transport tries to resume reading after loop is gone
2018-07-11 07:39:41serhiy.storchakasettype: crash -> behavior
2015-04-01 13:53:59vstinnersetfiles: + read_pipe_pause_reading.patch
keywords: + patch
messages: + msg239810
2015-04-01 10:06:02mwfcreate