erdewit / nest_asyncio

Patch asyncio to allow nested event loops
BSD 2-Clause "Simplified" License
693 stars 79 forks source link

tasks are not cancelled after ctrl-c #58

Closed qci-amos closed 2 years ago

qci-amos commented 3 years ago

This seems related enough to #57 to be a duplicate, but it also seems sufficiently different (i.e., the CancelledError is not raised) to merit its own issue.

What I'd like is for unfinished tasked to be cancelled when, e.g., a ctrl-c happens.

Here's a minimal example:

import asyncio
import nest_asyncio

if not True:
    print("applying nest_asyncio")
    nest_asyncio.apply()

async def get_results():
    print("start")
    try:
        await asyncio.sleep(5)
    except asyncio.CancelledError:
        print("cancelled!")
    print("stop")
    return 123

try:
    res = asyncio.run(get_results())
    print("res", res)
except KeyboardInterrupt:
    print("received ctrl-c")
    raise

results in

$ python cancel2.py
applying nest_asyncio
start
^Creceived ctrl-c
Traceback (most recent call last):
  File "cancel2.py", line 20, in <module>
    res = asyncio.run(get_results())
  File "/mnt/c/Users/me/Documents/nest_asyncio/nest_asyncio.py", line 32, in run
    return loop.run_until_complete(future)
  File "/mnt/c/Users/me/Documents/nest_asyncio/nest_asyncio.py", line 64, in run_until_complete
    self._run_once()
  File "/mnt/c/Users/me/Documents/nest_asyncio/nest_asyncio.py", line 87, in _run_once
    event_list = self._selector.select(timeout)
  File "/home/me/miniconda3/envs/aq/lib/python3.8/selectors.py", line 468, in select
    fd_event_list = self._selector.poll(timeout, max_ev)
KeyboardInterrupt

(the CancelledError is unexpectedly not raised), vs the desired:

$ python cancel2.py
start
^Ccancelled!
stop
received ctrl-c
Traceback (most recent call last):
  File "cancel2.py", line 20, in <module>
    res = asyncio.run(get_results())
  File "/home/me/miniconda3/envs/aq/lib/python3.8/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/home/me/miniconda3/envs/aq/lib/python3.8/asyncio/base_events.py", line 603, in run_until_complete
    self.run_forever()
  File "/home/me/miniconda3/envs/aq/lib/python3.8/asyncio/base_events.py", line 570, in run_forever
    self._run_once()
  File "/home/me/miniconda3/envs/aq/lib/python3.8/asyncio/base_events.py", line 1823, in _run_once
    event_list = self._selector.select(timeout)
  File "/home/me/miniconda3/envs/aq/lib/python3.8/selectors.py", line 468, in select
    fd_event_list = self._selector.poll(timeout, max_ev)
KeyboardInterrupt

I'm using the latest nest_asyncio installed from source.

$ pip list | grep nest_
nest-asyncio                  1.5.1               /mnt/c/Users/me/Documents/nest_asyncio
qci-amos commented 3 years ago

this seems to address the problem for me, but I have no idea how robust it is to various cases:

diff --git a/nest_asyncio.py b/nest_asyncio.py
index 302509a..5a01b12 100644
--- a/nest_asyncio.py
+++ b/nest_asyncio.py
@@ -29,7 +29,11 @@ def _patch_asyncio():
     def run(future, *, debug=False):
         loop = asyncio.get_event_loop()
         loop.set_debug(debug)
-        return loop.run_until_complete(future)
+        try:
+            return loop.run_until_complete(future)
+        finally:
+            if sys.version_info >= (3, 7, 0):
+                asyncio.runners._cancel_all_tasks(loop)

     if sys.version_info >= (3, 6, 0):
         asyncio.Task = asyncio.tasks._CTask = asyncio.tasks.Task = \
erdewit commented 3 years ago

asyncio.runners._cancel_all_tasks(loop)

This is what the standard asyncio.run uses. With standard asyncio there is at most one run invocation at a time, but with nest_asyncio there can be multiple. If one run is finished and cancels all running tasks, also from the other run(s), then problems arise.

Solving this would involve keeping track of all tasks associated with a certain run, which looks difficult to me right now.

Btw, this issue does not seem related to #57 as far as I can tell.

qci-amos commented 3 years ago

Thanks for looking into this.

Some thoughts:

If you're willing to alter your api to run, it would be an option for me to pass some kind of kwarg to run to tell nest_asyncio that it's ok to cancel everything.

That said, perhaps it's looking pretty hacky to support these limited use-cases?

erdewit commented 3 years ago

If it's okay to cancel all tasks, then why not implement the run method that you posted above in your code? It is not a must to use asyncio.run as the coroutine runner.

qci-amos commented 3 years ago

One thought is nest_asyncio could at least do the cancel in an except clause instead of finally. The idea being if there's an exception then it's ok if all tasks get cancelled across all nested levels. This matches the std implementation for this case.

I would think that I'm not the only person who wants to use async from jupyter with ability to interrupt. But yea, if nest_asyncio doesn't support it, then I should be able to do it myself without too much hassle as you suggest. It just seems awkward to monkey patch a monkey patch! 🤔

edraft commented 2 years ago

Any updates?

erdewit commented 2 years ago

This issue is fixed by canceling the task that is given to asyncio.run.