Skip to content

bpo-25782: avoid hang in PyErr_SetObject when current exception has a… #27626

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Aug 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 142 additions & 0 deletions Lib/test/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -953,6 +953,148 @@ def __del__(self):
pass
self.assertEqual(e, (None, None, None))

def test_raise_does_not_create_context_chain_cycle(self):
class A(Exception):
pass
class B(Exception):
pass
class C(Exception):
pass

# Create a context chain:
# C -> B -> A
# Then raise A in context of C.
try:
try:
raise A
except A as a_:
a = a_
try:
raise B
except B as b_:
b = b_
try:
raise C
except C as c_:
c = c_
self.assertIsInstance(a, A)
self.assertIsInstance(b, B)
self.assertIsInstance(c, C)
self.assertIsNone(a.__context__)
self.assertIs(b.__context__, a)
self.assertIs(c.__context__, b)
raise a
except A as e:
exc = e

# Expect A -> C -> B, without cycle
self.assertIs(exc, a)
self.assertIs(a.__context__, c)
self.assertIs(c.__context__, b)
self.assertIsNone(b.__context__)

def test_no_hang_on_context_chain_cycle1(self):
# See issue 25782. Cycle in context chain.

def cycle():
try:
raise ValueError(1)
except ValueError as ex:
ex.__context__ = ex
raise TypeError(2)

try:
cycle()
except Exception as e:
exc = e

self.assertIsInstance(exc, TypeError)
self.assertIsInstance(exc.__context__, ValueError)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense also to check exc.__context__.__context__ (to check that the cycle was preserved)?

self.assertIs(exc.__context__.__context__, exc.__context__)

def test_no_hang_on_context_chain_cycle2(self):
# See issue 25782. Cycle at head of context chain.

class A(Exception):
pass
class B(Exception):
pass
class C(Exception):
pass

# Context cycle:
# +-----------+
# V |
# C --> B --> A
with self.assertRaises(C) as cm:
try:
raise A()
except A as _a:
a = _a
try:
raise B()
except B as _b:
b = _b
try:
raise C()
except C as _c:
c = _c
a.__context__ = c
raise c

self.assertIs(cm.exception, c)
# Verify the expected context chain cycle
self.assertIs(c.__context__, b)
self.assertIs(b.__context__, a)
self.assertIs(a.__context__, c)

def test_no_hang_on_context_chain_cycle3(self):
# See issue 25782. Longer context chain with cycle.

class A(Exception):
pass
class B(Exception):
pass
class C(Exception):
pass
class D(Exception):
pass
class E(Exception):
pass

# Context cycle:
# +-----------+
# V |
# E --> D --> C --> B --> A
with self.assertRaises(E) as cm:
try:
raise A()
except A as _a:
a = _a
try:
raise B()
except B as _b:
b = _b
try:
raise C()
except C as _c:
c = _c
a.__context__ = c
try:
raise D()
except D as _d:
d = _d
e = E()
raise e

self.assertIs(cm.exception, e)
# Verify the expected context chain cycle
self.assertIs(e.__context__, d)
self.assertIs(d.__context__, c)
self.assertIs(c.__context__, b)
self.assertIs(b.__context__, a)
self.assertIs(a.__context__, c)

def test_unicode_change_attributes(self):
# See issue 7309. This was a crasher.

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix bug where ``PyErr_SetObject`` hangs when the current exception has a cycle in its context chain.
16 changes: 15 additions & 1 deletion Python/errors.c
Original file line number Diff line number Diff line change
Expand Up @@ -148,19 +148,33 @@ _PyErr_SetObject(PyThreadState *tstate, PyObject *exception, PyObject *value)
value = fixed_value;
}

/* Avoid reference cycles through the context chain.
/* Avoid creating new reference cycles through the
context chain, while taking care not to hang on
pre-existing ones.
This is O(chain length) but context chains are
usually very short. Sensitive readers may try
to inline the call to PyException_GetContext. */
if (exc_value != value) {
PyObject *o = exc_value, *context;
PyObject *slow_o = o; /* Floyd's cycle detection algo */
int slow_update_toggle = 0;
while ((context = PyException_GetContext(o))) {
Py_DECREF(context);
if (context == value) {
PyException_SetContext(o, NULL);
break;
}
o = context;
if (o == slow_o) {
/* pre-existing cycle - all exceptions on the
path were visited and checked. */
break;
}
if (slow_update_toggle) {
slow_o = PyException_GetContext(slow_o);
Py_DECREF(slow_o);
}
slow_update_toggle = !slow_update_toggle;
}
PyException_SetContext(value, exc_value);
}
Expand Down