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.
This commit is contained in:
Gregory P. Smith 2019-02-16 12:57:40 -08:00 committed by GitHub
parent 43766f82dd
commit 38f11cc3f6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 107 additions and 6 deletions

View File

@ -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(

View File

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

View File

@ -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.

View File

@ -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;
}

View File

@ -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

View File

@ -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"
@ -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);