[Python-checkins] bpo-25782: avoid hang in PyErr_SetObject when current exception has a cycle in its context chain (GH-27626) (GH-27707)
ambv
webhook-mailer at python.org
Tue Aug 10 07:09:04 EDT 2021
https://github.com/python/cpython/commit/6f4cdeddb97532144f93ca37b8b21451f445c7bf
commit: 6f4cdeddb97532144f93ca37b8b21451f445c7bf
branch: 3.9
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: ambv <lukasz at langa.pl>
date: 2021-08-10T13:08:41+02:00
summary:
bpo-25782: avoid hang in PyErr_SetObject when current exception has a cycle in its context chain (GH-27626) (GH-27707)
Co-authored-by: Dennis Sweeney 36520290+sweeneyde at users.noreply.github.com
(cherry picked from commit d5c217475c4957a8084ac3f92ae012ece5edc7cb)
Co-authored-by: Irit Katriel <1055913+iritkatriel at users.noreply.github.com>
files:
A Misc/NEWS.d/next/Core and Builtins/2021-08-07-21-39-19.bpo-25782.B22lMx.rst
M Lib/test/test_exceptions.py
M Python/errors.c
diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py
index 0f7b7f84809d9..cd7050bb20b03 100644
--- a/Lib/test/test_exceptions.py
+++ b/Lib/test/test_exceptions.py
@@ -923,6 +923,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)
+ 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.
diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-08-07-21-39-19.bpo-25782.B22lMx.rst b/Misc/NEWS.d/next/Core and Builtins/2021-08-07-21-39-19.bpo-25782.B22lMx.rst
new file mode 100644
index 0000000000000..1c52059f76c8f
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2021-08-07-21-39-19.bpo-25782.B22lMx.rst
@@ -0,0 +1 @@
+Fix bug where ``PyErr_SetObject`` hangs when the current exception has a cycle in its context chain.
\ No newline at end of file
diff --git a/Python/errors.c b/Python/errors.c
index 2c020cd2660c5..8a2ba8f9d7163 100644
--- a/Python/errors.c
+++ b/Python/errors.c
@@ -148,12 +148,16 @@ _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) {
@@ -161,6 +165,16 @@ _PyErr_SetObject(PyThreadState *tstate, PyObject *exception, PyObject *value)
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);
}
More information about the Python-checkins
mailing list