diff options
author | Gregory P. Smith <greg@krypto.org> | 2019-02-16 20:57:40 (GMT) |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-02-16 20:57:40 (GMT) |
commit | 38f11cc3f62db11a4a24354bd06273322ac91afa (patch) | |
tree | c41a7fd4bc4b3923cbfdd04664afc281947f56f8 | |
parent | 43766f82ddec84fad7a321eeec2e1cbff6ee44d2 (diff) | |
download | cpython-38f11cc3f62db11a4a24354bd06273322ac91afa.zip cpython-38f11cc3f62db11a4a24354bd06273322ac91afa.tar.gz cpython-38f11cc3f62db11a4a24354bd06273322ac91afa.tar.bz2 |
bpo-1054041: Exit properly after an uncaught ^C. (#11862)
* bpo-1054041: Exit properly by a signal after a ^C.
An uncaught KeyboardInterrupt exception means the user pressed ^C and
our code did not handle it. Programs that install SIGINT handlers are
supposed to reraise the SIGINT signal to the SIG_DFL handler in order
to exit in a manner that their calling process can detect that they
died due to a Ctrl-C. https://www.cons.org/cracauer/sigint.html
After this change on POSIX systems
while true; do python -c 'import time; time.sleep(23)'; done
can be stopped via a simple Ctrl-C instead of the shell infinitely
restarting a new python process.
What to do on Windows, or if anything needs to be done there has not
yet been determined. That belongs in its own PR.
TODO(gpshead): A unittest for this behavior is still needed.
* Do the unhandled ^C check after pymain_free.
* Return STATUS_CONTROL_C_EXIT on Windows.
* Fix ifdef around unistd.h include.
* 📜🤖 Added by blurb_it.
* Add STATUS_CTRL_C_EXIT to the os module on Windows
* Add unittests.
* Don't send CTRL_C_EVENT in the Windows test.
It was causing CI systems to bail out of the entire test suite.
See https://dev.azure.com/Python/cpython/_build/results?buildId=37980
for example.
* Correct posix test (fail on macOS?) check.
* STATUS_CONTROL_C_EXIT must be unsigned.
* Improve the error message.
* test typo :)
* Skip if the bash version is too old.
...and rename the windows test to reflect what it does.
* min bash version is 4.4, detect no bash.
* restore a blank line i didn't mean to delete.
* PyErr_Occurred() before the Py_DECREF(co);
* Don't add os.STATUS_CONTROL_C_EXIT as a constant.
* Update the Windows test comment.
* Refactor common logic into a run_eval_code_obj fn.
-rw-r--r-- | Include/internal/pycore_pylifecycle.h | 4 | ||||
-rw-r--r-- | Lib/test/test_signal.py | 61 | ||||
-rw-r--r-- | Misc/NEWS.d/next/Core and Builtins/2019-02-16-00-42-32.bpo-1054051.BL-WLd.rst | 1 | ||||
-rw-r--r-- | Modules/main.c | 30 | ||||
-rw-r--r-- | Python/pylifecycle.c | 1 | ||||
-rw-r--r-- | Python/pythonrun.c | 16 |
6 files changed, 107 insertions, 6 deletions
diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h index acb7391..9514b1c 100644 --- a/Include/internal/pycore_pylifecycle.h +++ b/Include/internal/pycore_pylifecycle.h @@ -8,6 +8,10 @@ extern "C" { # error "this header requires Py_BUILD_CORE or Py_BUILD_CORE_BUILTIN define" #endif +/* True if the main interpreter thread exited due to an unhandled + * KeyboardInterrupt exception, suggesting the user pressed ^C. */ +PyAPI_DATA(int) _Py_UnhandledKeyboardInterrupt; + PyAPI_FUNC(int) _Py_UnixMain(int argc, char **argv); PyAPI_FUNC(int) _Py_SetFileSystemEncoding( diff --git a/Lib/test/test_signal.py b/Lib/test/test_signal.py index 2a6217e..80c0ff4 100644 --- a/Lib/test/test_signal.py +++ b/Lib/test/test_signal.py @@ -78,6 +78,48 @@ class PosixTests(unittest.TestCase): self.assertNotIn(signal.NSIG, s) self.assertLess(len(s), signal.NSIG) + @unittest.skipUnless(sys.executable, "sys.executable required.") + def test_keyboard_interrupt_exit_code(self): + """KeyboardInterrupt triggers exit via SIGINT.""" + process = subprocess.run( + [sys.executable, "-c", + "import os,signal; os.kill(os.getpid(), signal.SIGINT)"], + stderr=subprocess.PIPE) + self.assertIn(b"KeyboardInterrupt", process.stderr) + self.assertEqual(process.returncode, -signal.SIGINT) + + @unittest.skipUnless(sys.executable, "sys.executable required.") + def test_keyboard_interrupt_communicated_to_shell(self): + """KeyboardInterrupt exits such that shells detect a ^C.""" + try: + bash_proc = subprocess.run( + ["bash", "-c", 'echo "${BASH_VERSION}"'], + stdout=subprocess.PIPE, stderr=subprocess.DEVNULL) + except OSError: + raise unittest.SkipTest("bash required.") + if bash_proc.returncode: + raise unittest.SkipTest("could not determine bash version.") + bash_ver = bash_proc.stdout.decode("ascii").strip() + bash_major_minor = [int(n) for n in bash_ver.split(".", 2)[:2]] + if bash_major_minor < [4, 4]: + # In older versions of bash, -i does not work as needed + # _for this automated test_. Older shells do behave as + # expected in manual interactive use. + raise unittest.SkipTest(f"bash version {bash_ver} is too old.") + # The motivation for https://bugs.python.org/issue1054041. + # An _interactive_ shell (bash -i simulates that here) detects + # when a command exits via ^C and stops executing further + # commands. + process = subprocess.run( + ["bash", "-ic", + f"{sys.executable} -c 'import os,signal; os.kill(os.getpid(), signal.SIGINT)'; " + "echo TESTFAIL using bash \"${BASH_VERSION}\""], + stderr=subprocess.PIPE, stdout=subprocess.PIPE) + self.assertIn(b"KeyboardInterrupt", process.stderr) + # An interactive shell will abort if python exits properly to + # indicate that a KeyboardInterrupt occurred. + self.assertNotIn(b"TESTFAIL", process.stdout) + @unittest.skipUnless(sys.platform == "win32", "Windows specific") class WindowsSignalTests(unittest.TestCase): @@ -112,6 +154,20 @@ class WindowsSignalTests(unittest.TestCase): with self.assertRaises(ValueError): signal.signal(7, handler) + @unittest.skipUnless(sys.executable, "sys.executable required.") + def test_keyboard_interrupt_exit_code(self): + """KeyboardInterrupt triggers an exit using STATUS_CONTROL_C_EXIT.""" + # We don't test via os.kill(os.getpid(), signal.CTRL_C_EVENT) here + # as that requires setting up a console control handler in a child + # in its own process group. Doable, but quite complicated. (see + # @eryksun on https://github.com/python/cpython/pull/11862) + process = subprocess.run( + [sys.executable, "-c", "raise KeyboardInterrupt"], + stderr=subprocess.PIPE) + self.assertIn(b"KeyboardInterrupt", process.stderr) + STATUS_CONTROL_C_EXIT = 0xC000013A + self.assertEqual(process.returncode, STATUS_CONTROL_C_EXIT) + class WakeupFDTests(unittest.TestCase): @@ -1217,11 +1273,8 @@ class StressTest(unittest.TestCase): class RaiseSignalTest(unittest.TestCase): def test_sigint(self): - try: + with self.assertRaises(KeyboardInterrupt): signal.raise_signal(signal.SIGINT) - self.fail("Expected KeyInterrupt") - except KeyboardInterrupt: - pass @unittest.skipIf(sys.platform != "win32", "Windows specific test") def test_invalid_argument(self): diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-02-16-00-42-32.bpo-1054051.BL-WLd.rst b/Misc/NEWS.d/next/Core and Builtins/2019-02-16-00-42-32.bpo-1054051.BL-WLd.rst new file mode 100644 index 0000000..e61fc0b --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-02-16-00-42-32.bpo-1054051.BL-WLd.rst @@ -0,0 +1 @@ +When the main interpreter exits due to an uncaught KeyboardInterrupt, the process now exits in the appropriate manner for its parent process to detect that a SIGINT or ^C terminated the process. This allows shells and batch scripts to understand that the user has asked them to stop.
\ No newline at end of file diff --git a/Modules/main.c b/Modules/main.c index da79a63..a062991 100644 --- a/Modules/main.c +++ b/Modules/main.c @@ -9,6 +9,13 @@ #include "pycore_pystate.h" #include <locale.h> +#ifdef HAVE_SIGNAL_H +#include <signal.h> +#endif +#include <stdio.h> +#if defined(HAVE_GETPID) && defined(HAVE_UNISTD_H) +#include <unistd.h> +#endif #if defined(MS_WINDOWS) || defined(__CYGWIN__) # include <windows.h> @@ -1830,6 +1837,29 @@ pymain_main(_PyMain *pymain) pymain_free(pymain); + if (_Py_UnhandledKeyboardInterrupt) { + /* https://bugs.python.org/issue1054041 - We need to exit via the + * SIG_DFL handler for SIGINT if KeyboardInterrupt went unhandled. + * If we don't, a calling process such as a shell may not know + * about the user's ^C. https://www.cons.org/cracauer/sigint.html */ +#if defined(HAVE_GETPID) && !defined(MS_WINDOWS) + if (PyOS_setsig(SIGINT, SIG_DFL) == SIG_ERR) { + perror("signal"); /* Impossible in normal environments. */ + } else { + kill(getpid(), SIGINT); + } + /* If setting SIG_DFL failed, or kill failed to terminate us, + * there isn't much else we can do aside from an error code. */ +#endif /* HAVE_GETPID && !MS_WINDOWS */ +#ifdef MS_WINDOWS + /* cmd.exe detects this, prints ^C, and offers to terminate. */ + /* https://msdn.microsoft.com/en-us/library/cc704588.aspx */ + pymain->status = STATUS_CONTROL_C_EXIT; +#else + pymain->status = SIGINT + 128; +#endif /* !MS_WINDOWS */ + } + return pymain->status; } diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 5d5ec4a..8d0075a 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -66,6 +66,7 @@ static void call_py_exitfuncs(PyInterpreterState *); static void wait_for_thread_shutdown(void); static void call_ll_exitfuncs(void); +int _Py_UnhandledKeyboardInterrupt = 0; _PyRuntimeState _PyRuntime = _PyRuntimeState_INIT; _PyInitError diff --git a/Python/pythonrun.c b/Python/pythonrun.c index c7a622c..94fcc67 100644 --- a/Python/pythonrun.c +++ b/Python/pythonrun.c @@ -12,6 +12,7 @@ #include "Python-ast.h" #undef Yield /* undefine macro conflicting with <winbase.h> */ +#include "pycore_pylifecycle.h" #include "pycore_pystate.h" #include "grammar.h" #include "node.h" @@ -1028,6 +1029,17 @@ flush_io(void) } static PyObject * +run_eval_code_obj(PyCodeObject *co, PyObject *globals, PyObject *locals) +{ + PyObject *v; + v = PyEval_EvalCode((PyObject*)co, globals, locals); + if (!v && PyErr_Occurred() == PyExc_KeyboardInterrupt) { + _Py_UnhandledKeyboardInterrupt = 1; + } + return v; +} + +static PyObject * run_mod(mod_ty mod, PyObject *filename, PyObject *globals, PyObject *locals, PyCompilerFlags *flags, PyArena *arena) { @@ -1036,7 +1048,7 @@ run_mod(mod_ty mod, PyObject *filename, PyObject *globals, PyObject *locals, co = PyAST_CompileObject(mod, filename, flags, -1, arena); if (co == NULL) return NULL; - v = PyEval_EvalCode((PyObject*)co, globals, locals); + v = run_eval_code_obj(co, globals, locals); Py_DECREF(co); return v; } @@ -1073,7 +1085,7 @@ run_pyc_file(FILE *fp, const char *filename, PyObject *globals, } fclose(fp); co = (PyCodeObject *)v; - v = PyEval_EvalCode((PyObject*)co, globals, locals); + v = run_eval_code_obj(co, globals, locals); if (v && flags) flags->cf_flags |= (co->co_flags & PyCF_MASK); Py_DECREF(co); |