-
-
Notifications
You must be signed in to change notification settings - Fork 31.1k
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
inconsistency in asyncio.Task between cancellation while running vs. cancellation immediately after it finishes #82429
Comments
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 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 :-) |
Yeah, I think this is the solution we should do in 3.8. |
Can this issue be closed now? |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: