asyncio.TaskGroup corrupts uncancel() stack when the parent task replaces the cancelled error

Bug report

import asyncio

class MyException(Exception):
    pass

async def async_fn():
    await asyncio.sleep(0)
    raise MyException

async def main():
    task = asyncio.current_task()

    try:
        async with asyncio.TaskGroup() as tg:
            tg.create_task(async_fn())
            try:
                await asyncio.sleep(1)
            except asyncio.CancelledError:
                pass
    except* MyException:
        print("done!")

    print(f"{task.cancelling()=} should be 0")

asyncio.run(main())

this is also quite easy to trigger in typical task group code:

import asyncio
import contextlib

class ConnectionClosedError(Exception):
    pass


async def poll_database(database):
    raise ConnectionClosedError


async def close_connection():
    raise ConnectionClosedError


@contextlib.asynccontextmanager
async def database():
    try:
        yield
    finally:
        await close_connection()


async def main():
    task = asyncio.current_task()

    try:
        async with asyncio.TaskGroup() as tg:
            async with database() as db:
                tg.create_task(poll_database(db))
                await asyncio.sleep(1)
    except* ConnectionClosedError:
        print("done!")

    print(f"{task.cancelling()=} should be 0")


asyncio.run(main())

@ambv

cc @ambv who helped me find this

@pablogsal

Tagging @pablogsal as potential release-blocker

Hello, sorry for my question but I have never seen a * after an except statement. I couldn't find an explanation on the web. What does it mean, please?

In your first example, if you replace pass with raise, you have the expected result.
May be this will help you ?

Hello, sorry for my question but I have never seen a * after an except statement. I couldn't find an explanation on the web. What does it mean, please?

Hi there - it's the new 3.11 ExceptionGroup feature https://peps.python.org/pep-0654/ it's required to handle an issue where multiple errors can happen concurrently. Eg a connection error canceles a group of concurrent operations, and the cancellation error leads to some teardown that also fails. In 3.10 library authors had to either choose an error to raise or wrap them in a list and raise a generic "MultiError"

@graingert

Hi @graingert, thank you for your response about except * feature.

About the issue, I read the documentation about 'Task groups' and IMHO there is a part about the two cases, in the second paragraph below the exemple:

... At this point, if the body of the async with statement is still active (__aexit__() hasn’t been called yet), the task directly containing the async with statement is also cancelled.

Do you think that your both examples match the situation described ?

@YvesDup

@YvesDup, you stopped your quote just before the sentence that describes why what Thomas reported is a bug:

The resulting asyncio.CancelledError will interrupt an await, but it will not bubble out of the containing async with statement.

In other words, after leaving async with we want print(f"{task.cancelling()=} should be 0").

Have you tried this on the edgedb's task group ¹ implementation from which the current one is derived from?

https://github.com/edgedb/edgedb/blob/master/edb/common/taskgroup.py ↩

@graingert

Alas, that doesn't fix the original bug report. But here's a variant that does:

diff --git a/Lib/asyncio/taskgroups.py b/Lib/asyncio/taskgroups.py
index 3ca65062ef..0dde87f1ca 100644
--- a/Lib/asyncio/taskgroups.py
+++ b/Lib/asyncio/taskgroups.py
@@ -61,14 +61,12 @@ async def __aexit__(self, et, exc, tb):
                 self._base_error is None):
             self._base_error = exc
 
-        if et is not None:
-            if et is exceptions.CancelledError:
-                if self._parent_cancel_requested and not self._parent_task.uncancel():
-                    # Do nothing, i.e. swallow the error.
-                    pass
-                else:
-                    propagate_cancellation_error = exc
+        if (self._parent_cancel_requested and
+                self._parent_task.uncancel() != 0 and
+                et is exceptions.CancelledError):
+            propagate_cancellation_error = exc
 
+        if et is not None:
             if not self._aborting:
                 # Our parent task is being cancelled:
                 #

I have other things to do, maybe @graingert and/or @kumaraditya303 can check my fix and turn it into a PR with the two examples as new tests?

Hm, there’s one case where the effect is different from the original: if a CancellationError occurs while _parent_cancel_request is not set. Then the original sets propagate_cancellation_error = exc but the new code does nothing. Does that matter? No tests fail.

That would affect the case where the parent task is cancelled externally — it should propagate the cancellation but with my code it will exit cleanly. This can be fixed and requires a separate test.

Oh, I think I understand why that doesn't matter. In this case we end up returning None from __aexit__() and that causes the interpreter to re-raise the original exception that was being handled. (In fact we may use this fact to simplify the code even further?)

pythonGH-95289: Always call uncancel() when required

GH-95289: Always call uncancel() when parent cancellation is requested (

pythonGH-95289: Always call uncancel() when parent cancellation is re…

GH-95289: Always call uncancel() when parent cancellation is requested (

Fixed by #95602

Navigation Menu

Repository navigation

asyncio.TaskGroup corrupts uncancel() stack when the parent task replaces the cancelled error #95289

Bug report

7 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

asyncio.TaskGroup corrupts uncancel() stack when the parent task replaces the cancelled error #95289

Description

Bug report

Activity

graingert commented on Jul 26, 2022

graingert commented on Jul 26, 2022

graingert commented on Jul 27, 2022

YvesDup commented on Jul 27, 2022

graingert commented on Jul 27, 2022

YvesDup commented on Jul 29, 2022

ambv commented on Jul 29, 2022

kumaraditya303 commented on Jul 29, 2022

Footnotes

7 remaining items

gvanrossum commented on Jul 30, 2022

gvanrossum commented on Jul 31, 2022

gvanrossum commented on Jul 31, 2022

gvanrossum commented on Aug 1, 2022

kumaraditya303 commented on Aug 4, 2022

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions