From e89f95bfd0881a9b80c3b1430d154a77bdf5a824 Mon Sep 17 00:00:00 2001 From: Yury Selivanov Date: Fri, 9 Jun 2017 17:06:39 -0400 Subject: [3.6] bpo-30039: Don't run signal handlers while resuming a yield from stack (GH-1081) (#1640) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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.. (cherry picked from commit ab4413a7e9bda95b6fcd517073e2a51dafaa1624) --- Lib/test/test_generators.py | 29 +++++++++++++++++++++++++++++ Misc/NEWS | 4 ++++ Modules/_testcapimodule.c | 24 ++++++++++++++++++++++++ Python/ceval.c | 17 ++++++++++++++--- 4 files changed, 71 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index f81c82f..7107bd9 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -10,6 +10,35 @@ import types from test import support +_testcapi = support.import_module('_testcapi') + + +# This tests to make sure that if a SIGINT arrives just before we send into a +# yield from chain, the KeyboardInterrupt is raised in the innermost +# generator (see bpo-30039). +class SignalAndYieldFromTest(unittest.TestCase): + + def generator1(self): + return (yield from self.generator2()) + + def generator2(self): + try: + yield + except KeyboardInterrupt: + return "PASSED" + else: + return "FAILED" + + def test_raise_and_yield_from(self): + gen = self.generator1() + gen.send(None) + try: + _testcapi.raise_SIGINT_then_send_None(gen) + except BaseException as _exc: + exc = _exc + self.assertIs(type(exc), StopIteration) + self.assertEqual(exc.value, "PASSED") + class FinalizationTest(unittest.TestCase): diff --git a/Misc/NEWS b/Misc/NEWS index d10e7a3..580f579 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -19,6 +19,10 @@ Core and Builtins - bpo-25794: Fixed type.__setattr__() and type.__delattr__() for non-interned attribute names. Based on patch by Eryk Sun. +- bpo-30039: If a KeyboardInterrupt happens when the interpreter is in + the middle of resuming a chain of nested 'yield from' or 'await' + calls, it's now correctly delivered to the innermost frame. + - bpo-12414: sys.getsizeof() on a code object now returns the sizes which includes the code struct and sizes of objects which it references. Patch by Dong-hee Na. diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 5e73293..c3d829c 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -4125,6 +4125,29 @@ test_pyobject_fastcallkeywords(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}, @@ -4331,6 +4354,7 @@ static PyMethodDef TestMethods[] = { {"pyobject_fastcall", test_pyobject_fastcall, METH_VARARGS}, {"pyobject_fastcalldict", test_pyobject_fastcalldict, METH_VARARGS}, {"pyobject_fastcallkeywords", test_pyobject_fastcallkeywords, METH_VARARGS}, + {"raise_SIGINT_then_send_None", raise_SIGINT_then_send_None, METH_VARARGS}, {NULL, NULL} /* sentinel */ }; diff --git a/Python/ceval.c b/Python/ceval.c index 5dc0444..eba892c 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1119,9 +1119,20 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag) Py_MakePendingCalls() above. */ if (_Py_atomic_load_relaxed(&eval_breaker)) { - if (_Py_OPCODE(*next_instr) == SETUP_FINALLY) { - /* Make the last opcode before - a try: finally: block uninterruptible. */ + if (_Py_OPCODE(*next_instr) == SETUP_FINALLY || + _Py_OPCODE(*next_instr) == YIELD_FROM) { + /* Two cases where we skip running signal handlers and other + pending calls: + - If we're about to enter the try: of a try/finally (not + *very* useful, but might help in some cases and it's + traditional) + - If we're resuming a chain of nested 'yield from' or + 'await' calls, then each frame is parked with YIELD_FROM + as its next opcode. If the user hit control-C we want to + wait until we've reached the innermost frame before + running the signal handler and raising KeyboardInterrupt + (see bpo-30039). + */ goto fast_next_opcode; } if (_Py_atomic_load_relaxed(&pendingcalls_to_do)) { -- cgit v0.12