bpo-46771: Implement task cancel requests

[Tinche]

Keep track of requested task cancellations as an int, instead of a bool.

https://bugs.python.org/issue46771

[gvanrossum]

So this needs to be reviewed carefully by @agronholm and @Tinche -- not so much regarding "code review" but regarding "does this address our concerns, does it fix the edge cases, can we implement Trio-like behavior in 3rd party libraries when we need it". I'd also like @cjerdonek to think about this.

We also, separately, need to debate whether the lines

        if self._num_cancels_requested > 1:
            return False

should stay in or not. IIRC @agronholm has claimed that this (or the previous incarnation) broke several AnyIO tests.

There are two aspects that would change if we removed these two lines:

1. The cancel message. See https://bugs.python.org/issue46829. In 3.10, after t.cancel("first"); t.cancel("2nd") the cancel message ended up being set to "2nd"; with 3.11 main it ends up "first". If we remove these two lines it would become "2nd" again.
2. How many CancelledError exceptions will the task see? In 3.10, the task may see multiple cancellations -- but it may not. In particular, if the second cancel() call finds the must-cancel flag already set, only one cancellation will be delivered. However, if the first cancellation is delivered, and the task catches the CancelledError and then uses await to do some cleanup, in 3.10 (and again if we remove those two lines) that cleanup await will be cancelled; but if we keep those two lines, that await will be shielded.

[gvanrossum]

The more I think about it, the more I believe we should keep the two lines in cancel(), so I am merging this as-is. Thanks @Tinche. (Your first PR merged into CPython, I believe -- congrats!)

[asvetlov]

Is the cancelling() the best name for returning the number or cancel requests?
It was good for bool flag, a counter needs better name IMHO.

[gvanrossum]

Hm, do you have a suggestion? I was thinking that it's still usable as a "truthy" value. Usually when you're interested in the cancel count you should call uncancel() and use its return value.

[asvetlov]

I have no good answer.
cancel_requests_count() describes exact meaning but the name is too long.
cancelling() can return bool but it looks like procrastination: why return less info than we really have.
The third option is dropping cancelling() method entirely, it is not required by the current asyncio code.
We can consider adding the method later on-demand.

[asvetlov]

Note: .cancelling() doesn't exist in Python 3.10

[gvanrossum]

It's used in TaskGroup currently, so if we were to delete it we'd have to rewrite the code there in this same PR. It's also used by Task.__repr__() (via _task_repr_info()). But of those will work even though we upgraded it from a bool to an int. I'd say let's keep it for now, if we regret it we can rename or remove it before beta 1.

[asvetlov]

Deal!

[Tinche]

Great! Let me take another look if any of the method comments need rewrites.

As for cancelling(), while thinking about this PR I came to the same conclusion as you folks (that it's not needed) but I left it in. In the process I rewrote the TaskGroup very slightly to not use it (and potentially cancel its children only once). We're leaving cancelling(), but would this TaskGroup patch also be interesting?

[gvanrossum]

Yes, I am very much interested in your TaskGroup PR. There are a couple of things there that could be improved:

• It doesn't always preserve the cancel message (since it does propagate_cancellation_error = et instead of propagate_cancellation_error = exc -- I don't know why not, could be a carry-over from the pre-3.9 days when it didn't matter).
• There's weirdness around self._base_error. It seems that if __aexit__() receives a CancelledError (which is a base error) then that will always be raised (L124) before we get to raise propagate_cancellation_error (L130)?
• We could, if we wanted, distinguish between a child task being cancelled by TaskGroup._abort()` and it being cancelled for some other reason. Currently, if it's the latter, the CancellationError from the child task is swallowed and treated as if it exited successfully.

There's also some weirdness around self._on_completed_fut. It is set to None twice (once in the while loop and once after the loop has finished). But this is not related to cancellation (and the issue is benign).

[gvanrossum]

@Tinche I forgot to mention: I'm waiting for you before I land it. If you don't mind I'd rather see the TaskGroup changes in a separate PR (since this PR is complete as-is and the TaskGroup improvements should probably be tagged with bpo-46752).

[Tinche]

@gvanrossum Roger, task group will be a different PR. I tweaked the docstrings, please merge if it's good.

[gvanrossum]

Great! Will merge now (for real).

[gvanrossum]

Oh, you have to re-run clinic (make clinic) and commit and push the resulting changes. (Done)

[gvanrossum]

Congrats @Tinche on your first merged Python PR! Thanks for your patience. I hope this will ease the pain for Quattro.

[Tinche]

Thank you kindly. I will get started on a TaskGroup PR soon.