diff --git a/newsfragments/1770.bugfix.rst b/newsfragments/1770.bugfix.rst new file mode 100644 index 0000000000..4ef2303d9d --- /dev/null +++ b/newsfragments/1770.bugfix.rst @@ -0,0 +1,3 @@ +Trio now avoids creating cyclic garbage as often. This should have a +minimal impact on most programs, but can slightly reduce how often the +cycle collector GC runs on CPython, which can reduce latency spikes. diff --git a/trio/_core/_multierror.py b/trio/_core/_multierror.py index 9e0de7cf9f..13fc3f3d0f 100644 --- a/trio/_core/_multierror.py +++ b/trio/_core/_multierror.py @@ -114,6 +114,9 @@ def push_tb_down(tb, exc, preserved): preserved = set() new_root_exc = filter_tree(root_exc, preserved) push_tb_down(None, root_exc, preserved) + # Delete the local functions to avoid a reference cycle (see + # test_simple_cancel_scope_usage_doesnt_create_cyclic_garbage) + del filter_tree, push_tb_down return new_root_exc @@ -132,6 +135,7 @@ def __enter__(self): def __exit__(self, etype, exc, tb): if exc is not None: filtered_exc = MultiError.filter(self._handler, exc) + if filtered_exc is exc: # Let the interpreter re-raise it return False diff --git a/trio/_core/_run.py b/trio/_core/_run.py index 154b9bb590..e56977f386 100644 --- a/trio/_core/_run.py +++ b/trio/_core/_run.py @@ -937,7 +937,12 @@ def aborted(raise_cancel): popped = self._parent_task._child_nurseries.pop() assert popped is self if self._pending_excs: - return MultiError(self._pending_excs) + try: + return MultiError(self._pending_excs) + finally: + # avoid a garbage cycle + # (see test_nursery_cancel_doesnt_create_cyclic_garbage) + del self._pending_excs def start_soon(self, async_fn, *args, name=None): """Creates a child task, scheduling ``await async_fn(*args)``. diff --git a/trio/_core/tests/test_run.py b/trio/_core/tests/test_run.py index bc1f7867a0..4c4e12b5df 100644 --- a/trio/_core/tests/test_run.py +++ b/trio/_core/tests/test_run.py @@ -9,6 +9,7 @@ from contextlib import contextmanager, ExitStack from math import inf from textwrap import dedent +import gc import attr import outcome @@ -2195,3 +2196,56 @@ async def test_cancel_scope_deadline_duplicates(): cscope.deadline = now + 9998 cscope.deadline = now + 9999 await sleep(0.01) + + +@pytest.mark.skipif( + sys.implementation.name != "cpython", reason="Only makes sense with refcounting GC" +) +async def test_simple_cancel_scope_usage_doesnt_create_cyclic_garbage(): + # https://github.com/python-trio/trio/issues/1770 + gc.collect() + + async def do_a_cancel(): + with _core.CancelScope() as cscope: + cscope.cancel() + await sleep_forever() + + old_flags = gc.get_debug() + try: + gc.collect() + gc.set_debug(gc.DEBUG_SAVEALL) + + await do_a_cancel() + await do_a_cancel() + + async with _core.open_nursery() as nursery: + nursery.start_soon(do_a_cancel) + + gc.collect() + assert not gc.garbage + finally: + gc.set_debug(old_flags) + gc.garbage.clear() + + +@pytest.mark.skipif( + sys.implementation.name != "cpython", reason="Only makes sense with refcounting GC" +) +async def test_nursery_cancel_doesnt_create_cyclic_garbage(): + # https://github.com/python-trio/trio/issues/1770#issuecomment-730229423 + gc.collect() + + old_flags = gc.get_debug() + try: + for i in range(3): + async with _core.open_nursery() as nursery: + gc.collect() + gc.set_debug(gc.DEBUG_LEAK) + nursery.cancel_scope.cancel() + + gc.collect() + gc.set_debug(0) + assert not gc.garbage + finally: + gc.set_debug(old_flags) + gc.garbage.clear()