Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-66435: Allow nested event loops #93338

Closed
wants to merge 1 commit into from

Conversation

davidbrochart
Copy link

@davidbrochart davidbrochart commented May 29, 2022

This PR is basically a copy/paste of @erdewit's nest_asyncio library. It allows the event loop to be re-entrant, by making it possible to call asyncio.run() inside an already running event loop.
This feature request was rejected in the past, but @1st1 recently mentioned that he could reconsider this decision.
Here is an example:

import asyncio

async def task(name):
    for i in range(10):
        await asyncio.sleep(0.1)
        print(f"from {name}: {i}")

async def bar():
    asyncio.create_task(task("bar"))
    await asyncio.sleep(1.1)
    print("bar done")

def foo():
    # asyncio.run inside an already running event loop
    # pre-empts the execution of any other task in the event loop
    asyncio.run(bar())

async def main():
    t = asyncio.create_task(task("main"))  # not executed until foo() is done
    foo()
    await asyncio.sleep(1.1)  # t resumes execution

asyncio.run(main())

This will print:

from bar: 0
from bar: 1
from bar: 2
from bar: 3
from bar: 4
from bar: 5
from bar: 6
from bar: 7
from bar: 8
from bar: 9
bar done
from main: 0
from main: 1
from main: 2
from main: 3
from main: 4
from main: 5
from main: 6
from main: 7
from main: 8
from main: 9

@cpython-cla-bot
Copy link

cpython-cla-bot bot commented May 29, 2022

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@AA-Turner
Copy link
Member

Please update the title to gh-66435: Allow nested event loops (although closed, it is the most appropriate issue to link against). You also need a NEWS entry.

A

@davidbrochart davidbrochart changed the title Allow nested event loops gh-66435: Allow nested event loops May 30, 2022
@maartenbreddels
Copy link

This is different from nest_asyncio right?

This PR only allows 'nested event' loops, which I think is really great (I vaex we do this using this decorator https://github.com/vaexio/vaex/blob/633970528cb5091ef376dbca2e4721cd42525419/packages/vaex-core/vaex/asyncio.py#L7 but it's using the asyncio.events._set_running_loop private API).

'nest_asyncio' will also run the tasks of the parent/previous event loop, i.e. it will give an output like this:

from main: 0
from bar: 0
from main: 1
from bar: 1
from main: 2
from bar: 2
from main: 3
from bar: 3
from main: 4
from bar: 4
from main: 5
from bar: 5
from main: 6
from bar: 6
from main: 7
from bar: 7
from main: 8
from bar: 8
from main: 9
from bar: 9
bar done

Which basically leads to a sync_call_that_can_resume_the_async_task_I_created_on_the_previous_line() which can be very dangerous (although sometimes very useful).

I think with this PR it is not possible to resume a task from the previous event_loop, is that right?

To make it more concrete, this example works with nest_asyncio:

 import asyncio
 
+import nest_asyncio
+
+nest_asyncio.apply()
+
+t = None
+
 
 async def task(name):
     for i in range(10):
@@ -8,6 +14,7 @@
 
 
 async def bar():
+    await t
     asyncio.create_task(task("bar"))
     await asyncio.sleep(1.1)
     print("bar done")
@@ -20,6 +27,7 @@
 
 
 async def main():
+    global t
     t = asyncio.create_task(task("main"))  # not executed until foo() is done
     foo()
     await asyncio.sleep(1.1)  # t resumes execution

output:

from main: 0
from main: 1
from main: 2
from main: 3
from main: 4
from main: 5
from main: 6
from main: 7
from main: 8
from main: 9
from bar: 0
from bar: 1
from bar: 2
from bar: 3
from bar: 4
from bar: 5
from bar: 6
from bar: 7
from bar: 8
from bar: 9
bar done

Does this PR give an exception, does it run it, or does it deadlock? I think the appropriate behavior should be an exception (complaining the awaited task t was not created with the running event loop).

@davidbrochart
Copy link
Author

@maartenbreddels indeed, I didn't copy exactly the code from nest_asyncio and I must have forgotten something. I'll look at it again, but anyway as @erdewit mentioned, this code would probably need to be written by core developers of CPython, and this PR is just a POC.
It raises this question though: do we want asyncio.run() to block the execution of other tasks, as it is the case at the current stage of this PR, or do we want them to also execute concurrently, as it is the case in nest_asyncio? In the former case, maybe your solution in Vaex is enough. Correct me if I'm wrong, but it's "just" allowing to run another event loop while already in a running event loop, right?

@maartenbreddels
Copy link

do we want asyncio.run() to block the execution of other tasks

If we don't, then it is not cooperative multitasking anymore. But I can't find in the Python docs that asyncio claims it is actually. In any case, it will probably break existing code.

@davidbrochart
Copy link
Author

then it is not cooperative multitasking anymore.

I think it would be less dangerous, as you mentioned, that asyncio.run() always blocks any other task. Otherwise, having other tasks running while it doesn't have await would really be confusing and break the most basic rule of asyncio.
I think it's still different than what you do in Vaex, since it allows to re-use the same event loop, while in Vaex you run in a new event loop. In jupyter_client for instance, we need to re-use the same event loop as pyzmq.

In any case, it will probably break existing code.

Yes, that's why we would need to opt-in for this behavior.

@graingert
Copy link
Contributor

@davidbrochart would it be possible for jupyter_client to spawn a python subprocess for running user code in instead?

@davidbrochart
Copy link
Author

A Jupyter kernel already runs in its own process, and jupyter-client communicates with it through ZMQ sockets.
When we added the async API to jupyter-client, we also tried implementing the blocking API by running the async functions in a separate thread with its own event loop (similarly to what anyio's to_thread.run_sync does), but we ran into issues related to pyzmq's async context running in a different event loop.
I still think allowing nested event loops would be useful (as an opt-in behavior), but I'd like to know if this PR could get a chance to get in before investing more time in it.

@davidbrochart
Copy link
Author

I've simplified the changes to what I think is the minimum to get the behavior described in the top comment. Now the nested call to asyncio.run should explicitly pass running_ok=True in order to re-enter the event loop.

asyncio.run(bar(), running_ok=True)

Copy link
Contributor

@willingc willingc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidbrochart @maartenbreddels If you have time to update this PR, we can discuss it at JupyterCon.

@davidbrochart
Copy link
Author

davidbrochart commented Apr 27, 2023

Thanks for the heads up @willingc, I rebased on main.
Here is an example usage:

import asyncio

async def task(name):
    for i in range(10):
        await asyncio.sleep(0.1)
        print(f"from {name}: {i}")

async def bar():
    asyncio.create_task(task("bar"))
    await asyncio.sleep(1.1)
    print("bar done")

def foo():
    # asyncio.run inside an already running event loop
    # pre-empts the execution of any other task in the event loop
    asyncio.run(bar(), running_ok=True)

async def main():
    t = asyncio.create_task(task("main"))  # not executed until foo() is done
    foo()
    await asyncio.sleep(1.1)  # t resumes execution

asyncio.run(main())

That will print:

from bar: 0
from bar: 1
from bar: 2
from bar: 3
from bar: 4
from bar: 5
from bar: 6
from bar: 7
from bar: 8
from bar: 9
bar done
from main: 0
from main: 1
from main: 2
from main: 3
from main: 4
from main: 5
from main: 6
from main: 7
from main: 8
from main: 9

If we wanted the nested call to asyncio.run not block the top-level one, i.e. have:

from main: 0
from bar: 0
from main: 1
from bar: 1
from main: 2
from bar: 2
from main: 3
from bar: 3
from main: 4
from bar: 4
from main: 5
from bar: 5
from main: 6
from bar: 6
from main: 7
from bar: 7
from main: 8
from bar: 8
from main: 9
from bar: 9
bar done

We would not really need this PR, we would just check if a loop is already running, and if yes then call asyncio.create_task, otherwise call asyncio.run.

@arhadthedev
Copy link
Member

@davidbrochart Could you sign the CLA, please? #93338 (comment)

@davidbrochart
Copy link
Author

@davidbrochart Could you sign the CLA, please? #93338 (comment)

Done.

@gvanrossum
Copy link
Member

Whoa. A process issue: discussions about whether a particular feature should be accepted should happen on an Issue, not in a PR. The best thing to do is to reopen gh-66435, which I will do. See you there! (I do appreciate the discussion and the clear examples of two different proposed behaviors.)

@willingc
Copy link
Contributor

@davidbrochart I'm closing this PR since we decided to not add nested asyncio to the library. #66435 (comment) Thanks for your effort here and your suggestion on the issue for a third party library.

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

Successfully merging this pull request may close these issues.

8 participants