Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit bc21454

Browse filesBrowse files
graingertkumaraditya303
graingert
and
kumaraditya303
authored
[3.12] gh-128588: fix refcycles in eager task creation and remove eager tasks optimization that missed and introduced incorrect cancellations (#129063) (#128586)
Co-authored-by: Kumar Aditya <kumaraditya@python.org>
1 parent 23cb53a commit bc21454
Copy full SHA for bc21454

File tree

Expand file treeCollapse file tree

5 files changed

+141
-10
lines changed
Filter options
Expand file treeCollapse file tree

5 files changed

+141
-10
lines changed

‎Lib/asyncio/base_events.py

Copy file name to clipboardExpand all lines: Lib/asyncio/base_events.py
+6-1Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,12 @@ def create_task(self, coro, *, name=None, context=None):
466466

467467
tasks._set_task_name(task, name)
468468

469-
return task
469+
try:
470+
return task
471+
finally:
472+
# gh-128552: prevent a refcycle of
473+
# task.exception().__traceback__->BaseEventLoop.create_task->task
474+
del task
470475

471476
def set_task_factory(self, factory):
472477
"""Set a task factory that will be used by loop.create_task().

‎Lib/asyncio/taskgroups.py

Copy file name to clipboardExpand all lines: Lib/asyncio/taskgroups.py
+13-8Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -185,15 +185,20 @@ def create_task(self, coro, *, name=None, context=None):
185185
else:
186186
task = self._loop.create_task(coro, context=context)
187187
tasks._set_task_name(task, name)
188-
# optimization: Immediately call the done callback if the task is
188+
189+
# Always schedule the done callback even if the task is
189190
# already done (e.g. if the coro was able to complete eagerly),
190-
# and skip scheduling a done callback
191-
if task.done():
192-
self._on_task_done(task)
193-
else:
194-
self._tasks.add(task)
195-
task.add_done_callback(self._on_task_done)
196-
return task
191+
# otherwise if the task completes with an exception then it will cancel
192+
# the current task too early. gh-128550, gh-128588
193+
194+
self._tasks.add(task)
195+
task.add_done_callback(self._on_task_done)
196+
try:
197+
return task
198+
finally:
199+
# gh-128552: prevent a refcycle of
200+
# task.exception().__traceback__->TaskGroup.create_task->task
201+
del task
197202

198203
# Since Python 3.8 Tasks propagate all exceptions correctly,
199204
# except for KeyboardInterrupt and SystemExit which are

‎Lib/test/test_asyncio/test_taskgroups.py

Copy file name to clipboardExpand all lines: Lib/test/test_asyncio/test_taskgroups.py
+120-1Lines changed: 120 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
# Adapted with permission from the EdgeDB project;
22
# license: PSFL.
33

4+
import weakref
5+
import sys
46
import gc
57
import asyncio
68
import contextvars
@@ -27,7 +29,25 @@ def get_error_types(eg):
2729
return {type(exc) for exc in eg.exceptions}
2830

2931

30-
class TestTaskGroup(unittest.IsolatedAsyncioTestCase):
32+
def set_gc_state(enabled):
33+
was_enabled = gc.isenabled()
34+
if enabled:
35+
gc.enable()
36+
else:
37+
gc.disable()
38+
return was_enabled
39+
40+
41+
@contextlib.contextmanager
42+
def disable_gc():
43+
was_enabled = set_gc_state(enabled=False)
44+
try:
45+
yield
46+
finally:
47+
set_gc_state(enabled=was_enabled)
48+
49+
50+
class BaseTestTaskGroup:
3151

3252
async def test_taskgroup_01(self):
3353

@@ -880,6 +900,30 @@ async def coro_fn():
880900
self.assertIsInstance(exc, _Done)
881901
self.assertListEqual(gc.get_referrers(exc), [])
882902

903+
904+
async def test_exception_refcycles_parent_task_wr(self):
905+
"""Test that TaskGroup deletes self._parent_task and create_task() deletes task"""
906+
tg = asyncio.TaskGroup()
907+
exc = None
908+
909+
class _Done(Exception):
910+
pass
911+
912+
async def coro_fn():
913+
async with tg:
914+
raise _Done
915+
916+
with disable_gc():
917+
try:
918+
async with asyncio.TaskGroup() as tg2:
919+
task_wr = weakref.ref(tg2.create_task(coro_fn()))
920+
except* _Done as excs:
921+
exc = excs.exceptions[0].exceptions[0]
922+
923+
self.assertIsNone(task_wr())
924+
self.assertIsInstance(exc, _Done)
925+
self.assertListEqual(gc.get_referrers(exc), [])
926+
883927
async def test_exception_refcycles_propagate_cancellation_error(self):
884928
"""Test that TaskGroup deletes propagate_cancellation_error"""
885929
tg = asyncio.TaskGroup()
@@ -912,6 +956,81 @@ class MyKeyboardInterrupt(KeyboardInterrupt):
912956
self.assertIsNotNone(exc)
913957
self.assertListEqual(gc.get_referrers(exc), [])
914958

959+
async def test_cancels_task_if_created_during_creation(self):
960+
# regression test for gh-128550
961+
ran = False
962+
class MyError(Exception):
963+
pass
964+
965+
exc = None
966+
try:
967+
async with asyncio.TaskGroup() as tg:
968+
async def third_task():
969+
raise MyError("third task failed")
970+
971+
async def second_task():
972+
nonlocal ran
973+
tg.create_task(third_task())
974+
with self.assertRaises(asyncio.CancelledError):
975+
await asyncio.sleep(0) # eager tasks cancel here
976+
await asyncio.sleep(0) # lazy tasks cancel here
977+
ran = True
978+
979+
tg.create_task(second_task())
980+
except* MyError as excs:
981+
exc = excs.exceptions[0]
982+
983+
self.assertTrue(ran)
984+
self.assertIsInstance(exc, MyError)
985+
986+
async def test_cancellation_does_not_leak_out_of_tg(self):
987+
class MyError(Exception):
988+
pass
989+
990+
async def throw_error():
991+
raise MyError
992+
993+
try:
994+
async with asyncio.TaskGroup() as tg:
995+
tg.create_task(throw_error())
996+
except* MyError:
997+
pass
998+
else:
999+
self.fail("should have raised one MyError in group")
1000+
1001+
# if this test fails this current task will be cancelled
1002+
# outside the task group and inside unittest internals
1003+
# we yield to the event loop with sleep(0) so that
1004+
# cancellation happens here and error is more understandable
1005+
await asyncio.sleep(0)
1006+
1007+
1008+
if sys.platform == "win32":
1009+
EventLoop = asyncio.ProactorEventLoop
1010+
else:
1011+
EventLoop = asyncio.SelectorEventLoop
1012+
1013+
1014+
class IsolatedAsyncioTestCase(unittest.IsolatedAsyncioTestCase):
1015+
loop_factory = None
1016+
1017+
def _setupAsyncioRunner(self):
1018+
assert self._asyncioRunner is None, 'asyncio runner is already initialized'
1019+
runner = asyncio.Runner(debug=True, loop_factory=self.loop_factory)
1020+
self._asyncioRunner = runner
1021+
1022+
1023+
class TestTaskGroup(BaseTestTaskGroup, IsolatedAsyncioTestCase):
1024+
loop_factory = EventLoop
1025+
1026+
1027+
class TestEagerTaskTaskGroup(BaseTestTaskGroup, IsolatedAsyncioTestCase):
1028+
@staticmethod
1029+
def loop_factory():
1030+
loop = EventLoop()
1031+
loop.set_task_factory(asyncio.eager_task_factory)
1032+
return loop
1033+
9151034

9161035
if __name__ == "__main__":
9171036
unittest.main()

‎Misc/NEWS.d/next/Library/2025-01-06-18-41-08.gh-issue-128552.fV-f8j.rst

Copy file name to clipboard
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix cyclic garbage introduced by :meth:`asyncio.loop.create_task` and :meth:`asyncio.TaskGroup.create_task` holding a reference to the created task if it is eager.

‎Misc/NEWS.d/next/Library/2025-01-20-13-12-39.gh-issue-128550.AJ5TOL.rst

Copy file name to clipboard
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Removed an incorrect optimization relating to eager tasks in :class:`asyncio.TaskGroup` that resulted in cancellations being missed.

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.