diff options
author | Nathaniel J. Smith <njs@pobox.com> | 2017-05-17 20:33:23 (GMT) |
---|---|---|
committer | Yury Selivanov <yury@magic.io> | 2017-05-17 20:33:23 (GMT) |
commit | ab4413a7e9bda95b6fcd517073e2a51dafaa1624 (patch) | |
tree | 5cc59c52b7903122be7032955a51add8892dd669 /Modules | |
parent | 4486a0934680af26a21f308eb9c295d2525570f2 (diff) | |
download | cpython-ab4413a7e9bda95b6fcd517073e2a51dafaa1624.zip cpython-ab4413a7e9bda95b6fcd517073e2a51dafaa1624.tar.gz cpython-ab4413a7e9bda95b6fcd517073e2a51dafaa1624.tar.bz2 |
bpo-30039: Don't run signal handlers while resuming a yield from stack (#1081)
If we have a chain of generators/coroutines that are 'yield from'ing
each other, then resuming the stack works like:
- call send() on the outermost generator
- this enters _PyEval_EvalFrameDefault, which re-executes the
YIELD_FROM opcode
- which calls send() on the next generator
- which enters _PyEval_EvalFrameDefault, which re-executes the
YIELD_FROM opcode
- ...etc.
However, every time we enter _PyEval_EvalFrameDefault, the first thing
we do is to check for pending signals, and if there are any then we
run the signal handler. And if it raises an exception, then we
immediately propagate that exception *instead* of starting to execute
bytecode. This means that e.g. a SIGINT at the wrong moment can "break
the chain" – it can be raised in the middle of our yield from chain,
with the bottom part of the stack abandoned for the garbage collector.
The fix is pretty simple: there's already a special case in
_PyEval_EvalFrameEx where it skips running signal handlers if the next
opcode is SETUP_FINALLY. (I don't see how this accomplishes anything
useful, but that's another story.) If we extend this check to also
skip running signal handlers when the next opcode is YIELD_FROM, then
that closes the hole – now the exception can only be raised at the
innermost stack frame.
This shouldn't have any performance implications, because the opcode
check happens inside the "slow path" after we've already determined
that there's a pending signal or something similar for us to process;
the vast majority of the time this isn't true and the new check
doesn't run at all.
Diffstat (limited to 'Modules')
-rw-r--r-- | Modules/_testcapimodule.c | 24 |
1 files changed, 24 insertions, 0 deletions
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index f9de940..7f7a13e 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -4028,6 +4028,29 @@ dict_get_version(PyObject *self, PyObject *args) } +static PyObject * +raise_SIGINT_then_send_None(PyObject *self, PyObject *args) +{ + PyGenObject *gen; + + if (!PyArg_ParseTuple(args, "O!", &PyGen_Type, &gen)) + return NULL; + + /* This is used in a test to check what happens if a signal arrives just + as we're in the process of entering a yield from chain (see + bpo-30039). + + Needs to be done in C, because: + - we don't have a Python wrapper for raise() + - we need to make sure that the Python-level signal handler doesn't run + *before* we enter the generator frame, which is impossible in Python + because we check for signals before every bytecode operation. + */ + raise(SIGINT); + return _PyGen_Send(gen, Py_None); +} + + static PyMethodDef TestMethods[] = { {"raise_exception", raise_exception, METH_VARARGS}, {"raise_memoryerror", (PyCFunction)raise_memoryerror, METH_NOARGS}, @@ -4232,6 +4255,7 @@ static PyMethodDef TestMethods[] = { {"tracemalloc_untrack", tracemalloc_untrack, METH_VARARGS}, {"tracemalloc_get_traceback", tracemalloc_get_traceback, METH_VARARGS}, {"dict_get_version", dict_get_version, METH_VARARGS}, + {"raise_SIGINT_then_send_None", raise_SIGINT_then_send_None, METH_VARARGS}, {NULL, NULL} /* sentinel */ }; |