-
Notifications
You must be signed in to change notification settings - Fork 0
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
Breaks with nest-asyncio #3
Comments
I would consider this a bug in nest_asyncio. This hook is based on patching That means the workaround ought to be to use the pre- print("before")
loop = asyncio.new_event_loop()
loop.run_until_complete(main())
loop.close()
print("after") I'm not sure if there's a reason nest_asyncio doesn't close the loop like asyncio.run does (it would make sense if it re-uses event loops, unlike asyncio.run). If it doesn't re-use loops, this is likely a tiny fix in nest_asyncio. |
Thanks @minrk. AFAIK it does reuse loops so the fix may be more involved. Appreciate the workaround, I'll take a look. |
I opened erdewit/nest_asyncio#79 which should fix regular (not re-entrant) calls to asyncio.run. Can I ask more about how you are using it and when you want to make sure cleanup is called? The big assumption here is that event_loop.close will be called, because otherwise we can't be sure it will be. But one thing we can do is also register loop.close with actual atexit, so that it will at least be called at process teardown. |
#6 adds an actual atexit hook to call loop.close if it hasn't been called yet. So if you don't call loop.close, you'll get a ResourceWarning and the loop will be closed. It's still a change with nest_asyncio, in that the close will be called at a different time (process teardown rather than at the end of asyncio.run), but at least it will be called. There should be no change if loop.close is called explicitly. |
My use case (and I suspect many others) is to allow using Jupyter is the primary example here where notebook cells are technically executed within an event loop, but many users aren't aware of this and just use sync code. import asyncio
## My library
# By default my library uses async/await but also exposes a sync API using asyncio.run
async def my_library_function():
pass
def sync_api_for_my_library():
return asyncio.run(my_library_function()) # This fails if not using nest-asyncio
## User code
# Many users are not aware or in the habit of using async/await in Jupyter
# so they just use regular sync functions and APIs
def jupyter_notebook_cell():
sync_api_for_my_library()
## Jupyter notebook kernel
async def jupyter_kernel_runtime():
jupyter_notebook_cell()
if __name__ == "__main__":
asyncio.run(jupyter_kernel_runtime()) In an ideal world Jupyter users would use the asyncio API for my library. But if they don't they will run into errors unless we use something like |
Thinking about it some more I don't really want the atexit to be called when the inner If I change the outer loop startup to use the old style # workaround-mre.py
import asyncio
import asyncio_atexit
import nest_asyncio
nest_asyncio.apply()
async def finalizer():
print("running finalizer")
async def main():
asyncio_atexit.register(finalizer)
if __name__ == "__main__":
print("before")
loop = asyncio.new_event_loop()
loop.run_until_complete(main())
loop.close()
print("after") $ python workaround-mre.py
before
running finalizer
after But this would mean that folks using my library would need to ensure they start the loop in the same way, which I can't guarantee. |
Because using In gneral, asyncio_atexit relies on the event loop cleanly shutting down, just like atexit relies on the process cleanly shutting down (e.g. doesn't run if killed), so if there isn't a clean shutdown it may not get called. #6 should ensure close gets called, even without ipython/ipykernel#1116 (again, assuming the kernel actually does finish shutting down cleanly). I've found running my own event loop in a thread for sync wrappers to be a simpler and more reliable approach to using nest_asyncio. But I know threads don't always work for people. And the advantage (and maybe disadvantage sometimes?) of nest_asyncio is that I think it actually cooperates with the parent loop, rather than blocking it, which a thread actually does. nest_asyncio rejected my patch, but it wouldn't have affected the IPython case anyway, since it only touched the 'standard' not re-entrant calls to asyncio.run in your MWE, and not the case you actually care about. |
Thanks @minrk for all the discussion here and for spending time on this. Funnily enough I've done exactly that and changed my implementation to start a second loop in a new thread and use that to run my coroutines. This way I don't need Given how I'm not too worried about the thread blocking the outer event loop because the user has called a sync function in an event loop anyway, so the choice to block the loop has already been made. |
Technically this issue is still valid, |
Depends a little on what you mean. I think they are compatible, but there's a caveat worth documenting in that nest_asyncio changes when event loops are closed, which in turn changes when this hook may fire. As long as the eventloop is closed eventually, this should be okay. I'm not sure why #3 isn't working in your case, but it does seem like a reasonable addition, anyway. Something like:
I personally think nest_asyncio makes a fundamental mistake in breaking the expectation that asyncio.run runs an event loop to completion, which it no longer does. It has instead made the choice to always leave the event loop open and never clean it up, which means it's on the user or application to call |
Yeah I totally agree. |
I recently added
nest-asyncio
to a project that usesasyncio-atexit
and it seems to break it. Do you have any thoughts on how I could use both together?Minimal reproducible example
Before
After
The text was updated successfully, but these errors were encountered: