Skip to content
This repository was archived by the owner on Nov 23, 2017. It is now read-only.

Using subprocesses involves the global event loop #421

Closed
Tinche opened this issue Sep 17, 2016 · 3 comments
Closed

Using subprocesses involves the global event loop #421

Tinche opened this issue Sep 17, 2016 · 3 comments

Comments

@Tinche
Copy link

Tinche commented Sep 17, 2016

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:

  • can't the event_loop instance passed in be used for this functionality, instead of the global event loop policy? If an instance of the loop is provided, in my opinion asyncio shouldn't care about any globals.
  • event_loop_policy.get_child_watcher() is undocumented as far as I can see.
@vxgmichel
Copy link

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
Copy link
Member

1st1 commented Oct 5, 2016

Should we close this one?

@Tinche
Copy link
Author

Tinche commented Oct 5, 2016

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants