fix: free the retrying object when a call returns

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``.

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:

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

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,98 @@ 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
+
+    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
+
+    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()

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,96 @@ 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
+
+    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
+
+    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."""
 

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())