python / asyncio

asyncio historical repository
https://docs.python.org/3/library/asyncio.html
1.03k stars 178 forks source link

subprocess._loop deleted too soon causing exception when trying to read from pipe #484

Open BotoX opened 7 years ago

BotoX commented 7 years ago

From https://bugs.python.org/issue23242#msg284930

The following script is used to reproduce the bug:

import asyncio

async def execute():
    process = await asyncio.create_subprocess_exec(
        "timeout", "0.1", "cat", "/dev/urandom", stdout=asyncio.subprocess.PIPE)

    while True:
        data = await process.stdout.read(65536)
        print('read %d bytes' % len(data))
        if data:
            await asyncio.sleep(0.3)
        else:
            break

asyncio.get_event_loop().run_until_complete(execute())

will produce following output and terminate with exception:

read 65536 bytes
read 65536 bytes
Traceback (most recent call last):
  File "read_subprocess.py", line 18, in <module>
    asyncio.get_event_loop().run_until_complete(execute())
  File "/usr/lib/python3.6/asyncio/base_events.py", line 466, in run_until_complete
    return future.result()
  File "read_subprocess.py", line 9, in execute
    data = await process.stdout.read(65536)
  File "/usr/lib/python3.6/asyncio/streams.py", line 634, in read
    self._maybe_resume_transport()
  File "/usr/lib/python3.6/asyncio/streams.py", line 402, in _maybe_resume_transport
    self._transport.resume_reading()
  File "/usr/lib/python3.6/asyncio/unix_events.py", line 401, in resume_reading
    self._loop._add_reader(self._fileno, self._read_ready)
AttributeError: 'NoneType' object has no attribute '_add_reader'

When the process exits https://github.com/python/asyncio/blob/master/asyncio/unix_events.py#L444 is called which sets this._loop = None Next time read() is called on the pipe the above exception is thrown. I have tried to fix this issue myself but would sometimes have read terminate too early and miss the last chunks of data.

sethmlarson commented 7 years ago

Could this be solved by checking that self._transport._loop is not None within StreamReader._maybe_resume_transport?

When I add that it doesn't complain about _add_reader not being an attribute of NoneType.

BotoX commented 7 years ago

That'd fix the exception but if something is left in the pipe then that is gone...

sethmlarson commented 7 years ago

That's true, I'll look for a better fix with subprocess.

sethmlarson commented 7 years ago

So I turned SubprocessStreamProtocol.process_exited into a no-op but that will leave the Transport unclosed but fixes the problem and all data is received. Perhaps there's a way to do this via Process so the transport is only closed when an EOF is reached?

EDIT: So I've got a patch for this that I'll submit as a PR later tonight. Basically have to wait for all pipes to be closed/lost before finally closing the Transport.

sethmlarson commented 7 years ago

Submitted a PR #485 to fix this issue. Will work more on it tonight.