Skip to content

bpo-30039: Don't run signal handlers while resuming a yield from stack #1081

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 17, 2017
Merged

bpo-30039: Don't run signal handlers while resuming a yield from stack #1081

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7126cc1
bpo-30039: Don't run signal handlers while resuming a yield from stack
njsmith Apr 11, 2017
b324f82
Merge branch 'master' into no-signal-handlers-during-yield-from
njsmith Apr 23, 2017
51a3605
Merge branch 'master' into no-signal-handlers-during-yield-from
1st1 May 1, 2017
30bd745
Merge branch 'master' into no-signal-handlers-during-yield-from
1st1 May 17, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions Lib/test/test_generators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,35 @@

from test import support

_testcapi = support.import_module('_testcapi')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This broke the specs on other implementation, which don't have a _testcapi module.
It should always be guarded by try: except ImportError and skip the spec with the unittest.skipIf decorator.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please open a new bug. It seems like there are other bugs, like test_float.py:

    @support.requires_IEEE_754
    def test_serialized_float_rounding(self):
        from _testcapi import FLT_MAX
        ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, a new bug is the way to go. And possibly the developers guide should be updated to say whatever the policy is? (What other implementations are you thinking of? I'm a little wary about just skipping tests – this issue this test is checking for can easily exist on other implementations too...)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll create a new bug.

@njsmith I'm working on a java implementation which is based on jython. Your concern is valid, the issue might exist as well in other implementations, that's why I didn't suggest @support.cpython_only, but since the helper method from the _testcapi is also new, it should still be guarded by skipIf, or else the whole test crashes.

Copy link
Contributor

@isaiah isaiah Dec 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vstinner just checked the one you posted, it works fine, because it's guarded by @support.requires_IEEE_754, and test.support is implemented in python.



# 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):

Expand Down
4 changes: 4 additions & 0 deletions Misc/NEWS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ What's New in Python 3.7.0 alpha 1?
Core and Builtins
-----------------

- 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-28974: ``object.__format__(x, '')`` is now equivalent to ``str(x)``
rather than ``format(str(self), '')``.

Expand Down
24 changes: 24 additions & 0 deletions Modules/_testcapimodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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 */
};

Expand Down
17 changes: 14 additions & 3 deletions Python/ceval.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1064,9 +1064,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)) {
Expand Down