From: Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com>
Date: Mon, 20 Jan 2025 17:36:50 +0000 (+0100)
Subject: [3.13] gh-128588: gh-128550: remove eager tasks optimization that missed and introduc... 
X-Git-Tag: v3.13.2~73
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=a1c48a750cc52332b0ed16ea8ffa07c18eaee201;p=thirdparty%2FPython%2Fcpython.git

[3.13] gh-128588: gh-128550: remove eager tasks optimization that missed and introduced incorrect cancellations (GH-129063) (#129089)

gh-128588: gh-128550: remove eager tasks optimization that missed and introduced incorrect cancellations (GH-129063)
(cherry picked from commit ed6934e71e55d398df8263f4697f58e4a3815f69)

Co-authored-by: Thomas Grainger <tagrain@gmail.com>
Co-authored-by: Kumar Aditya <kumaraditya@python.org>
---

diff --git a/Lib/asyncio/taskgroups.py b/Lib/asyncio/taskgroups.py
index 8af199d6dcc4..8fda6c8d55e1 100644
--- a/Lib/asyncio/taskgroups.py
+++ b/Lib/asyncio/taskgroups.py
@@ -197,14 +197,12 @@ class TaskGroup:
         else:
             task = self._loop.create_task(coro, name=name, context=context)
 
-        # optimization: Immediately call the done callback if the task is
+        # Always schedule the done callback even if the task is
         # already done (e.g. if the coro was able to complete eagerly),
-        # and skip scheduling a done callback
-        if task.done():
-            self._on_task_done(task)
-        else:
-            self._tasks.add(task)
-            task.add_done_callback(self._on_task_done)
+        # otherwise if the task completes with an exception then it will cancel
+        # the current task too early. gh-128550, gh-128588
+        self._tasks.add(task)
+        task.add_done_callback(self._on_task_done)
         try:
             return task
         finally:
diff --git a/Lib/test/test_asyncio/test_taskgroups.py b/Lib/test/test_asyncio/test_taskgroups.py
index e52061aac923..ad61cb46c7c0 100644
--- a/Lib/test/test_asyncio/test_taskgroups.py
+++ b/Lib/test/test_asyncio/test_taskgroups.py
@@ -1032,6 +1032,56 @@ class BaseTestTaskGroup:
         self.assertListEqual(gc.get_referrers(exc), [])
 
 
+    async def test_cancels_task_if_created_during_creation(self):
+        # regression test for gh-128550
+        ran = False
+        class MyError(Exception):
+            pass
+
+        exc = None
+        try:
+            async with asyncio.TaskGroup() as tg:
+                async def third_task():
+                    raise MyError("third task failed")
+
+                async def second_task():
+                    nonlocal ran
+                    tg.create_task(third_task())
+                    with self.assertRaises(asyncio.CancelledError):
+                        await asyncio.sleep(0)  # eager tasks cancel here
+                        await asyncio.sleep(0)  # lazy tasks cancel here
+                    ran = True
+
+                tg.create_task(second_task())
+        except* MyError as excs:
+            exc = excs.exceptions[0]
+
+        self.assertTrue(ran)
+        self.assertIsInstance(exc, MyError)
+
+
+    async def test_cancellation_does_not_leak_out_of_tg(self):
+        class MyError(Exception):
+            pass
+
+        async def throw_error():
+            raise MyError
+
+        try:
+            async with asyncio.TaskGroup() as tg:
+                tg.create_task(throw_error())
+        except* MyError:
+            pass
+        else:
+            self.fail("should have raised one MyError in group")
+
+        # if this test fails this current task will be cancelled
+        # outside the task group and inside unittest internals
+        # we yield to the event loop with sleep(0) so that
+        # cancellation happens here and error is more understandable
+        await asyncio.sleep(0)
+
+
 class TestTaskGroup(BaseTestTaskGroup, unittest.IsolatedAsyncioTestCase):
     loop_factory = asyncio.EventLoop
 
diff --git a/Misc/NEWS.d/next/Library/2025-01-20-13-12-39.gh-issue-128550.AJ5TOL.rst b/Misc/NEWS.d/next/Library/2025-01-20-13-12-39.gh-issue-128550.AJ5TOL.rst
new file mode 100644
index 000000000000..f59feac795ea
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2025-01-20-13-12-39.gh-issue-128550.AJ5TOL.rst
@@ -0,0 +1 @@
+Removed an incorrect optimization relating to eager tasks in :class:`asyncio.TaskGroup` that resulted in cancellations being missed.