From de86073a761cd3539aaca6f886a1f55effc0d9da Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Tue, 17 Oct 2017 17:29:39 -0400 Subject: bpo-28603: Fix formatting tracebacks for unhashable exceptions (#4014) --- Lib/idlelib/idle_test/test_run.py | 35 ++++++++++++++++ Lib/idlelib/run.py | 6 +-- Lib/test/test_traceback.py | 46 ++++++++++++++++++++++ Lib/traceback.py | 6 +-- Misc/ACKS | 1 + .../2017-10-17-13-29-19.bpo-28603._-oia3.rst | 3 ++ .../IDLE/2017-10-17-13-26-13.bpo-28603.TMEQfp.rst | 2 + .../2017-10-17-12-29-18.bpo-28603.tGuX2C.rst | 3 ++ Python/pythonrun.c | 21 ++++++++-- 9 files changed, 114 insertions(+), 9 deletions(-) create mode 100644 Lib/idlelib/idle_test/test_run.py create mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-10-17-13-29-19.bpo-28603._-oia3.rst create mode 100644 Misc/NEWS.d/next/IDLE/2017-10-17-13-26-13.bpo-28603.TMEQfp.rst create mode 100644 Misc/NEWS.d/next/Library/2017-10-17-12-29-18.bpo-28603.tGuX2C.rst diff --git a/Lib/idlelib/idle_test/test_run.py b/Lib/idlelib/idle_test/test_run.py new file mode 100644 index 0000000..d7e627d --- /dev/null +++ b/Lib/idlelib/idle_test/test_run.py @@ -0,0 +1,35 @@ +import unittest +from unittest import mock + +from test.support import captured_stderr +import idlelib.run as idlerun + + +class RunTest(unittest.TestCase): + def test_print_exception_unhashable(self): + class UnhashableException(Exception): + def __eq__(self, other): + return True + + ex1 = UnhashableException('ex1') + ex2 = UnhashableException('ex2') + try: + raise ex2 from ex1 + except UnhashableException: + try: + raise ex1 + except UnhashableException: + with captured_stderr() as output: + with mock.patch.object(idlerun, + 'cleanup_traceback') as ct: + ct.side_effect = lambda t, e: t + idlerun.print_exception() + + tb = output.getvalue().strip().splitlines() + self.assertEqual(11, len(tb)) + self.assertIn('UnhashableException: ex2', tb[3]) + self.assertIn('UnhashableException: ex1', tb[10]) + + +if __name__ == '__main__': + unittest.main(verbosity=2) diff --git a/Lib/idlelib/run.py b/Lib/idlelib/run.py index 39e0c11..6440e67 100644 --- a/Lib/idlelib/run.py +++ b/Lib/idlelib/run.py @@ -203,16 +203,16 @@ def print_exception(): seen = set() def print_exc(typ, exc, tb): - seen.add(exc) + seen.add(id(exc)) context = exc.__context__ cause = exc.__cause__ - if cause is not None and cause not in seen: + if cause is not None and id(cause) not in seen: print_exc(type(cause), cause, cause.__traceback__) print("\nThe above exception was the direct cause " "of the following exception:\n", file=efile) elif (context is not None and not exc.__suppress_context__ and - context not in seen): + id(context) not in seen): print_exc(type(context), context, context.__traceback__) print("\nDuring handling of the above exception, " "another exception occurred:\n", file=efile) diff --git a/Lib/test/test_traceback.py b/Lib/test/test_traceback.py index e483353..bffc03e 100644 --- a/Lib/test/test_traceback.py +++ b/Lib/test/test_traceback.py @@ -443,6 +443,33 @@ class TracebackFormatTests(unittest.TestCase): ' return traceback.format_stack()\n' % (__file__, lineno+1), ]) + @cpython_only + def test_unhashable(self): + from _testcapi import exception_print + + class UnhashableException(Exception): + def __eq__(self, other): + return True + + ex1 = UnhashableException('ex1') + ex2 = UnhashableException('ex2') + try: + raise ex2 from ex1 + except UnhashableException: + try: + raise ex1 + except UnhashableException: + exc_type, exc_val, exc_tb = sys.exc_info() + + with captured_output("stderr") as stderr_f: + exception_print(exc_val) + + tb = stderr_f.getvalue().strip().splitlines() + self.assertEqual(11, len(tb)) + self.assertEqual(context_message.strip(), tb[5]) + self.assertIn('UnhashableException: ex2', tb[3]) + self.assertIn('UnhashableException: ex1', tb[10]) + cause_message = ( "\nThe above exception was the direct cause " @@ -994,6 +1021,25 @@ class TestTracebackException(unittest.TestCase): self.assertEqual(exc_info[0], exc.exc_type) self.assertEqual(str(exc_info[1]), str(exc)) + def test_unhashable(self): + class UnhashableException(Exception): + def __eq__(self, other): + return True + + ex1 = UnhashableException('ex1') + ex2 = UnhashableException('ex2') + try: + raise ex2 from ex1 + except UnhashableException: + try: + raise ex1 + except UnhashableException: + exc_info = sys.exc_info() + exc = traceback.TracebackException(*exc_info) + formatted = list(exc.format()) + self.assertIn('UnhashableException: ex2\n', formatted[2]) + self.assertIn('UnhashableException: ex1\n', formatted[6]) + def test_limit(self): def recurse(n): if n: diff --git a/Lib/traceback.py b/Lib/traceback.py index fb3bce1..ee8c739 100644 --- a/Lib/traceback.py +++ b/Lib/traceback.py @@ -458,11 +458,11 @@ class TracebackException: # Handle loops in __cause__ or __context__. if _seen is None: _seen = set() - _seen.add(exc_value) + _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 exc_value.__cause__ not in _seen): + and id(exc_value.__cause__) not in _seen): cause = TracebackException( type(exc_value.__cause__), exc_value.__cause__, @@ -474,7 +474,7 @@ class TracebackException: else: cause = None if (exc_value and exc_value.__context__ is not None - and exc_value.__context__ not in _seen): + and id(exc_value.__context__) not in _seen): context = TracebackException( type(exc_value.__context__), exc_value.__context__, diff --git a/Misc/ACKS b/Misc/ACKS index 8b9f62e..f1130dc 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -147,6 +147,7 @@ Dominic Binks Philippe Biondi Michael Birtwell Stuart Bishop +Zane Bitter Roy Bixler Daniel Black Jonathan Black diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-10-17-13-29-19.bpo-28603._-oia3.rst b/Misc/NEWS.d/next/Core and Builtins/2017-10-17-13-29-19.bpo-28603._-oia3.rst new file mode 100644 index 0000000..a3542ac --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2017-10-17-13-29-19.bpo-28603._-oia3.rst @@ -0,0 +1,3 @@ +Print the full context/cause chain of exceptions on interpreter exit, even +if an exception in the chain is unhashable or compares equal to later ones. +Patch by Zane Bitter. diff --git a/Misc/NEWS.d/next/IDLE/2017-10-17-13-26-13.bpo-28603.TMEQfp.rst b/Misc/NEWS.d/next/IDLE/2017-10-17-13-26-13.bpo-28603.TMEQfp.rst new file mode 100644 index 0000000..e69f7f6 --- /dev/null +++ b/Misc/NEWS.d/next/IDLE/2017-10-17-13-26-13.bpo-28603.TMEQfp.rst @@ -0,0 +1,2 @@ +Fix a TypeError that caused a shell restart when printing a traceback that +includes an exception that is unhashable. Patch by Zane Bitter. diff --git a/Misc/NEWS.d/next/Library/2017-10-17-12-29-18.bpo-28603.tGuX2C.rst b/Misc/NEWS.d/next/Library/2017-10-17-12-29-18.bpo-28603.tGuX2C.rst new file mode 100644 index 0000000..1a5fb0a --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-10-17-12-29-18.bpo-28603.tGuX2C.rst @@ -0,0 +1,3 @@ +traceback: Fix a TypeError that occurred during printing of exception +tracebacks when either the current exception or an exception in its +context/cause chain is unhashable. Patch by Zane Bitter. diff --git a/Python/pythonrun.c b/Python/pythonrun.c index df814fb..25e2da4 100644 --- a/Python/pythonrun.c +++ b/Python/pythonrun.c @@ -817,13 +817,21 @@ print_exception_recursive(PyObject *f, PyObject *value, PyObject *seen) if (seen != NULL) { /* Exception chaining */ - if (PySet_Add(seen, value) == -1) + PyObject *value_id = PyLong_FromVoidPtr(value); + if (value_id == NULL || PySet_Add(seen, value_id) == -1) PyErr_Clear(); else if (PyExceptionInstance_Check(value)) { + PyObject *check_id = NULL; cause = PyException_GetCause(value); context = PyException_GetContext(value); if (cause) { - res = PySet_Contains(seen, cause); + check_id = PyLong_FromVoidPtr(cause); + if (check_id == NULL) { + res = -1; + } else { + res = PySet_Contains(seen, check_id); + Py_DECREF(check_id); + } if (res == -1) PyErr_Clear(); if (res == 0) { @@ -835,7 +843,13 @@ print_exception_recursive(PyObject *f, PyObject *value, PyObject *seen) } else if (context && !((PyBaseExceptionObject *)value)->suppress_context) { - res = PySet_Contains(seen, context); + check_id = PyLong_FromVoidPtr(context); + if (check_id == NULL) { + res = -1; + } else { + res = PySet_Contains(seen, check_id); + Py_DECREF(check_id); + } if (res == -1) PyErr_Clear(); if (res == 0) { @@ -848,6 +862,7 @@ print_exception_recursive(PyObject *f, PyObject *value, PyObject *seen) Py_XDECREF(context); Py_XDECREF(cause); } + Py_XDECREF(value_id); } print_exception(f, value); if (err != 0) -- cgit v0.12