(original) (raw)

changeset: 88637:b3c2472c12a1 branch: 3.3 parent: 88631:1b89fd73c625 user: Nick Coghlan ncoghlan@gmail.com date: Wed Jan 22 22:24:46 2014 +1000 files: Lib/contextlib.py Lib/test/test_contextlib.py Misc/NEWS description: Issue #20317: Don't create a reference loop in ExitStack diff -r 1b89fd73c625 -r b3c2472c12a1 Lib/contextlib.py --- a/Lib/contextlib.py Tue Jan 21 21:12:24 2014 -0500 +++ b/Lib/contextlib.py Wed Jan 22 22:24:46 2014 +1000 @@ -231,11 +231,19 @@ # 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 -r 1b89fd73c625 -r b3c2472c12a1 Lib/test/test_contextlib.py --- a/Lib/test/test_contextlib.py Tue Jan 21 21:12:24 2014 -0500 +++ b/Lib/test/test_contextlib.py Wed Jan 22 22:24:46 2014 +1000 @@ -600,6 +600,29 @@ 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 -r 1b89fd73c625 -r b3c2472c12a1 Misc/NEWS --- a/Misc/NEWS Tue Jan 21 21:12:24 2014 -0500 +++ b/Misc/NEWS Wed Jan 22 22:24:46 2014 +1000 @@ -50,6 +50,12 @@ 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. /ncoghlan@gmail.com