From 6dfd1734f5b230bb8fbd2a9df806c1333b6652a8 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 12 Jan 2021 22:14:27 +0000 Subject: [PATCH] bpo-42848: remove recursion from TracebackException (GH-24158) --- Lib/test/test_traceback.py | 25 ++++ Lib/traceback.py | 115 +++++++++++------- .../2021-01-12-19-34-06.bpo-42848.5G8oBl.rst | 1 + 3 files changed, 95 insertions(+), 46 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2021-01-12-19-34-06.bpo-42848.5G8oBl.rst diff --git a/Lib/test/test_traceback.py b/Lib/test/test_traceback.py index abb5762cd43..07555a0411a 100644 --- a/Lib/test/test_traceback.py +++ b/Lib/test/test_traceback.py @@ -1148,6 +1148,31 @@ class TestTracebackException(unittest.TestCase): self.assertEqual(exc_info[0], exc.exc_type) self.assertEqual(str(exc_info[1]), str(exc)) + def test_long_context_chain(self): + def f(): + try: + 1/0 + except: + f() + + try: + f() + except RecursionError: + exc_info = sys.exc_info() + else: + self.fail("Exception not raised") + + te = traceback.TracebackException(*exc_info) + res = list(te.format()) + + # many ZeroDiv errors followed by the RecursionError + self.assertGreater(len(res), sys.getrecursionlimit()) + self.assertGreater( + len([l for l in res if 'ZeroDivisionError:' in l]), + sys.getrecursionlimit() * 0.5) + self.assertIn( + "RecursionError: maximum recursion depth exceeded", res[-1]) + def test_no_refs_to_exception_and_traceback_objects(self): try: 1/0 diff --git a/Lib/traceback.py b/Lib/traceback.py index 4e008bc0e08..aef37c9a7af 100644 --- a/Lib/traceback.py +++ b/Lib/traceback.py @@ -481,39 +481,10 @@ class TracebackException: # permit backwards compat with the existing API, otherwise we # need stub thunk objects just to glue it together. # Handle loops in __cause__ or __context__. + is_recursive_call = _seen is not None if _seen is None: _seen = set() _seen.add(id(exc_value)) - # Gracefully handle (the way Python 2.4 and earlier did) the case of - # being called with no type or value (None, None, None). - if (exc_value and exc_value.__cause__ is not None - and id(exc_value.__cause__) not in _seen): - cause = TracebackException( - type(exc_value.__cause__), - exc_value.__cause__, - exc_value.__cause__.__traceback__, - limit=limit, - lookup_lines=False, - capture_locals=capture_locals, - _seen=_seen) - else: - cause = None - if (exc_value and exc_value.__context__ is not None - and id(exc_value.__context__) not in _seen): - context = TracebackException( - type(exc_value.__context__), - exc_value.__context__, - exc_value.__context__.__traceback__, - limit=limit, - lookup_lines=False, - capture_locals=capture_locals, - _seen=_seen) - else: - context = None - self.__cause__ = cause - self.__context__ = context - self.__suppress_context__ = \ - exc_value.__suppress_context__ if exc_value else False # TODO: locals. self.stack = StackSummary.extract( walk_tb(exc_traceback), limit=limit, lookup_lines=lookup_lines, @@ -532,6 +503,45 @@ class TracebackException: self.msg = exc_value.msg if lookup_lines: self._load_lines() + self.__suppress_context__ = \ + exc_value.__suppress_context__ if exc_value else False + + # Convert __cause__ and __context__ to `TracebackExceptions`s, use a + # queue to avoid recursion (only the top-level call gets _seen == None) + if not is_recursive_call: + queue = [(self, exc_value)] + while queue: + te, e = queue.pop() + if (e and e.__cause__ is not None + and id(e.__cause__) not in _seen): + cause = TracebackException( + type(e.__cause__), + e.__cause__, + e.__cause__.__traceback__, + limit=limit, + lookup_lines=lookup_lines, + capture_locals=capture_locals, + _seen=_seen) + else: + cause = None + if (e and e.__context__ is not None + and id(e.__context__) not in _seen): + context = TracebackException( + type(e.__context__), + e.__context__, + e.__context__.__traceback__, + limit=limit, + lookup_lines=lookup_lines, + capture_locals=capture_locals, + _seen=_seen) + else: + context = None + te.__cause__ = cause + te.__context__ = context + if cause: + queue.append((te.__cause__, e.__cause__)) + if context: + queue.append((te.__context__, e.__context__)) @classmethod def from_exception(cls, exc, *args, **kwargs): @@ -542,10 +552,6 @@ class TracebackException: """Private API. force all lines in the stack to be loaded.""" for frame in self.stack: frame.line - if self.__context__: - self.__context__._load_lines() - if self.__cause__: - self.__cause__._load_lines() def __eq__(self, other): if isinstance(other, TracebackException): @@ -622,15 +628,32 @@ class TracebackException: The message indicating which exception occurred is always the last string in the output. """ - if chain: - if self.__cause__ is not None: - yield from self.__cause__.format(chain=chain) - yield _cause_message - elif (self.__context__ is not None and - not self.__suppress_context__): - yield from self.__context__.format(chain=chain) - yield _context_message - if self.stack: - yield 'Traceback (most recent call last):\n' - yield from self.stack.format() - yield from self.format_exception_only() + + output = [] + exc = self + while exc: + if chain: + if exc.__cause__ is not None: + chained_msg = _cause_message + chained_exc = exc.__cause__ + elif (exc.__context__ is not None and + not exc.__suppress_context__): + chained_msg = _context_message + chained_exc = exc.__context__ + else: + chained_msg = None + chained_exc = None + + output.append((chained_msg, exc)) + exc = chained_exc + else: + output.append((None, exc)) + exc = None + + for msg, exc in reversed(output): + if msg is not None: + yield msg + if exc.stack: + yield 'Traceback (most recent call last):\n' + yield from exc.stack.format() + yield from exc.format_exception_only() diff --git a/Misc/NEWS.d/next/Library/2021-01-12-19-34-06.bpo-42848.5G8oBl.rst b/Misc/NEWS.d/next/Library/2021-01-12-19-34-06.bpo-42848.5G8oBl.rst new file mode 100644 index 00000000000..4490b6ae340 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-01-12-19-34-06.bpo-42848.5G8oBl.rst @@ -0,0 +1 @@ +Removed recursion from :class:`~traceback.TracebackException` to allow it to handle long exception chains. \ No newline at end of file