asyncio.CancelledError should contain more information on cancellations

[Dreamsorcerer]

BPO	45098
Nosy	@asvetlov, @1st1, @Dreamsorcerer

^{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:

assignee = None
closed_at = <Date 2022-03-23.23:18:35.957>
created_at = <Date 2021-09-04.13:17:43.301>
labels = ['3.11', 'type-bug', '3.9', '3.10', 'expert-asyncio']
title = 'asyncio.CancelledError should contain more information on cancellations'
updated_at = <Date 2022-03-23.23:18:35.957>
user = 'https://github.com/Dreamsorcerer'

bugs.python.org fields:

activity = <Date 2022-03-23.23:18:35.957>
actor = 'asvetlov'
assignee = 'none'
closed = True
closed_date = <Date 2022-03-23.23:18:35.957>
closer = 'asvetlov'
components = ['asyncio']
creation = <Date 2021-09-04.13:17:43.301>
creator = 'dreamsorcerer'
dependencies = []
files = []
hgrepos = []
issue_num = 45098
keywords = []
message_count = 4.0
messages = ['401045', '413394', '413549', '415915']
nosy_count = 3.0
nosy_names = ['asvetlov', 'yselivanov', 'dreamsorcerer']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue45098'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

[Dreamsorcerer]

There are awkward edge cases caused by race conditions when cancelling tasks which make it impossible to reliably cancel a task.

For example, in the async-timeout library there appears to be no way to avoid suppressing an explicit t.cancel() if that cancellation occurs immediately after the timeout.

In the alternative case where a cancellation happens immediately before the timeout, the solutions feel dependant on the internal details of how asynico.Task works and could easily break if the behaviour is tweaked in some way.

What we really need to know is how many times a task was cancelled as a cause of the CancelledError and ideally were the cancellations caused by us.

The solution I'd like to propose is that the args on the exception contain all the messages of every cancel() call leading up to that exception, rather than just the first one.

e.g. In these race conditions e.args would look like (None, SENTINEL), where SENTINEL was sent in our own cancellations. From this we can see that the task was cancelled twice and only one was caused by us, therefore we don't want to suppress the CancelledError.

For more details to fully understand the problem:
aio-libs-abandoned/async-timeout#230
aio-libs-abandoned/async-timeout#229 (comment)
aio-libs-abandoned/async-timeout#237

[asvetlov]

After TaskGroup merging, the second cancel() call returns False and doesn't initiate the actual cancellation if it was scheduled by the first cancel() call.

I believe it "fixes" async-timeout: the first canceller wins.

[Dreamsorcerer]

I think there's still a problem, in that the user still expects a task to be cancelled in the example previously: aio-libs-abandoned/async-timeout#229 (comment)

If we encounter the race condition where the timeout cancels the task and then the user cancels the task, then we still have the case that async-timeout swallows the cancellation and the task will run forever. This would basically require the user to check everytime they want to cancel the task, with something awkward like:

while not task.cancel() and not task.cancelled():
    await asyncio.sleep(0)

I think this change is still necessary, but rather than adding multiple values to e.args, we can use the new ExceptionGroup to raise multiple CancelledErrors. So, each call of task.cancel() will create a new CancelledError, and then all of those CancelledErrors will get raised together.

For async-timeout, we can then just catch the CancelledError with our sentinel and raise a TimeoutError, while reraising any other CancelledErrors.

[asvetlov]

bpo-46829 deprecates cancellation messages.
bpo-46771 implements timeout context manager based on previously added cancellation counter and task.uncancel()

I think this issue should be closed.