Skip to content

Commit 7f77ac4

Browse files
miss-islingtoncjerdonek
miss-islington
and
cjerdonek
authored
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. (cherry picked from commit 7c30d12) Co-authored-by: Chris Jerdonek <chris.jerdonek@gmail.com>
1 parent a08b7c3 commit 7f77ac4

File tree

7 files changed

+130
-23
lines changed

7 files changed

+130
-23
lines changed

‎Include/internal/pycore_pyerrors.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ PyAPI_FUNC(void) _PyErr_SetObject(
5151
PyObject *value);
5252

5353
PyAPI_FUNC(void) _PyErr_ChainStackItem(
54-
_PyErr_StackItem *exc_state);
54+
_PyErr_StackItem *exc_info);
5555

5656
PyAPI_FUNC(void) _PyErr_Clear(PyThreadState *tstate);
5757

‎Lib/test/test_asyncio/test_tasks.py‎

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -536,9 +536,42 @@ async def run():
536536
self.assertEqual((type(chained), chained.args),
537537
(KeyError, (3,)))
538538

539-
task = self.new_task(loop, run())
540-
loop.run_until_complete(task)
541-
loop.close()
539+
try:
540+
task = self.new_task(loop, run())
541+
loop.run_until_complete(task)
542+
finally:
543+
loop.close()
544+
545+
def test_exception_chaining_after_await_with_context_cycle(self):
546+
# Check trying to create an exception context cycle:
547+
# https://bugs.python.org/issue40696
548+
has_cycle = None
549+
loop = asyncio.new_event_loop()
550+
self.set_event_loop(loop)
551+
552+
async def process_exc(exc):
553+
raise exc
554+
555+
async def run():
556+
nonlocal has_cycle
557+
try:
558+
raise KeyError('a')
559+
except Exception as exc:
560+
task = self.new_task(loop, process_exc(exc))
561+
try:
562+
await task
563+
except BaseException as exc:
564+
has_cycle = (exc is exc.__context__)
565+
# Prevent a hang if has_cycle is True.
566+
exc.__context__ = None
567+
568+
try:
569+
task = self.new_task(loop, run())
570+
loop.run_until_complete(task)
571+
finally:
572+
loop.close()
573+
# This also distinguishes from the initial has_cycle=None.
574+
self.assertEqual(has_cycle, False)
542575

543576
def test_cancel(self):
544577

‎Lib/test/test_generators.py‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,32 @@ def g():
371371
context = cm.exception.__context__
372372
self.assertEqual((type(context), context.args), (KeyError, ('a',)))
373373

374+
def test_exception_context_with_yield_from_with_context_cycle(self):
375+
# Check trying to create an exception context cycle:
376+
# https://bugs.python.org/issue40696
377+
has_cycle = None
378+
379+
def f():
380+
yield
381+
382+
def g(exc):
383+
nonlocal has_cycle
384+
try:
385+
raise exc
386+
except Exception:
387+
try:
388+
yield from f()
389+
except Exception as exc:
390+
has_cycle = (exc is exc.__context__)
391+
yield
392+
393+
exc = KeyError('a')
394+
gen = g(exc)
395+
gen.send(None)
396+
gen.throw(exc)
397+
# This also distinguishes from the initial has_cycle=None.
398+
self.assertEqual(has_cycle, False)
399+
374400
def test_throw_after_none_exc_type(self):
375401
def g():
376402
try:

‎Misc/NEWS.d/next/Core and Builtins/2020-05-21-01-54-00.bpo-40696.u3n8Wx.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix a hang that can arise after :meth:`generator.throw` due to a cycle
2+
in the exception context chain.

‎Modules/_asynciomodule.c‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -612,19 +612,20 @@ create_cancelled_error(PyObject *msg)
612612
}
613613

614614
static void
615-
set_cancelled_error(PyObject *msg)
615+
future_set_cancelled_error(FutureObj *fut)
616616
{
617-
PyObject *exc = create_cancelled_error(msg);
617+
PyObject *exc = create_cancelled_error(fut->fut_cancel_msg);
618618
PyErr_SetObject(asyncio_CancelledError, exc);
619619
Py_DECREF(exc);
620+
621+
_PyErr_ChainStackItem(&fut->fut_cancelled_exc_state);
620622
}
621623

622624
static int
623625
future_get_result(FutureObj *fut, PyObject **result)
624626
{
625627
if (fut->fut_state == STATE_CANCELLED) {
626-
set_cancelled_error(fut->fut_cancel_msg);
627-
_PyErr_ChainStackItem(&fut->fut_cancelled_exc_state);
628+
future_set_cancelled_error(fut);
628629
return -1;
629630
}
630631

@@ -866,8 +867,7 @@ _asyncio_Future_exception_impl(FutureObj *self)
866867
}
867868

868869
if (self->fut_state == STATE_CANCELLED) {
869-
set_cancelled_error(self->fut_cancel_msg);
870-
_PyErr_ChainStackItem(&self->fut_cancelled_exc_state);
870+
future_set_cancelled_error(self);
871871
return NULL;
872872
}
873873

‎Objects/genobject.c‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,13 +203,15 @@ gen_send_ex(PyGenObject *gen, PyObject *arg, int exc, int closing)
203203
assert(f->f_back == NULL);
204204
f->f_back = tstate->frame;
205205

206-
if (exc) {
207-
_PyErr_ChainStackItem(&gen->gi_exc_state);
208-
}
209-
210206
gen->gi_running = 1;
211207
gen->gi_exc_state.previous_item = tstate->exc_info;
212208
tstate->exc_info = &gen->gi_exc_state;
209+
210+
if (exc) {
211+
assert(_PyErr_Occurred(tstate));
212+
_PyErr_ChainStackItem(NULL);
213+
}
214+
213215
result = _PyEval_EvalFrame(tstate, f, exc);
214216
tstate->exc_info = gen->gi_exc_state.previous_item;
215217
gen->gi_exc_state.previous_item = NULL;

‎Python/errors.c‎

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,9 @@ PyErr_SetExcInfo(PyObject *p_type, PyObject *p_value, PyObject *p_traceback)
477477

478478
/* Like PyErr_Restore(), but if an exception is already set,
479479
set the context associated with it.
480-
*/
480+
481+
The caller is responsible for ensuring that this call won't create
482+
any cycles in the exception context chain. */
481483
void
482484
_PyErr_ChainExceptions(PyObject *exc, PyObject *val, PyObject *tb)
483485
{
@@ -512,18 +514,60 @@ _PyErr_ChainExceptions(PyObject *exc, PyObject *val, PyObject *tb)
512514
}
513515
}
514516

517+
/* Set the currently set exception's context to the given exception.
518+
519+
If the provided exc_info is NULL, then the current Python thread state's
520+
exc_info will be used for the context instead.
521+
522+
This function can only be called when _PyErr_Occurred() is true.
523+
Also, this function won't create any cycles in the exception context
524+
chain to the extent that _PyErr_SetObject ensures this. */
515525
void
516-
_PyErr_ChainStackItem(_PyErr_StackItem *exc_state)
526+
_PyErr_ChainStackItem(_PyErr_StackItem *exc_info)
517527
{
518-
if (exc_state->exc_type == NULL || exc_state->exc_type == Py_None) {
528+
PyThreadState *tstate = _PyThreadState_GET();
529+
assert(_PyErr_Occurred(tstate));
530+
531+
int exc_info_given;
532+
if (exc_info == NULL) {
533+
exc_info_given = 0;
534+
exc_info = tstate->exc_info;
535+
} else {
536+
exc_info_given = 1;
537+
}
538+
if (exc_info->exc_type == NULL || exc_info->exc_type == Py_None) {
519539
return;
520540
}
521-
Py_INCREF(exc_state->exc_type);
522-
Py_XINCREF(exc_state->exc_value);
523-
Py_XINCREF(exc_state->exc_traceback);
524-
_PyErr_ChainExceptions(exc_state->exc_type,
525-
exc_state->exc_value,
526-
exc_state->exc_traceback);
541+
542+
_PyErr_StackItem *saved_exc_info;
543+
if (exc_info_given) {
544+
/* Temporarily set the thread state's exc_info since this is what
545+
_PyErr_SetObject uses for implicit exception chaining. */
546+
saved_exc_info = tstate->exc_info;
547+
tstate->exc_info = exc_info;
548+
}
549+
550+
PyObject *exc, *val, *tb;
551+
_PyErr_Fetch(tstate, &exc, &val, &tb);
552+
553+
PyObject *exc2, *val2, *tb2;
554+
exc2 = exc_info->exc_type;
555+
val2 = exc_info->exc_value;
556+
tb2 = exc_info->exc_traceback;
557+
_PyErr_NormalizeException(tstate, &exc2, &val2, &tb2);
558+
if (tb2 != NULL) {
559+
PyException_SetTraceback(val2, tb2);
560+
}
561+
562+
/* _PyErr_SetObject sets the context from PyThreadState. */
563+
_PyErr_SetObject(tstate, exc, val);
564+
Py_DECREF(exc); // since _PyErr_Occurred was true
565+
Py_XDECREF(val);
566+
Py_XDECREF(tb);
567+
568+
if (exc_info_given) {
569+
tstate->exc_info = saved_exc_info;
570+
}
527571
}
528572

529573
static PyObject *

0 commit comments

Comments
 (0)