From 38f11cc3f62db11a4a24354bd06273322ac91afa Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sat, 16 Feb 2019 12:57:40 -0800 Subject: [PATCH] bpo-1054041: Exit properly after an uncaught ^C. (#11862) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit * 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. --- Include/internal/pycore_pylifecycle.h | 4 ++ Lib/test/test_signal.py | 61 +++++++++++++++++-- ...2019-02-16-00-42-32.bpo-1054051.BL-WLd.rst | 1 + Modules/main.c | 30 +++++++++ Python/pylifecycle.c | 1 + Python/pythonrun.c | 16 ++++- 6 files changed, 107 insertions(+), 6 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-02-16-00-42-32.bpo-1054051.BL-WLd.rst diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h index acb7391c0a..9514b1c46a 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 2a6217ef64..80c0ff4cb4 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 0000000000..e61fc0bd61 --- /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 da79a6397b..a0629911ad 100644 --- a/Modules/main.c +++ b/Modules/main.c @@ -9,6 +9,13 @@ #include "pycore_pystate.h" #include +#ifdef HAVE_SIGNAL_H +#include +#endif +#include +#if defined(HAVE_GETPID) && defined(HAVE_UNISTD_H) +#include +#endif #if defined(MS_WINDOWS) || defined(__CYGWIN__) # include @@ -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 5d5ec4a632..8d0075a48d 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 c7a622c83d..94fcc6725e 100644 --- a/Python/pythonrun.c +++ b/Python/pythonrun.c @@ -12,6 +12,7 @@ #include "Python-ast.h" #undef Yield /* undefine macro conflicting with */ +#include "pycore_pylifecycle.h" #include "pycore_pystate.h" #include "grammar.h" #include "node.h" @@ -1027,6 +1028,17 @@ flush_io(void) PyErr_Restore(type, value, traceback); } +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); -- 2.40.0