achimnol
May 1, 2023, 2:17am
3
opened 07:50PM - 12 Apr 23 UTC
type-feature
topic-asyncio
# Feature or enhancement
I propose adding a function to `asyncio` to cancel a… task and then safely wait for the cancellation to complete.
The main difficulty with this is deciding whether to swallow or re-raise the `CancelledError` once we catch one. It we call the task that we're waiting on "dependency", and the task doing the waiting "controller", then there are two distinct possibilities:
1. The "dependency" has finished its cleanup, passed its own `CancelledError` all the way up its own stack, and then further up through the `await` in the "controller". In this case, the `CancelledError` should be swallowed, and the "controller" can continue its work normally.
2. The "controller" itself was cancelled, while being on the `await`. In this case, the `CancelledError` is the signal to cancel the "controller" itself, and should be either re-raised further up, or its swallowing should be accompanied by a call to `uncancel`.
[The documentation for `asyncio.Task.cancel`](https://docs.python.org/3/library/asyncio-task.html#asyncio.Task.cancel), in fact, does not make this decision correctly, and thus would be a bug. Copying the example, and changing the names to match this issue's terminology:
```py
async def dependency():
print('dependency(): start')
try:
await asyncio.sleep(3600) # a long or infinite loop
except asyncio.CancelledError:
print('dependency(): cancel')
raise
finally:
print('dependency(): cleanup')
async def controller():
dependency_task = asyncio.create_task(dependency())
await asyncio.sleep(1)
dependency_task.cancel()
try:
await dependency_task # controller itself may be cancelled at this moment
except asyncio.CancelledError:
print("controller(): dependency is cancelled now")
# CancelledError swallowed unconditionally
```
If I'm not missing anything, the correct procedure would look like this:
```py
async def controller():
dependency_task = asyncio.create_task(dependency())
await asyncio.sleep(1)
dependency_task.cancel()
try:
await dependency_task
except asyncio.CancelledError:
print("controller(): dependency is cancelled now")
if asyncio.current_task().cancelling() > 0:
raise
```
Thus, I propose to make these changes:
1. Introduce a function to `asyncio` or `Task`:
```py
async def cancel_and_wait(task, msg=None):
task.cancel(msg)
try:
await task
except asyncio.CancelledError:
if asyncio.current_task().cancelling() == 0:
raise
else:
return # this is the only non-exceptional return
else:
raise RuntimeError("Cancelled task did not end with an exception")
```
Having a specialized function would reduce the possibility of someone making this mistake in their code (like the author of the example probably did :) ), and allow the implementation to be changed or improved in the future. One such possible enhancement, for example, could be adding a `repeat` parameter to instruct the function, in case the task uncancels itself, to keep cancelling it again in a loop.
2. In the documentation for `asyncio`, add a warning for this kind of mistake, in the "Task Cancellation" section or in the description of `asyncio.Task.cancel`.
3. Change the code example for `asyncio.Task.cancel` to account for cancellation of `main`. I know that, in this specific snippet, it is impossible for `main` itself to be cancelled; but a developer unsuspecting of this issue may copy this example into a situation where the controller is indeed cancellable, and end up with a bug in their code.
I think your argument exactly matches with the above GitHub issue.
async def cancel_and_wait(task, msg=None):
task.cancel(msg)
try:
await task
except asyncio.CancelledError:
if asyncio.current_task().cancelling() == 0:
raise
else:
return # this is the only non-exceptional return
else:
raise RuntimeError("Cancelled task did not end with an exception")
This seems to be a pretty much repeating pattern along with structured concurrency.
2 Likes