bpo-28603: Fix formatting tracebacks for unhashable exceptions (#4014)

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)

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)

Lib/test/test_traceback.py

@@ -443,6 +443,33 @@ def fmt():
             '    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 @@ def test_context(self):
         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:

Lib/traceback.py

@@ -458,11 +458,11 @@ def __init__(self, exc_type, exc_value, exc_traceback, *, limit=None,
         # 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 @@ def __init__(self, exc_type, exc_value, exc_traceback, *, limit=None,
         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__,

Misc/ACKS

@@ -147,6 +147,7 @@ Dominic Binks
 Philippe Biondi
 Michael Birtwell
 Stuart Bishop
+Zane Bitter
 Roy Bixler
 Daniel Black
 Jonathan Black

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.

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.

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.

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)