Skip to content
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

[3.13] gh-125472: Revert "gh-124958: fix asyncio.TaskGroup and _PyFuture refcycles (GH-12… (GH-125476) #125478

Closed
Closed
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
6 changes: 4 additions & 2 deletions Lib/asyncio/futures.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,8 @@ def result(self):
the future is done and has an exception set, this exception is raised.
"""
if self._state == _CANCELLED:
raise self._make_cancelled_error()
exc = self._make_cancelled_error()
raise exc
if self._state != _FINISHED:
raise exceptions.InvalidStateError('Result is not ready.')
self.__log_traceback = False
Expand All @@ -208,7 +209,8 @@ def exception(self):
InvalidStateError.
"""
if self._state == _CANCELLED:
raise self._make_cancelled_error()
exc = self._make_cancelled_error()
raise exc
if self._state != _FINISHED:
raise exceptions.InvalidStateError('Exception is not set.')
self.__log_traceback = False
Expand Down
41 changes: 9 additions & 32 deletions Lib/asyncio/taskgroups.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,20 +66,6 @@ async def __aenter__(self):
return self

async def __aexit__(self, et, exc, tb):
tb = None
try:
return await self._aexit(et, exc)
finally:
# Exceptions are heavy objects that can have object
# cycles (bad for GC); let's not keep a reference to
# a bunch of them. It would be nicer to use a try/finally
# in __aexit__ directly but that introduced some diff noise
self._parent_task = None
self._errors = None
self._base_error = None
exc = None

async def _aexit(self, et, exc):
self._exiting = True

if (exc is not None and
Expand Down Expand Up @@ -136,10 +122,7 @@ async def _aexit(self, et, exc):
assert not self._tasks

if self._base_error is not None:
try:
raise self._base_error
finally:
exc = None
raise self._base_error

if self._parent_cancel_requested:
# If this flag is set we *must* call uncancel().
Expand All @@ -150,14 +133,8 @@ async def _aexit(self, et, exc):

# Propagate CancelledError if there is one, except if there
# are other errors -- those have priority.
try:
if propagate_cancellation_error is not None and not self._errors:
try:
raise propagate_cancellation_error
finally:
exc = None
finally:
propagate_cancellation_error = None
if propagate_cancellation_error is not None and not self._errors:
raise propagate_cancellation_error

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

self._errors = None

def create_task(self, coro, *, name=None, context=None):
"""Create a new task in this group and return it.
Expand Down
22 changes: 0 additions & 22 deletions Lib/test/test_asyncio/test_futures.py
Original file line number Diff line number Diff line change
Expand Up @@ -659,28 +659,6 @@ 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
92 changes: 2 additions & 90 deletions 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,7 @@

from test.test_asyncio.utils import await_without_task


# To prevent a warning "test altered the execution environment"
def tearDownModule():
asyncio.set_event_loop_policy(None)
Expand Down Expand Up @@ -898,95 +899,6 @@ 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()

This file was deleted.

Loading