python / asyncio

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

Using subprocesses involves the global event loop #421

Closed Tinche closed 7 years ago

Tinche commented 7 years ago

Hello,

I'm the maintainer of pytest-asyncio. Pytest-asyncio allows users to block access to the global event loop, and pass the event loop explicitly; this is considered by some in the community as a best-practices approach.

However I've just received a bug report this approach doesn't work with subprocess (on Unix at least). Consider this test:

@pytest.mark.asyncio(forbid_global_loop=True)
async def test_subprocess(event_loop):
    await asyncio.create_subprocess_shell('ls', loop=event_loop)

======================================================================== FAILURES ========================================================================
____________________________________________________________________ test_subprocess _____________________________________________________________________

event_loop = <_UnixSelectorEventLoop running=False closed=False debug=False>

    @pytest.mark.asyncio(forbid_global_loop=True)
    async def test_subprocess(event_loop):
>       await asyncio.create_subprocess_shell('ls', loop=event_loop)

tests/test_subprocess.py:7: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/usr/lib/python3.5/asyncio/subprocess.py:197: in create_subprocess_shell
    stderr=stderr, **kwds)
/usr/lib/python3.5/asyncio/base_events.py:1049: in subprocess_shell
    protocol, cmd, True, stdin, stdout, stderr, bufsize, **kwargs)
/usr/lib/python3.5/asyncio/unix_events.py:179: in _make_subprocess_transport
    with events.get_child_watcher() as watcher:
/usr/lib/python3.5/asyncio/events.py:647: in get_child_watcher
    return get_event_loop_policy().get_child_watcher()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <pytest_asyncio.plugin.ForbiddenEventLoopPolicy object at 0x7f2050255588>

    def get_child_watcher(self):
        "Get the watcher for child processes."
>       raise NotImplementedError
E       NotImplementedError

/usr/lib/python3.5/asyncio/events.py:538: NotImplementedError

Two comments:

vxgmichel commented 7 years ago

I ran into this issue at some point, and here is the explanation:

  • Only the main thread can register signals, so the child watcher has to be bound to the main loop (the loop running in the main thread).
  • It is fine for other loops to watch subprocesses, as long as the main loop is running (so the main loop can notify the other ones using call_soon_threadsafe).

This was my conclusion:

From the design point of view, it is up to the policy to decide how the watcher is supposed to run. So if one chooses to go explicit (i.e. set_event_loop(None)), the loop should also be attached explicitly:

asyncio.set_event_loop(None)
loop = asyncio.new_event_loop()
asyncio.get_child_watcher().attach_loop(loop)

More information in issue #390 and PR #391.

1st1 commented 7 years ago

Should we close this one?

Tinche commented 7 years ago

Sure, I'll go through the linked issues and see what do to in pytest-asyncio.