New submission from Nathaniel Smith <n...@pobox.com>:
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 :-)
----------
messages: 352964
nosy: asvetlov, njs, yselivanov
priority: normal
severity: normal
status: open
title: inconsistency in asyncio.Task between cancellation while running vs.
cancellation immediately after it finishes
_______________________________________
Python tracker <rep...@bugs.python.org>
<https://bugs.python.org/issue38248>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
