From 5977c629cbf68771ba794e76ec37648657e05b49 Mon Sep 17 00:00:00 2001
From: "Nathaniel J. Smith" <njs@pobox.com>
Date: Sun, 15 Nov 2020 18:44:34 -0800
Subject: [PATCH] Avoid creating a reference cycle when calling Error.unwrap

This avoids invoking the cycle collector as often; see

  https://github.com/python-trio/trio/issues/1770

for more details.
---
 src/outcome/_impl.py | 17 ++++++++++++++++-
 tests/test_sync.py   | 20 ++++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/src/outcome/_impl.py b/src/outcome/_impl.py
index 25ab2b4..864f372 100644
--- a/src/outcome/_impl.py
+++ b/src/outcome/_impl.py
@@ -134,7 +134,22 @@ def unwrap(self):
         # Tracebacks show the 'raise' line below out of context, so let's give
         # this variable a name that makes sense out of context.
         captured_error = self.error
-        raise captured_error
+        try:
+            raise captured_error
+        finally:
+            # We want to avoid creating a reference cycle here. Python does
+            # collect cycles just fine, so it wouldn't be the end of the world
+            # if we did create a cycle, but the cyclic garbage collector adds
+            # latency to Python programs, and the more cycles you create, the
+            # more often it runs, so it's nicer to avoid creating them in the
+            # first place. For more details see:
+            #
+            #    https://github.com/python-trio/trio/issues/1770
+            #
+            # In particuar, by deleting this local variables from the 'unwrap'
+            # methods frame, we avoid the 'captured_error' object's
+            # __traceback__ from indirectly referencing 'captured_error'.
+            del captured_error, self
 
     def send(self, it):
         self._set_unwrapped()
diff --git a/tests/test_sync.py b/tests/test_sync.py
index 9e0c508..44312b4 100644
--- a/tests/test_sync.py
+++ b/tests/test_sync.py
@@ -1,4 +1,5 @@
 import traceback
+import sys
 
 import pytest
 
@@ -117,3 +118,22 @@ def raise_ValueError(x):
     frames = traceback.extract_tb(exc_info.value.__traceback__)
     functions = [function for _, _, function, _ in frames]
     assert functions[-2:] == ['unwrap', 'raise_ValueError']
+
+
+def test_Error_unwrap_does_not_create_reference_cycles():
+    # See comment in Error.unwrap for why reference cycles are tricky
+    exc = ValueError()
+    err = Error(exc)
+    try:
+        err.unwrap()
+    except ValueError:
+        pass
+    # Top frame in the traceback is the current test function; we don't care
+    # about it's references
+    assert exc.__traceback__.tb_frame is sys._getframe()
+    # The next frame down is the 'unwrap' frame; we want to make sure it
+    # doesn't reference the exception (or anything else for that matter, just
+    # to be thorough)
+    unwrap_frame = exc.__traceback__.tb_next.tb_frame
+    assert unwrap_frame.f_code.co_name == "unwrap"
+    assert unwrap_frame.f_locals == {}