Skip to content

[3.13] gh-128552: fix refcycles in eager task creation (#128553) #128585

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion Lib/asyncio/base_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,12 @@ def create_task(self, coro, *, name=None, context=None):

task.set_name(name)

return task
try:
return task
finally:
# gh-128552: prevent a refcycle of
# task.exception().__traceback__->BaseEventLoop.create_task->task
del task

def set_task_factory(self, factory):
"""Set a task factory that will be used by loop.create_task().
Expand Down
7 changes: 6 additions & 1 deletion Lib/asyncio/taskgroups.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,12 @@ def create_task(self, coro, *, name=None, context=None):
else:
self._tasks.add(task)
task.add_done_callback(self._on_task_done)
return task
try:
return task
finally:
# gh-128552: prevent a refcycle of
# task.exception().__traceback__->TaskGroup.create_task->task
del task

# Since Python 3.8 Tasks propagate all exceptions correctly,
# except for KeyboardInterrupt and SystemExit which are
Expand Down
63 changes: 59 additions & 4 deletions Lib/test/test_asyncio/test_taskgroups.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Adapted with permission from the EdgeDB project;
# license: PSFL.

import weakref
import sys
import gc
import asyncio
import contextvars
Expand Down Expand Up @@ -28,7 +30,25 @@ def get_error_types(eg):
return {type(exc) for exc in eg.exceptions}


class TestTaskGroup(unittest.IsolatedAsyncioTestCase):
def set_gc_state(enabled):
was_enabled = gc.isenabled()
if enabled:
gc.enable()
else:
gc.disable()
return was_enabled


@contextlib.contextmanager
def disable_gc():
was_enabled = set_gc_state(enabled=False)
try:
yield
finally:
set_gc_state(enabled=was_enabled)


class BaseTestTaskGroup:

async def test_taskgroup_01(self):

Expand Down Expand Up @@ -822,15 +842,15 @@ async def test_taskgroup_without_parent_task(self):
with self.assertRaisesRegex(RuntimeError, "has not been entered"):
tg.create_task(coro)

def test_coro_closed_when_tg_closed(self):
async def test_coro_closed_when_tg_closed(self):
async def run_coro_after_tg_closes():
async with taskgroups.TaskGroup() as tg:
pass
coro = asyncio.sleep(0)
with self.assertRaisesRegex(RuntimeError, "is finished"):
tg.create_task(coro)
loop = asyncio.get_event_loop()
loop.run_until_complete(run_coro_after_tg_closes())

await run_coro_after_tg_closes()

async def test_cancelling_level_preserved(self):
async def raise_after(t, e):
Expand Down Expand Up @@ -955,6 +975,30 @@ async def coro_fn():
self.assertIsInstance(exc, _Done)
self.assertListEqual(gc.get_referrers(exc), [])


async def test_exception_refcycles_parent_task_wr(self):
"""Test that TaskGroup deletes self._parent_task and create_task() deletes task"""
tg = asyncio.TaskGroup()
exc = None

class _Done(Exception):
pass

async def coro_fn():
async with tg:
raise _Done

with disable_gc():
try:
async with asyncio.TaskGroup() as tg2:
task_wr = weakref.ref(tg2.create_task(coro_fn()))
except* _Done as excs:
exc = excs.exceptions[0].exceptions[0]

self.assertIsNone(task_wr())
self.assertIsInstance(exc, _Done)
self.assertListEqual(gc.get_referrers(exc), [])

async def test_exception_refcycles_propagate_cancellation_error(self):
"""Test that TaskGroup deletes propagate_cancellation_error"""
tg = asyncio.TaskGroup()
Expand Down Expand Up @@ -988,5 +1032,16 @@ class MyKeyboardInterrupt(KeyboardInterrupt):
self.assertListEqual(gc.get_referrers(exc), [])


class TestTaskGroup(BaseTestTaskGroup, unittest.IsolatedAsyncioTestCase):
loop_factory = asyncio.EventLoop

class TestEagerTaskTaskGroup(BaseTestTaskGroup, unittest.IsolatedAsyncioTestCase):
@staticmethod
def loop_factory():
loop = asyncio.EventLoop()
loop.set_task_factory(asyncio.eager_task_factory)
return loop


if __name__ == "__main__":
unittest.main()
Original file line number Diff line number Diff line change
@@ -0,0 +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.
Loading