Issue #20317: Don't create a reference loop in ExitStack

diff --git a/Lib/contextlib.py b/Lib/contextlib.py
index f8e026b7e2feec0dc46ef21a5d56db304dc74edb..f87828569a166ec030946e382df09da67e7330a6 100644 (file)
--- a/Lib/contextlib.py
+++ b/Lib/contextlib.py
@@ -231,11 +231,19 @@ class ExitStack(object):
         # we were actually nesting multiple with statements
         frame_exc = sys.exc_info()[1]
         def _fix_exception_context(new_exc, old_exc):
+            # Context isn't what we want, so find the end of the chain
             while 1:
                 exc_context = new_exc.__context__
-                if exc_context in (None, frame_exc):
+                if exc_context is old_exc:
+                    # Context is already set correctly (see issue 20317)
+                    return
+                if exc_context is None or exc_context is frame_exc:
                     break
+                details = id(new_exc), id(old_exc), id(exc_context)
+                raise Exception(str(details))
                 new_exc = exc_context
+            # Change the end of the chain to point to the exception
+            # we expect it to reference
             new_exc.__context__ = old_exc
 
         # Callbacks are invoked in LIFO order to match the behaviour of

diff --git a/Lib/test/test_contextlib.py b/Lib/test/test_contextlib.py
index 9e45f70f85712caf80606b997fd6f8b327947476..e5365f78259f3d3cede5989b73099281d84d56bd 100644 (file)
--- a/Lib/test/test_contextlib.py
+++ b/Lib/test/test_contextlib.py
@@ -600,6 +600,29 @@ class TestExitStack(unittest.TestCase):
         else:
             self.fail("Expected KeyError, but no exception was raised")
 
+    def test_exit_exception_with_correct_context(self):
+        # http://bugs.python.org/issue20317
+        @contextmanager
+        def gets_the_context_right():
+            try:
+                yield 6
+            finally:
+                1 / 0
+
+        # The contextmanager already fixes the context, so prior to the
+        # fix, ExitStack would try to fix it *again* and get into an
+        # infinite self-referential loop
+        try:
+            with ExitStack() as stack:
+                stack.enter_context(gets_the_context_right())
+                stack.enter_context(gets_the_context_right())
+                stack.enter_context(gets_the_context_right())
+        except ZeroDivisionError as exc:
+            self.assertIsInstance(exc.__context__, ZeroDivisionError)
+            self.assertIsInstance(exc.__context__.__context__, ZeroDivisionError)
+            self.assertIsNone(exc.__context__.__context__.__context__)
+
+
     def test_body_exception_suppress(self):
         def suppress_exc(*exc_details):
             return True

diff --git a/Misc/NEWS b/Misc/NEWS
index 66f52207f0f3b425c15ebffdc425a05c8fea4719..a89762d85087775e74dea5b5607819d6d5ee2589 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -50,6 +50,12 @@ Core and Builtins
 Library
 -------
 
+- Issue #20317: ExitStack.__exit__ could create a self-referential loop if an
+  exception raised by a cleanup operation already had its context set
+  correctly (for example, by the @contextmanager decorator). The infinite
+  loop this caused is now avoided by checking if the expected context is
+  already set before trying to fix it.
+
 - Issue #20311: select.epoll.poll() now rounds the timeout away from zero,
   instead of rounding towards zero. For example, a timeout of one microsecond
   is now rounded to one millisecond, instead of being rounded to zero.