FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-25782: avoid hang in PyErr_SetObject when current exception has a… by iritkatriel · Pull Request #27626 · python/cpython · GitHub

/ cpython Public

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.

Sign up for GitHub

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
ambv merged 7 commits into from
Aug 10, 2021
Conversation 12 Commits 7 Checks 0 Files changed
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f33574a
bpo-25782: avoid hang in PyErr_SetObject when current exception has a…
iritkatriel Aug 6, 2021
70ebdd4
📜🤖 Added by blurb_it.
blurb-it[bot] Aug 7, 2021
dd4f8ec
add test
iritkatriel Aug 7, 2021
a97fe66
add test with cycle at head
iritkatriel Aug 8, 2021
7b20749
edit comment
iritkatriel Aug 8, 2021
e45bdf2
fix test name
iritkatriel Aug 8, 2021
bfc5f6b
add test for not-creating a cycle in raise
iritkatriel Aug 9, 2021
File filter

Filter by extension

Filter by extension .c  (1) .py  (1) .rst  (1) All 3 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
142 changes: 142 additions & 0 deletions Lib/test/test_exceptions.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
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)

cjerdonek Aug 8, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Core and Builtins/2021-08-07-21-39-19.bpo-25782.B22lMx.rst
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
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
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
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
Back | FazBrowse Home | New Git URL