From 12e8395cf6939ec9f00df122487dc66ef774fec8 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Sat, 26 Oct 2024 11:20:15 +0100 Subject: [PATCH 1/4] avoid refcycles in trio.run, especially if an Exception is raised --- demo.py | 72 ++++++++++++++++++++++++++++++++++++++++++ src/trio/_core/_run.py | 19 +++++++---- 2 files changed, 85 insertions(+), 6 deletions(-) create mode 100644 demo.py diff --git a/demo.py b/demo.py new file mode 100644 index 0000000000..054d8fd6f1 --- /dev/null +++ b/demo.py @@ -0,0 +1,72 @@ +import trio + + +async def main(): + err = None + with trio.CancelScope() as scope: + scope.cancel() + try: + await trio.sleep_forever() + except BaseException as e: + err = e + raise + breakpoint() + + +# trio.run(main) + +import gc + +import objgraph +from anyio import CancelScope, get_cancelled_exc_class + + +async def test_exception_refcycles_propagate_cancellation_error() -> None: + """Test that TaskGroup deletes cancelled_exc""" + exc = None + + with CancelScope() as cs: + cs.cancel() + try: + await trio.sleep_forever() + except get_cancelled_exc_class() as e: + exc = e + raise + + assert isinstance(exc, get_cancelled_exc_class()) + gc.collect() + objgraph.show_chain( + objgraph.find_backref_chain( + gc.get_referrers(exc)[0], + objgraph.is_proper_module, + ), + ) + + +# trio.run(test_exception_refcycles_propagate_cancellation_error) + + +class MyException(Exception): + pass + + +async def main(): + raise MyException + + +def inner(): + try: + trio.run(main) + except MyException: + pass + + +import refcycle + +gc.disable() +gc.collect() +inner() +garbage = refcycle.garbage() +for i, component in enumerate(garbage.source_components()): + component.export_image(f"{i}_example.svg") +garbage.export_image("example.svg") diff --git a/src/trio/_core/_run.py b/src/trio/_core/_run.py index 3961a6e102..76fc7cb065 100644 --- a/src/trio/_core/_run.py +++ b/src/trio/_core/_run.py @@ -1706,6 +1706,7 @@ def close(self) -> None: self.asyncgens.close() if "after_run" in self.instruments: self.instruments.call("after_run") + self.system_nursery: Nursery | None = None # This is where KI protection gets disabled, so we do it last self.ki_manager.close() @@ -1920,6 +1921,7 @@ def task_exited(self, task: Task, outcome: Outcome[Any]) -> None: task._activate_cancel_status(None) self.tasks.remove(task) if task is self.init_task: + self.init_task = None # If the init task crashed, then something is very wrong and we # let the error propagate. (It'll eventually be wrapped in a # TrioInternalError.) @@ -1930,6 +1932,7 @@ def task_exited(self, task: Task, outcome: Outcome[Any]) -> None: raise TrioInternalError else: if task is self.main_task: + self.main_task = None self.main_task_outcome = outcome outcome = Value(None) assert task._parent_nursery is not None, task @@ -2394,12 +2397,15 @@ def run( sniffio_library.name = prev_library # Inlined copy of runner.main_task_outcome.unwrap() to avoid # cluttering every single Trio traceback with an extra frame. - if isinstance(runner.main_task_outcome, Value): - return cast(RetT, runner.main_task_outcome.value) - elif isinstance(runner.main_task_outcome, Error): - raise runner.main_task_outcome.error - else: # pragma: no cover - raise AssertionError(runner.main_task_outcome) + try: + if isinstance(runner.main_task_outcome, Value): + return cast(RetT, runner.main_task_outcome.value) + elif isinstance(runner.main_task_outcome, Error): + raise runner.main_task_outcome.error + else: # pragma: no cover + raise AssertionError(runner.main_task_outcome) + finally: + del runner def start_guest_run( @@ -2808,6 +2814,7 @@ def unrolled_run( if isinstance(runner.main_task_outcome, Error): ki.__context__ = runner.main_task_outcome.error runner.main_task_outcome = Error(ki) + del runner ################################################################ From acf8c4f5e6627f16842cee031aae7f5bd309ea83 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Sun, 22 Dec 2024 09:10:29 +0000 Subject: [PATCH 2/4] Delete demo.py --- demo.py | 72 --------------------------------------------------------- 1 file changed, 72 deletions(-) delete mode 100644 demo.py diff --git a/demo.py b/demo.py deleted file mode 100644 index 054d8fd6f1..0000000000 --- a/demo.py +++ /dev/null @@ -1,72 +0,0 @@ -import trio - - -async def main(): - err = None - with trio.CancelScope() as scope: - scope.cancel() - try: - await trio.sleep_forever() - except BaseException as e: - err = e - raise - breakpoint() - - -# trio.run(main) - -import gc - -import objgraph -from anyio import CancelScope, get_cancelled_exc_class - - -async def test_exception_refcycles_propagate_cancellation_error() -> None: - """Test that TaskGroup deletes cancelled_exc""" - exc = None - - with CancelScope() as cs: - cs.cancel() - try: - await trio.sleep_forever() - except get_cancelled_exc_class() as e: - exc = e - raise - - assert isinstance(exc, get_cancelled_exc_class()) - gc.collect() - objgraph.show_chain( - objgraph.find_backref_chain( - gc.get_referrers(exc)[0], - objgraph.is_proper_module, - ), - ) - - -# trio.run(test_exception_refcycles_propagate_cancellation_error) - - -class MyException(Exception): - pass - - -async def main(): - raise MyException - - -def inner(): - try: - trio.run(main) - except MyException: - pass - - -import refcycle - -gc.disable() -gc.collect() -inner() -garbage = refcycle.garbage() -for i, component in enumerate(garbage.source_components()): - component.export_image(f"{i}_example.svg") -garbage.export_image("example.svg") From 7c0fb4c911a2365bd092432644eaefc906ab9a83 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Sun, 22 Dec 2024 09:11:23 +0000 Subject: [PATCH 3/4] Update src/trio/_core/_run.py --- src/trio/_core/_run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/trio/_core/_run.py b/src/trio/_core/_run.py index 0d8aa45adc..1dba56b023 100644 --- a/src/trio/_core/_run.py +++ b/src/trio/_core/_run.py @@ -1726,7 +1726,7 @@ def close(self) -> None: self.asyncgens.close() if "after_run" in self.instruments: self.instruments.call("after_run") - self.system_nursery: Nursery | None = None + self.system_nursery = None # This is where KI protection gets disabled, so we do it last self.ki_manager.close() From d0ada54ce16f9c0ee91046de5668f0e339c84ff9 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Tue, 24 Dec 2024 09:19:41 +0000 Subject: [PATCH 4/4] move assert --- src/trio/_core/_run.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/trio/_core/_run.py b/src/trio/_core/_run.py index 1dba56b023..c364aa81d6 100644 --- a/src/trio/_core/_run.py +++ b/src/trio/_core/_run.py @@ -2104,14 +2104,14 @@ def deliver_ki(self) -> None: def _deliver_ki_cb(self) -> None: if not self.ki_pending: return - # Can't happen because main_task and run_sync_soon_task are created at - # the same time -- so even if KI arrives before main_task is created, - # we won't get here until afterwards. - assert self.main_task is not None if self.main_task_outcome is not None: # We're already in the process of exiting -- leave ki_pending set # and we'll check it again on our way out of run(). return + # Can't happen because main_task and run_sync_soon_task are created at + # the same time -- so even if KI arrives before main_task is created, + # we won't get here until afterwards. + assert self.main_task is not None self.main_task._attempt_delivery_of_pending_ki() ################