bpo-40696: Fix a hang that can arise after gen.throw() (GH-20287)

This updates _PyErr_ChainStackItem() to use _PyErr_SetObject()
instead of _PyErr_ChainExceptions(). This prevents a hang in
certain circumstances because _PyErr_SetObject() performs checks
to prevent cycles in the exception context chain while
_PyErr_ChainExceptions() doesn't.
This commit is contained in:
Chris Jerdonek 2020-05-22 13:33:27 -07:00 committed by GitHub
parent 909b5714e1
commit 7c30d12bd5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 130 additions and 23 deletions

View File

@ -51,7 +51,7 @@ PyAPI_FUNC(void) _PyErr_SetObject(
PyObject *value); PyObject *value);
PyAPI_FUNC(void) _PyErr_ChainStackItem( PyAPI_FUNC(void) _PyErr_ChainStackItem(
_PyErr_StackItem *exc_state); _PyErr_StackItem *exc_info);
PyAPI_FUNC(void) _PyErr_Clear(PyThreadState *tstate); PyAPI_FUNC(void) _PyErr_Clear(PyThreadState *tstate);

View File

@ -536,9 +536,42 @@ class BaseTaskTests:
self.assertEqual((type(chained), chained.args), self.assertEqual((type(chained), chained.args),
(KeyError, (3,))) (KeyError, (3,)))
task = self.new_task(loop, run()) try:
loop.run_until_complete(task) task = self.new_task(loop, run())
loop.close() loop.run_until_complete(task)
finally:
loop.close()
def test_exception_chaining_after_await_with_context_cycle(self):
# Check trying to create an exception context cycle:
# https://bugs.python.org/issue40696
has_cycle = None
loop = asyncio.new_event_loop()
self.set_event_loop(loop)
async def process_exc(exc):
raise exc
async def run():
nonlocal has_cycle
try:
raise KeyError('a')
except Exception as exc:
task = self.new_task(loop, process_exc(exc))
try:
await task
except BaseException as exc:
has_cycle = (exc is exc.__context__)
# Prevent a hang if has_cycle is True.
exc.__context__ = None
try:
task = self.new_task(loop, run())
loop.run_until_complete(task)
finally:
loop.close()
# This also distinguishes from the initial has_cycle=None.
self.assertEqual(has_cycle, False)
def test_cancel(self): def test_cancel(self):

View File

@ -371,6 +371,32 @@ class GeneratorThrowTest(unittest.TestCase):
context = cm.exception.__context__ context = cm.exception.__context__
self.assertEqual((type(context), context.args), (KeyError, ('a',))) self.assertEqual((type(context), context.args), (KeyError, ('a',)))
def test_exception_context_with_yield_from_with_context_cycle(self):
# Check trying to create an exception context cycle:
# https://bugs.python.org/issue40696
has_cycle = None
def f():
yield
def g(exc):
nonlocal has_cycle
try:
raise exc
except Exception:
try:
yield from f()
except Exception as exc:
has_cycle = (exc is exc.__context__)
yield
exc = KeyError('a')
gen = g(exc)
gen.send(None)
gen.throw(exc)
# This also distinguishes from the initial has_cycle=None.
self.assertEqual(has_cycle, False)
def test_throw_after_none_exc_type(self): def test_throw_after_none_exc_type(self):
def g(): def g():
try: try:

View File

@ -0,0 +1,2 @@
Fix a hang that can arise after :meth:`generator.throw` due to a cycle
in the exception context chain.

View File

@ -612,19 +612,20 @@ create_cancelled_error(PyObject *msg)
} }
static void static void
set_cancelled_error(PyObject *msg) future_set_cancelled_error(FutureObj *fut)
{ {
PyObject *exc = create_cancelled_error(msg); PyObject *exc = create_cancelled_error(fut->fut_cancel_msg);
PyErr_SetObject(asyncio_CancelledError, exc); PyErr_SetObject(asyncio_CancelledError, exc);
Py_DECREF(exc); Py_DECREF(exc);
_PyErr_ChainStackItem(&fut->fut_cancelled_exc_state);
} }
static int static int
future_get_result(FutureObj *fut, PyObject **result) future_get_result(FutureObj *fut, PyObject **result)
{ {
if (fut->fut_state == STATE_CANCELLED) { if (fut->fut_state == STATE_CANCELLED) {
set_cancelled_error(fut->fut_cancel_msg); future_set_cancelled_error(fut);
_PyErr_ChainStackItem(&fut->fut_cancelled_exc_state);
return -1; return -1;
} }
@ -866,8 +867,7 @@ _asyncio_Future_exception_impl(FutureObj *self)
} }
if (self->fut_state == STATE_CANCELLED) { if (self->fut_state == STATE_CANCELLED) {
set_cancelled_error(self->fut_cancel_msg); future_set_cancelled_error(self);
_PyErr_ChainStackItem(&self->fut_cancelled_exc_state);
return NULL; return NULL;
} }

View File

@ -203,13 +203,15 @@ gen_send_ex(PyGenObject *gen, PyObject *arg, int exc, int closing)
assert(f->f_back == NULL); assert(f->f_back == NULL);
f->f_back = tstate->frame; f->f_back = tstate->frame;
if (exc) {
_PyErr_ChainStackItem(&gen->gi_exc_state);
}
gen->gi_running = 1; gen->gi_running = 1;
gen->gi_exc_state.previous_item = tstate->exc_info; gen->gi_exc_state.previous_item = tstate->exc_info;
tstate->exc_info = &gen->gi_exc_state; tstate->exc_info = &gen->gi_exc_state;
if (exc) {
assert(_PyErr_Occurred(tstate));
_PyErr_ChainStackItem(NULL);
}
result = _PyEval_EvalFrame(tstate, f, exc); result = _PyEval_EvalFrame(tstate, f, exc);
tstate->exc_info = gen->gi_exc_state.previous_item; tstate->exc_info = gen->gi_exc_state.previous_item;
gen->gi_exc_state.previous_item = NULL; gen->gi_exc_state.previous_item = NULL;

View File

@ -477,7 +477,9 @@ PyErr_SetExcInfo(PyObject *p_type, PyObject *p_value, PyObject *p_traceback)
/* Like PyErr_Restore(), but if an exception is already set, /* Like PyErr_Restore(), but if an exception is already set,
set the context associated with it. set the context associated with it.
*/
The caller is responsible for ensuring that this call won't create
any cycles in the exception context chain. */
void void
_PyErr_ChainExceptions(PyObject *exc, PyObject *val, PyObject *tb) _PyErr_ChainExceptions(PyObject *exc, PyObject *val, PyObject *tb)
{ {
@ -512,18 +514,60 @@ _PyErr_ChainExceptions(PyObject *exc, PyObject *val, PyObject *tb)
} }
} }
/* Set the currently set exception's context to the given exception.
If the provided exc_info is NULL, then the current Python thread state's
exc_info will be used for the context instead.
This function can only be called when _PyErr_Occurred() is true.
Also, this function won't create any cycles in the exception context
chain to the extent that _PyErr_SetObject ensures this. */
void void
_PyErr_ChainStackItem(_PyErr_StackItem *exc_state) _PyErr_ChainStackItem(_PyErr_StackItem *exc_info)
{ {
if (exc_state->exc_type == NULL || exc_state->exc_type == Py_None) { PyThreadState *tstate = _PyThreadState_GET();
assert(_PyErr_Occurred(tstate));
int exc_info_given;
if (exc_info == NULL) {
exc_info_given = 0;
exc_info = tstate->exc_info;
} else {
exc_info_given = 1;
}
if (exc_info->exc_type == NULL || exc_info->exc_type == Py_None) {
return; return;
} }
Py_INCREF(exc_state->exc_type);
Py_XINCREF(exc_state->exc_value); _PyErr_StackItem *saved_exc_info;
Py_XINCREF(exc_state->exc_traceback); if (exc_info_given) {
_PyErr_ChainExceptions(exc_state->exc_type, /* Temporarily set the thread state's exc_info since this is what
exc_state->exc_value, _PyErr_SetObject uses for implicit exception chaining. */
exc_state->exc_traceback); saved_exc_info = tstate->exc_info;
tstate->exc_info = exc_info;
}
PyObject *exc, *val, *tb;
_PyErr_Fetch(tstate, &exc, &val, &tb);
PyObject *exc2, *val2, *tb2;
exc2 = exc_info->exc_type;
val2 = exc_info->exc_value;
tb2 = exc_info->exc_traceback;
_PyErr_NormalizeException(tstate, &exc2, &val2, &tb2);
if (tb2 != NULL) {
PyException_SetTraceback(val2, tb2);
}
/* _PyErr_SetObject sets the context from PyThreadState. */
_PyErr_SetObject(tstate, exc, val);
Py_DECREF(exc); // since _PyErr_Occurred was true
Py_XDECREF(val);
Py_XDECREF(tb);
if (exc_info_given) {
tstate->exc_info = saved_exc_info;
}
} }
static PyObject * static PyObject *