Skip to content

Commit

Permalink
gh-124958: fix asyncio.TaskGroup and _PyFuture refcycles
Browse files Browse the repository at this point in the history
  • Loading branch information
graingert committed Oct 4, 2024
1 parent 5e9e506 commit 1a4f2b5
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 9 deletions.
6 changes: 2 additions & 4 deletions Lib/asyncio/futures.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,7 @@ def result(self):
the future is done and has an exception set, this exception is raised.
"""
if self._state == _CANCELLED:
exc = self._make_cancelled_error()
raise exc
raise self._make_cancelled_error()
if self._state != _FINISHED:
raise exceptions.InvalidStateError('Result is not ready.')
self.__log_traceback = False
Expand All @@ -208,8 +207,7 @@ def exception(self):
InvalidStateError.
"""
if self._state == _CANCELLED:
exc = self._make_cancelled_error()
raise exc
raise self._make_cancelled_error()
if self._state != _FINISHED:
raise exceptions.InvalidStateError('Exception is not set.')
self.__log_traceback = False
Expand Down
17 changes: 13 additions & 4 deletions Lib/asyncio/taskgroups.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ async def __aenter__(self):
return self

async def __aexit__(self, et, exc, tb):
try:
self._exiting = True

if (exc is not None and
Expand Down Expand Up @@ -146,14 +147,22 @@ async def __aexit__(self, et, exc, tb):
if self._parent_task.cancelling():
self._parent_task.uncancel()
self._parent_task.cancel()
raise BaseExceptionGroup(
'unhandled errors in a TaskGroup',
self._errors,
) from None
finally:
# Exceptions are heavy objects that can have object
# cycles (bad for GC); let's not keep a reference to
# a bunch of them.
try:
me = BaseExceptionGroup('unhandled errors in a TaskGroup', self._errors)
raise me from None
finally:
propagate_cancellation_error = None
self._parent_task = None
self._errors = None
self._base_error = None
et = None
exc = None
tb = None


def create_task(self, coro, *, name=None, context=None):
"""Create a new task in this group and return it.
Expand Down
22 changes: 22 additions & 0 deletions Lib/test/test_asyncio/test_futures.py
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,28 @@ def __del__(self):
fut = self._new_future(loop=self.loop)
fut.set_result(Evil())

def test_future_cancelled_result_refcycles(self):
f = self._new_future(loop=self.loop)
f.cancel()
exc = None
try:
f.result()
except asyncio.CancelledError as e:
exc = e
self.assertIsNotNone(exc)
self.assertListEqual(gc.get_referrers(exc), [])

def test_future_cancelled_exception_refcycles(self):
f = self._new_future(loop=self.loop)
f.cancel()
exc = None
try:
f.exception()
except asyncio.CancelledError as e:
exc = e
self.assertIsNotNone(exc)
self.assertListEqual(gc.get_referrers(exc), [])


@unittest.skipUnless(hasattr(futures, '_CFuture'),
'requires the C _asyncio module')
Expand Down
95 changes: 94 additions & 1 deletion Lib/test/test_asyncio/test_taskgroups.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Adapted with permission from the EdgeDB project;
# license: PSFL.


import gc
import asyncio
import contextvars
import contextlib
Expand All @@ -11,6 +11,10 @@

from test.test_asyncio.utils import await_without_task

if False:
asyncio.Task = asyncio.tasks._CTask = asyncio.tasks.Task = asyncio.tasks._PyTask
asyncio.Future = asyncio.futures._CFuture = asyncio.futures.Future = asyncio.futures._PyFuture


# To prevent a warning "test altered the execution environment"
def tearDownModule():
Expand Down Expand Up @@ -899,6 +903,95 @@ async def outer():

await outer()

async def test_exception_refcycles_direct(self):
"""Test that TaskGroup doesn't keep a reference to the raised ExceptionGroup"""
tg = asyncio.TaskGroup()
exc = None

class _Done(Exception):
pass

try:
async with tg:
raise _Done
except ExceptionGroup as e:
exc = e

self.assertIsNotNone(exc)
self.assertListEqual(gc.get_referrers(exc), [])


async def test_exception_refcycles_errors(self):
"""Test that TaskGroup deletes self._errors, and __aexit__ args"""
tg = asyncio.TaskGroup()
exc = None

class _Done(Exception):
pass

try:
async with tg:
raise _Done
except* _Done as excs:
exc = excs.exceptions[0]

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


async def test_exception_refcycles_parent_task(self):
"""Test that TaskGroup deletes self._parent_task"""
tg = asyncio.TaskGroup()
exc = None

class _Done(Exception):
pass

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

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

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()
exc = None

try:
async with asyncio.timeout(-1):
async with tg:
await asyncio.sleep(0)
except TimeoutError as e:
exc = e.__cause__

self.assertIsInstance(exc, asyncio.CancelledError)
self.assertListEqual(gc.get_referrers(exc), [])

async def test_exception_refcycles_base_error(self):
"""Test that TaskGroup deletes self._base_error"""
class MyKeyboardInterrupt(KeyboardInterrupt):
pass

tg = asyncio.TaskGroup()
exc = None

try:
async with tg:
raise MyKeyboardInterrupt
except MyKeyboardInterrupt as e:
exc = e

self.assertIsNotNone(exc)
self.assertListEqual(gc.get_referrers(exc), [])


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

0 comments on commit 1a4f2b5

Please sign in to comment.