inconsistency in asyncio.Task between cancellation while running vs. cancellation immediately after it finishes #82429

Activity

1. njsmith commented on Sep 22, 2019

   

   njsmith

   on Sep 22, 2019

   ContributorAuthor

   More actions

   Just noticed this while looking at the code to asyncio.Task.__step

   asyncio Futures have two different error states: they can have an arbitrary exception set, or they can be marked as cancelled.

   asyncio Tasks handle this by detecting what exception was raised by the task code: if it's a CancelledError, then the mark the Future as cancelled, and if it's anything else, they set that as the Future's exception.

   There is also a special case handled inside the 'except StopIteration' clause in Task.__step. If we request that a Task be cancelled, but then the task exits normally before we have a chance to throw in a CancelledError, then we also want mark the Future as cancelled. BUT, this special case is handled incorrectly: it does Future.set_exception(CancelledError()), instead of Future.cancel(). Normally it's impossible for a task to end up with its exception set to CancelledError, but it does happen in this one weird edge case, basically as a race condition.

   Here's some sample code to illustrate the problem (tested on 3.7):

   ------

   import asyncio
   
   # This gets cancelled normally
   async def cancel_early():
       asyncio.current_task().cancel()
       await asyncio.sleep(1)
   
   async def cancel_late():
       asyncio.current_task().cancel()
       # No sleep, so CancelledError doesn't get delivered until after the
       # task exits
   
   async def main():
       t_early = asyncio.create_task(cancel_early())
       await asyncio.sleep(0.1)
       print(f"t_early.cancelled(): {t_early.cancelled()!r}")
   
       t_late = asyncio.create_task(cancel_late())
       await asyncio.sleep(0.1)
       print(f"t_late.cancelled(): {t_late.cancelled()!r}")
       print(f"t_late.exception(): {t_late.exception()!r}")
   
   asyncio.run(main())

   ---

   Output:

   t_early.cancelled(): True
   t_late.cancelled(): False
   t_late.exception(): CancelledError()

   The obvious fix would be to modify the 'except StopIteration' branch to handle this case by calling super().cancel() instead of super().set_exception(...).

   Alternatively, I could see an argument that asyncio.Task should always preserve the CancelledError, so that e.g. you can get a proper traceback. In that case we'd need to change the 'except CancelledError' branch to call super().set_exception(...) instead of super().cancel(). This would also need some more changes, like overriding .cancelled() to check for a stored exception and then return isinstance(stored_exc, CancelledError), and maybe others... I'm not sure of the full consequences.

   But handling these two cases differently is definitely wrong, that part I'm sure of :-)
2. 1st1 commented on Sep 22, 2019

   

   1st1

   on Sep 22, 2019

   Member

   More actions

   The obvious fix would be to modify the 'except StopIteration' branch to handle this case by calling super().cancel() instead of super().set_exception(...).

   Yeah, I think this is the solution we should do in 3.8.
3. willingc commented on Sep 25, 2019

   

   willingc

   on Sep 25, 2019

   Contributor

   More actions

   New changeset edad4d8 by Carol Willing (Yury Selivanov) in branch 'master':
   bpo-38248: Fix inconsistent immediate asyncio.Task cancellation (GH-16330)
   edad4d8
4. willingc commented on Sep 25, 2019

   

   willingc

   on Sep 25, 2019

   Contributor

   More actions

   New changeset 16cec13 by Carol Willing (Miss Islington (bot)) in branch '3.8':
   bpo-38248: Fix inconsistent immediate asyncio.Task cancellation (GH-16330) (GH-16383)
   16cec13
5. vstinner commented on Sep 30, 2019

   

   vstinner

   on Sep 30, 2019

   Member

   More actions

   New changeset efe74b6 by Victor Stinner in branch 'master':
   bpo-38321: Fix _asynciomodule.c compiler warning (GH-16493)
   efe74b6
6. vstinner commented on Oct 1, 2019

   

   vstinner

   on Oct 1, 2019

   Member

   More actions

   New changeset bfe1f74 by Victor Stinner in branch '3.8':
   [3.8] bpo-3832: Fix compiler warnings (GH-16518)
   bfe1f74
7. vstinner commented on Oct 1, 2019

   

   vstinner

   on Oct 1, 2019

   Member

   More actions

   Can this issue be closed now?
8. 1st1

   closed this as completedon Oct 1, 2019
9. ezio-melotti

   transferred this issue fromon Apr 10, 2022