Skip to content
This repository has been archived by the owner on Mar 6, 2024. It is now read-only.

tasks are not cancelled after ctrl-c #58

Closed
qci-amos opened this issue Aug 19, 2021 · 7 comments
Closed

tasks are not cancelled after ctrl-c #58

qci-amos opened this issue Aug 19, 2021 · 7 comments

Comments

@qci-amos
Copy link

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

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

erdewit commented Sep 8, 2021

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

qci-amos commented Sep 8, 2021

Thanks for looking into this.

Some thoughts:

  • the reason I'm using this is for integration with Jupyter -- so in practice there's only one run invocation at a time and the nesting is pretty simple.
  • At least for the use case I have in mind, ctrl-c, the user intends to kill everything. The cancel is to facilitate telling the remote server it can stop sending the client more data.

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

erdewit commented Sep 8, 2021

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

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

edraft commented Nov 16, 2021

Any updates?

@erdewit
Copy link
Owner

erdewit commented Nov 25, 2021

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

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