The design follows EdgeDB monkey-patch.

Speaking about public API, I can imagine an alternative design:

1. No publicproperties are added,Task.cancelling() (along withFuture.cancelling()) method is added. The boolean internal flag exists, sure -- but it is a private variable that is not exposed, except for__repr__ maybe.
2. The method returnsTrue if a cancellation was requested (__cancel_requested__ == True from initial implementation),False otherwise.
3. The flag is resetjust before raisingCancelledError inside a task. The reset doesn't change TaskGroup implementation logic but can help withstrange designed tasks that swallowCancelledError. Sure, good-written code should avoid such situations but the design is still valid and allowed. The reset before task execution maybe look a little awkward but I have no better idea. The reset after the first__step() call is even worse, the task can take several iterations with multiple awaits for graceful cancellation.
4. TaskGroup has a singletask.__cancel_requested__ = True assignment, it can be avoided easily by replacingif not self._parent_task.__cancel_requested__: withif not self._parent_cancel_requested and not self._parent_task.__cancel_requested__:

I believe the proposed design is cleaner; it doesn't allow.__cancel_requested__ setting outside of Task's implementation code. Resetting the flag before raisingCancelledError allows handling cancellation ignorance by a task code.

What do you think?
I can prepare the alternative PR to compare if the idea seems viable.

@asvetlov I apologize, I didn't see your large comment with the alternative design before my own comment. This stuff is excruciating to understand (for me anyways, since I didn't write EdgeDb's TaskGroup :-) so I have to experiment some more.

I will see if I can get the new TaskGroup to work with that, I will get back to you.

@asvetlov I cannot get this to work. Here's what I did.

(1) ImplementTask.cancelling() as follows:

diff --git a/Lib/asyncio/tasks.py b/Lib/asyncio/tasks.pyindex 2bee5c050d..6f726b2ff8 100644--- a/Lib/asyncio/tasks.py+++ b/Lib/asyncio/tasks.py@@ -105,6 +105,7 @@ def __init__(self, coro, *, loop=None, name=None):         else:             self._name = str(name) +        self._cancelling = False         self._must_cancel = False         self._fut_waiter = None         self._coro = coro@@ -178,6 +179,9 @@ def print_stack(self, *, limit=None, file=None):         """         return base_tasks._task_print_stack(self, limit, file) +    def cancelling(self):+        return self._cancelling+     def cancel(self, msg=None):         """Request that this task cancel itself. @@ -201,6 +205,9 @@ def cancel(self, msg=None):         self._log_traceback = False         if self.done():             return False+        if self._cancelling:+            return False+        self._cancelling = True         if self._fut_waiter is not None:             if self._fut_waiter.cancel(msg=msg):                 # Leave self._fut_waiter; it may be a Task that@@ -231,6 +238,7 @@ def __step(self, exc=None):                 # don't have `__iter__` and `__next__` methods.                 result = coro.send(None)             else:+                self._cancelling = False                 result = coro.throw(exc)         except StopIteration as exc:             if self._must_cancel:

(2) In taskgroups.py: Disable task patching in taskgroups.py; delete the assignment (of False) to__cancel_requested__; replace the check for__cancel_requested__ per your suggestion (sort of).

diff --git a/Lib/asyncio/taskgroups.py b/Lib/asyncio/taskgroups.pyindex 4f946eb0e4..721115a2d8 100644--- a/Lib/asyncio/taskgroups.py+++ b/Lib/asyncio/taskgroups.py@@ -78,7 +78,6 @@ async def __aenter__(self):         if self._parent_task is None:             raise RuntimeError(                 f'TaskGroup {self!r} cannot determine the parent task')-        self._patch_task(self._parent_task)          return self @@ -95,7 +94,7 @@ async def __aexit__(self, et, exc, tb):             if self._parent_cancel_requested:                 # Only if we did request task to cancel ourselves                 # we mark it as no longer cancelled.-                self._parent_task.__cancel_requested__ = False+                pass             else:                 propagate_cancellation_error = et @@ -244,7 +243,7 @@ def _on_task_done(self, task):             return          self._abort()-        if not self._parent_task.__cancel_requested__:+        if not self._parent_cancel_requested and not self._parent_task.cancelling():             # If parent task *is not* being cancelled, it means that we want             # to manually cancel it to abort whatever is being run right now             # in the TaskGroup.  But we want to mark parent task as

(3) Disable the C accellerator at the top oftest_taskgroups.py:

diff --git a/Lib/test/test_asyncio/test_taskgroups.py b/Lib/test/test_asyncio/test_taskgroups.pyindex 8a01985c06..2d909dad80 100644--- a/Lib/test/test_asyncio/test_taskgroups.py+++ b/Lib/test/test_asyncio/test_taskgroups.py@@ -16,6 +16,9 @@ # limitations under the License. # +import sys+assert "_asyncio" not in sys.modules+sys.modules["_asyncio"] = None  # Use the pure Python version  import asyncio

Now when I run

./python.exe -m test test_asyncio.test_taskgroups -v

I get failures intest_taskgroup_{15,16}. In both cases I get an unexpectedExceptionGroup wrappingZeroDivisionError, where aCancelledError is expected.

Can you repro this? Can you explain or fix it?

I think I understand the cause of the failure now (not sure this will lead to a fix yet).

Test 15 has a try/except in the body of the async with TaskGroup() block, where in the try block it awaits a long sleep (which will be cancelled from the outside after 0.1 second), and the except catches CancelledError and then sleeps some more (0.5s) before re-raising it. During this second sleep a subtask crashes (0.3s into the test).

The desired behavior here is that the sleep in the except block isnot cancelled when the subtask crashes, and the sleep(0.5) is allowed to finish after which things reach__aexit__ with CancelledError.

(Test 16 is a variation where there's another task wrapping the whole business -- the outermost task is cancelled, and this is expected to be propagated into the nested task.)

Because the initial cancellation of the parent task happens from the outside, it is essential that its__cancel_requested__ flag remains set while the sleep(0.5) runs, so any flag that is reset when the coroutine is entered is not good enough, and a flag that is set only by the code in TaskGroup won't work either. We really need to remember that the parent task was cancelled across potentially many resumptions and suspensions of its coroutine. (Coincidentally this is also the thing that the Trio folks brought up in my original PR -- Yury solved it with this__cancel_requested__ behavior.)

I did find that none of the tests fail if I replace the lineself._parent_task.__cancel_requested__ = False withpass, so perhaps we do not actually need the ability to reset this flag; or maybe there's a missing test. However, the checkif self._parent_cancel_requestedis required; if I just always setpropagate_cancellation_error = et once we get to that point, some tests fail.

That's as far as I got. If we're going to design a public API for this, it would have to be a flag on tasks that is set when the task is cancelled, and which then stays set for the duration. We could add an API to reset the flag, which could be used by a task that catches cancellation and decides to ignore it (rather than just taking its time to clean up, like tests 15 and 16). Until the flag is cleared, further efforts to cancel the task are ignored.

@asvetlov I would like to make progress on this. Mind if I just incorporate my version into the TaskGroup PR? My proposal is the following:

• There's an internal flag,_cancelling, that has the same meaning as what you implemented (and EdgeDb's TaskGroup uses) as__cancel_requested__, but the public API is two methods,t.cancelling() that is checked and set byt.cancel(), andt.uncancel() that resets it.
• The internal flag is inaccessible in the_asyncio.Task implementation, you must use the public API.
• There is no such API or flag for Future (I don't see the need).

(@agronholm I think this may solve your problem too.)

@asvetlov I am closing this. I took this as a starting point for a public API that defines .cancelling() and .uncancel() methods (on Tasks only) inGH-31270.

Thank you very much for this -- without this I wouldn't have know where to start!

Thanks,@gvanrossum
I love.cancelling() /.uncancel(). The behavior is very clear!