From a177afb59faed111c8633e5affe1ec2740dbcd05 Mon Sep 17 00:00:00 2001 From: spicysharkco <321699413+spicysharkco@users.noreply.github.com> Date: Thu, 15 Oct 2026 01:56:35 +0000 Subject: [PATCH 1/3] fix: free the retrying object when a call returns Every call of a @retry-decorated function runs on a copy of the retrying object, and BaseRetrying.iter() left that copy's iter_state.actions populated with bound methods of the copy. The copy then referenced itself through its own thread-local state, so only the cycle collector could free it: 11 to 14 unreachable objects per sync call and 19 to 22 per async call (about 1.6 KB and 2.3 KB), even when the first attempt succeeded. Clear the action list once iter() is done with it, also when an action raises. The list is only read inside iter() and is rebuilt on the next call; statistics, retry_state and the outcome are not touched. Retrying, AsyncRetrying and TornadoRetrying are all fixed through iter(). A call that gives up with a failed last attempt still forms the usual exception/traceback cycle through the frame that caught the exception. That case is reduced, not eliminated: 50 to 45 unreachable objects per sync call and 75 to 58 per async call on 3.10 (53 to 48 and 78 to 61 on 3.14). "async for attempt in AsyncRetrying()" also still leaves a cycle, because the retry state points back at the controller. It drops from 35 to 24 objects on 3.10 (38 to 27 on 3.14). --- ...call-reference-cycle-1f52613a199e9588.yaml | 9 +++ tenacity/__init__.py | 10 ++- tenacity/asyncio/__init__.py | 10 ++- tests/test_asyncio.py | 65 ++++++++++++++++ tests/test_tenacity.py | 74 +++++++++++++++++++ tests/test_tornado.py | 11 ++- 6 files changed, 174 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/fix-call-reference-cycle-1f52613a199e9588.yaml diff --git a/releasenotes/notes/fix-call-reference-cycle-1f52613a199e9588.yaml b/releasenotes/notes/fix-call-reference-cycle-1f52613a199e9588.yaml new file mode 100644 index 00000000..ad325f31 --- /dev/null +++ b/releasenotes/notes/fix-call-reference-cycle-1f52613a199e9588.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + A ``@retry``-decorated call that returns no longer leaves a reference + cycle behind. The iteration state of the retrying object used for each + call kept bound methods of that same object, so every call, even one + that succeeded on the first attempt, left garbage for the cycle collector + instead of being freed when the call returned. This applies to + ``Retrying``, ``AsyncRetrying`` and ``TornadoRetrying``. diff --git a/tenacity/__init__.py b/tenacity/__init__.py index 89eeca6e..fda9db1f 100644 --- a/tenacity/__init__.py +++ b/tenacity/__init__.py @@ -436,8 +436,14 @@ def _run_stop(self, retry_state: "RetryCallState") -> None: def iter(self, retry_state: "RetryCallState") -> DoAttempt | DoSleep | t.Any: self._begin_iter(retry_state) result = None - for action in self.iter_state.actions: - result = action(retry_state) + actions = self.iter_state.actions + try: + for action in actions: + result = action(retry_state) + finally: + # The actions are bound methods of ``self``: kept in the iteration + # state, they make a cycle that only the cycle collector can free. + actions.clear() return result def _begin_iter(self, retry_state: "RetryCallState") -> None: diff --git a/tenacity/asyncio/__init__.py b/tenacity/asyncio/__init__.py index 27bd2f8e..9a816495 100644 --- a/tenacity/asyncio/__init__.py +++ b/tenacity/asyncio/__init__.py @@ -167,8 +167,14 @@ async def _run_stop(self, retry_state: "RetryCallState") -> None: # type: ignor async def iter(self, retry_state: "RetryCallState") -> DoAttempt | DoSleep | t.Any: self._begin_iter(retry_state) result = None - for action in self.iter_state.actions: - result = await action(retry_state) + actions = self.iter_state.actions + try: + for action in actions: + result = await action(retry_state) + finally: + # The actions are bound methods of ``self``: kept in the iteration + # state, they make a cycle that only the cycle collector can free. + actions.clear() return result @override diff --git a/tests/test_asyncio.py b/tests/test_asyncio.py index 21c4e936..3cf554af 100644 --- a/tests/test_asyncio.py +++ b/tests/test_asyncio.py @@ -13,6 +13,7 @@ # limitations under the License. import asyncio +import contextlib import inspect import unittest from collections.abc import Callable, Coroutine @@ -43,8 +44,10 @@ from tenacity.wait import wait_fixed from .test_tenacity import ( + CustomError, NoIOErrorAfterCount, NoneReturnUntilAfterCount, + count_garbage, current_time_ms, ) @@ -658,5 +661,67 @@ def sync_function() -> Any: assert mock_sleep.await_count == 2 +def count_async_garbage(fn: Callable[[], Coroutine[Any, Any, object]]) -> int: + # asyncio.run() would count the garbage of creating and closing the loop. + loop = asyncio.new_event_loop() + try: + return count_garbage(lambda: loop.run_until_complete(fn())) + finally: + loop.close() + + +class TestAsyncReferenceCycles: + """A finished call must not leave the retrying object in a reference cycle.""" + + @pytest.mark.parametrize("reraise", [False, True]) + def test_decorator_success(self, reraise: bool) -> None: + @retry(stop=stop_after_attempt(3), reraise=reraise) + async def succeeds() -> int: + return 1 + + assert count_async_garbage(succeeds) == 0 + assert succeeds.statistics["attempt_number"] == 1 + + @pytest.mark.parametrize("reraise", [False, True]) + def test_decorator_retry_then_success(self, reraise: bool) -> None: + attempts = 0 + + @retry(stop=stop_after_attempt(3), reraise=reraise) + async def fails_once() -> int: + nonlocal attempts + attempts += 1 + if attempts % 2: + raise ValueError("fail") + return attempts + + assert count_async_garbage(fails_once) == 0 + assert fails_once.statistics["attempt_number"] == 2 + + def test_direct_call(self) -> None: + async def succeeds() -> int: + return 1 + + async def run() -> int: + return await AsyncRetrying(stop=stop_after_attempt(3))(succeeds) + + assert count_async_garbage(run) == 0 + + def test_error_in_action(self) -> None: + def stop(retry_state: RetryCallState) -> bool: + raise CustomError("stop failed") + + async def succeeds() -> int: + return 1 + + async def run() -> None: + controller = AsyncRetrying( + stop=stop, retry=retry_if_result(lambda result: True) + ) + with contextlib.suppress(CustomError): + await controller(succeeds) + + assert count_async_garbage(run) == 0 + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_tenacity.py b/tests/test_tenacity.py index 577720ec..45d80934 100644 --- a/tests/test_tenacity.py +++ b/tests/test_tenacity.py @@ -15,6 +15,7 @@ # limitations under the License. import contextlib import datetime +import gc import logging import pickle import re @@ -1219,6 +1220,20 @@ def current_time_ms() -> int: return round(time.time() * 1000) +def count_garbage(fn: typing.Callable[[], object]) -> int: + """Return the number of objects a call to ``fn`` leaves to the cycle collector.""" + fn() # warm up, so one-time caches are not counted + was_enabled = gc.isenabled() + gc.collect() + gc.disable() + try: + fn() + return gc.collect() + finally: + if was_enabled: + gc.enable() + + @retry( wait=tenacity.wait_fixed(0.05), retry=tenacity.retry_if_result(lambda result: result is None), @@ -2192,6 +2207,65 @@ def test() -> None: self.assertRaises(CustomError, test) +class TestReferenceCycles: + """A finished call must not leave the retrying object in a reference cycle. + + Cycles are only freed when the cycle collector runs, so a call that makes + one on every invocation piles up garbage until the next collection. + """ + + @pytest.mark.parametrize("reraise", [False, True]) + def test_decorator_success(self, reraise: bool) -> None: + @retry(stop=tenacity.stop_after_attempt(3), reraise=reraise) + def succeeds() -> int: + return 1 + + assert count_garbage(succeeds) == 0 + assert succeeds.statistics["attempt_number"] == 1 + + @pytest.mark.parametrize("reraise", [False, True]) + def test_decorator_retry_then_success(self, reraise: bool) -> None: + attempts = 0 + + @retry(stop=tenacity.stop_after_attempt(3), reraise=reraise) + def fails_once() -> int: + nonlocal attempts + attempts += 1 + if attempts % 2: + raise ValueError("fail") + return attempts + + assert count_garbage(fails_once) == 0 + assert fails_once.statistics["attempt_number"] == 2 + + def test_direct_call(self) -> None: + def run() -> int: + return Retrying(stop=tenacity.stop_after_attempt(3))(lambda: 1) + + assert count_garbage(run) == 0 + + def test_attempt_iteration(self) -> None: + def run() -> None: + for attempt in Retrying(stop=tenacity.stop_after_attempt(3)): + with attempt: + pass + + assert count_garbage(run) == 0 + + def test_error_in_action(self) -> None: + def stop(retry_state: RetryCallState) -> bool: + raise CustomError("stop failed") + + def run() -> None: + controller = Retrying( + stop=stop, retry=tenacity.retry_if_result(lambda result: True) + ) + with contextlib.suppress(CustomError): + controller(lambda: 1) + + assert count_garbage(run) == 0 + + class TestInvokeAsCallable: """Test direct invocation of Retrying as a callable.""" diff --git a/tests/test_tornado.py b/tests/test_tornado.py index f20a41b4..a23a5a9d 100644 --- a/tests/test_tornado.py +++ b/tests/test_tornado.py @@ -22,7 +22,7 @@ import tenacity from tenacity import RetryError, retry, stop_after_attempt, tornadoweb -from .test_tenacity import NoIOErrorAfterCount +from .test_tenacity import NoIOErrorAfterCount, count_garbage @retry @@ -58,6 +58,15 @@ def test_stop_after_attempt(self) -> Generator[Any, Any, None]: except RetryError: assert thing.counter == 2 + def test_no_reference_cycle(self) -> None: + @retry(stop=stop_after_attempt(3)) + @gen.coroutine + def succeeds() -> Generator[Any, Any, int]: + yield gen.sleep(0) + raise gen.Return(1) + + assert count_garbage(lambda: self.io_loop.run_sync(succeeds)) == 0 + def test_repr(self) -> None: repr(tornadoweb.TornadoRetrying()) From 792d75000d000b27c647c57200d6266216190377 Mon Sep 17 00:00:00 2001 From: spicysharkco <321699413+spicysharkco@users.noreply.github.com> Date: Fri, 16 Oct 2026 14:55:20 +0000 Subject: [PATCH 2/3] test: cover retry_error_callback returning a fallback Add a case to the reference-cycle tests, sync and async, where every attempt is retried and retry_error_callback returns a fallback value. The call must return the fallback, report the attempt statistics, and leave no garbage for the cycle collector. Co-authored-by: carlitogwapito <20962552+carlitogwapito@users.noreply.github.com> --- tests/test_asyncio.py | 13 +++++++++++++ tests/test_tenacity.py | 13 +++++++++++++ 2 files changed, 26 insertions(+) diff --git a/tests/test_asyncio.py b/tests/test_asyncio.py index 3cf554af..fa244049 100644 --- a/tests/test_asyncio.py +++ b/tests/test_asyncio.py @@ -722,6 +722,19 @@ async def run() -> None: assert count_async_garbage(run) == 0 + def test_retry_error_callback_fallback(self) -> None: + @retry( + stop=stop_after_attempt(3), + retry=retry_if_result(lambda result: True), + retry_error_callback=lambda retry_state: "fallback", + ) + async def always_retried() -> str: + return "retried" + + assert asyncio.run(always_retried()) == "fallback" + assert count_async_garbage(always_retried) == 0 + assert always_retried.statistics["attempt_number"] == 3 + if __name__ == "__main__": unittest.main() diff --git a/tests/test_tenacity.py b/tests/test_tenacity.py index 45d80934..0dae5303 100644 --- a/tests/test_tenacity.py +++ b/tests/test_tenacity.py @@ -2265,6 +2265,19 @@ def run() -> None: assert count_garbage(run) == 0 + def test_retry_error_callback_fallback(self) -> None: + @retry( + stop=tenacity.stop_after_attempt(3), + retry=tenacity.retry_if_result(lambda result: True), + retry_error_callback=lambda retry_state: "fallback", + ) + def always_retried() -> str: + return "retried" + + assert always_retried() == "fallback" + assert count_garbage(always_retried) == 0 + assert always_retried.statistics["attempt_number"] == 3 + class TestInvokeAsCallable: """Test direct invocation of Retrying as a callable.""" From 2c29ce7274a0cce35a9e1b40e4e695af8083ae52 Mon Sep 17 00:00:00 2001 From: spicysharkco <321699413+spicysharkco@users.noreply.github.com> Date: Tue, 20 Oct 2026 04:54:44 +0000 Subject: [PATCH 3/3] test: check that iter() clears the list it iterated iter() binds the action list to a local before running it and clears that list, not whatever iter_state.actions points to once the actions have run. Add a test per class where a before callback replaces the list mid-iteration, as another call on a shared controller would: the iterated list must end up empty and the replacement must be left alone. Clearing self.iter_state.actions instead fails both tests. --- tests/test_asyncio.py | 18 ++++++++++++++++++ tests/test_tenacity.py | 18 ++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/tests/test_asyncio.py b/tests/test_asyncio.py index fa244049..85a48147 100644 --- a/tests/test_asyncio.py +++ b/tests/test_asyncio.py @@ -735,6 +735,24 @@ async def always_retried() -> str: assert count_async_garbage(always_retried) == 0 assert always_retried.statistics["attempt_number"] == 3 + def test_clears_the_list_it_iterated(self) -> None: + iterated: list[Any] = [] + replacement: list[Any] = [print] + + async def before(retry_state: RetryCallState) -> None: + # Another call on the same controller starts a new iteration and + # replaces the list while this one is still running. + iterated.append(controller.iter_state.actions) + controller.iter_state.reset() + controller.iter_state.actions = replacement + + controller = AsyncRetrying(stop=stop_after_attempt(3), before=before) + retry_state = RetryCallState(controller, fn=None, args=(), kwargs={}) + asyncio.run(controller.iter(retry_state)) + + assert iterated[0] == [] + assert replacement == [print] + if __name__ == "__main__": unittest.main() diff --git a/tests/test_tenacity.py b/tests/test_tenacity.py index 0dae5303..b8422e66 100644 --- a/tests/test_tenacity.py +++ b/tests/test_tenacity.py @@ -2278,6 +2278,24 @@ def always_retried() -> str: assert count_garbage(always_retried) == 0 assert always_retried.statistics["attempt_number"] == 3 + def test_clears_the_list_it_iterated(self) -> None: + iterated: list[typing.Any] = [] + replacement: list[typing.Any] = [print] + + def before(retry_state: RetryCallState) -> None: + # Another call on the same controller starts a new iteration and + # replaces the list while this one is still running. + iterated.append(controller.iter_state.actions) + controller.iter_state.reset() + controller.iter_state.actions = replacement + + controller = Retrying(stop=tenacity.stop_after_attempt(3), before=before) + retry_state = RetryCallState(controller, fn=None, args=(), kwargs={}) + controller.iter(retry_state) + + assert iterated[0] == [] + assert replacement == [print] + class TestInvokeAsCallable: """Test direct invocation of Retrying as a callable."""