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

Revert "gh-124958: fix asyncio.TaskGroup and _PyFuture refcycles (#124959)"

This reverts commit d5dbbf4372cd3dbf3eead1cc70ddc4261c061fd9.

Lib/asyncio/futures.py

@@ -190,7 +190,8 @@ class Future:
         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
@@ -207,7 +208,8 @@ class Future:
         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

Lib/asyncio/taskgroups.py

@@ -66,20 +66,6 @@ class TaskGroup:
         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
@@ -136,10 +122,7 @@ class TaskGroup:
         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().
@@ -150,14 +133,8 @@ class TaskGroup:
 
         # 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)
@@ -169,14 +146,14 @@ class TaskGroup:
             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.

Lib/test/test_asyncio/test_futures.py

@@ -659,28 +659,6 @@ class BaseFutureTests:
             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')

Lib/test/test_asyncio/test_taskgroups.py

@@ -1,7 +1,7 @@
 # Adapted with permission from the EdgeDB project;
 # license: PSFL.
 
-import gc
+
 import asyncio
 import contextvars
 import contextlib
@@ -11,6 +11,7 @@ import warnings
 
 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)
@@ -898,95 +899,6 @@ class TestTaskGroup(unittest.IsolatedAsyncioTestCase):
 
         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()

Misc/NEWS.d/next/Library/2024-10-04-08-46-00.gh-issue-124958.rea9-x.rst

@@ -1 +0,0 @@
-Fix refcycles in exceptions raised from :class:`asyncio.TaskGroup` and the python implementation of :class:`asyncio.Future`