gh-128130: Fix unhandled keyboard interrupt data race (gh-129975)

Use an atomic operation when setting
`_PyRuntime.signals.unhandled_keyboard_interrupt`. We now only clear the
variable at the start of `_PyRun_Main`, which is the same function where
we check it.

This avoids race conditions where previously another thread might call
`run_eval_code_obj()` and erroneously clear the unhandled keyboard
interrupt.

diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h
index f426ae0e103b9c991229c1e1cc2c9c55ad12b715..a5f9418ec057183a2280c9e529fd0b9d853fe011 100644 (file)
--- a/Include/internal/pycore_pylifecycle.h
+++ b/Include/internal/pycore_pylifecycle.h
@@ -75,7 +75,7 @@ extern PyStatus _Py_PreInitializeFromConfig(
 
 extern wchar_t * _Py_GetStdlibDir(void);
 
-extern int _Py_HandleSystemExit(int *exitcode_p);
+extern int _Py_HandleSystemExitAndKeyboardInterrupt(int *exitcode_p);
 
 extern PyObject* _PyErr_WriteUnraisableDefaultHook(PyObject *unraisable);
 

diff --git a/Modules/main.c b/Modules/main.c
index f8a2438cdd0d93b476cf2a48082832140200935e..0aebce0d3b7aa237c33d3aacdcdc1cc05a618910 100644 (file)
--- a/Modules/main.c
+++ b/Modules/main.c
@@ -99,7 +99,7 @@ static int
 pymain_err_print(int *exitcode_p)
 {
     int exitcode;
-    if (_Py_HandleSystemExit(&exitcode)) {
+    if (_Py_HandleSystemExitAndKeyboardInterrupt(&exitcode)) {
         *exitcode_p = exitcode;
         return 1;
     }
@@ -292,11 +292,7 @@ pymain_start_pyrepl_no_main(void)
         goto done;
     }
     if (!PyDict_SetItemString(kwargs, "pythonstartup", _PyLong_GetOne())) {
-        _PyRuntime.signals.unhandled_keyboard_interrupt = 0;
         console_result = PyObject_Call(console, empty_tuple, kwargs);
-        if (!console_result && PyErr_Occurred() == PyExc_KeyboardInterrupt) {
-            _PyRuntime.signals.unhandled_keyboard_interrupt = 1;
-        }
         if (console_result == NULL) {
             res = pymain_exit_err_print();
         }
@@ -338,11 +334,7 @@ pymain_run_module(const wchar_t *modname, int set_argv0)
         Py_DECREF(module);
         return pymain_exit_err_print();
     }
-    _PyRuntime.signals.unhandled_keyboard_interrupt = 0;
     result = PyObject_Call(runmodule, runargs, NULL);
-    if (!result && PyErr_Occurred() == PyExc_KeyboardInterrupt) {
-        _PyRuntime.signals.unhandled_keyboard_interrupt = 1;
-    }
     Py_DECREF(runmodule);
     Py_DECREF(module);
     Py_DECREF(runargs);
@@ -763,6 +755,8 @@ Py_RunMain(void)
 {
     int exitcode = 0;
 
+    _PyRuntime.signals.unhandled_keyboard_interrupt = 0;
+
     pymain_run_python(&exitcode);
 
     if (Py_FinalizeEx() < 0) {

diff --git a/Python/pythonrun.c b/Python/pythonrun.c
index ae0df9685ac1592afa7d324e932e2bb0d60dc2a3..945e267ef72c6feb6a9dcea144971b89f6e38b6c 100644 (file)
--- a/Python/pythonrun.c
+++ b/Python/pythonrun.c
@@ -589,8 +589,13 @@ parse_exit_code(PyObject *code, int *exitcode_p)
 }
 
 int
-_Py_HandleSystemExit(int *exitcode_p)
+_Py_HandleSystemExitAndKeyboardInterrupt(int *exitcode_p)
 {
+    if (PyErr_ExceptionMatches(PyExc_KeyboardInterrupt)) {
+        _Py_atomic_store_int(&_PyRuntime.signals.unhandled_keyboard_interrupt, 1);
+        return 0;
+    }
+
     int inspect = _Py_GetConfig()->inspect;
     if (inspect) {
         /* Don't exit if -i flag was given. This flag is set to 0
@@ -646,7 +651,7 @@ static void
 handle_system_exit(void)
 {
     int exitcode;
-    if (_Py_HandleSystemExit(&exitcode)) {
+    if (_Py_HandleSystemExitAndKeyboardInterrupt(&exitcode)) {
         Py_Exit(exitcode);
     }
 }
@@ -1105,8 +1110,6 @@ _PyErr_Display(PyObject *file, PyObject *unused, PyObject *value, PyObject *tb)
         }
     }
 
-    int unhandled_keyboard_interrupt = _PyRuntime.signals.unhandled_keyboard_interrupt;
-
     // Try first with the stdlib traceback module
     PyObject *print_exception_fn = PyImport_ImportModuleAttrString(
         "traceback",
@@ -1120,11 +1123,9 @@ _PyErr_Display(PyObject *file, PyObject *unused, PyObject *value, PyObject *tb)
     Py_XDECREF(print_exception_fn);
     if (result) {
         Py_DECREF(result);
-        _PyRuntime.signals.unhandled_keyboard_interrupt = unhandled_keyboard_interrupt;
         return;
     }
 fallback:
-    _PyRuntime.signals.unhandled_keyboard_interrupt = unhandled_keyboard_interrupt;
 #ifdef Py_DEBUG
      if (PyErr_Occurred()) {
          PyErr_FormatUnraisable(
@@ -1297,20 +1298,6 @@ flush_io(void)
 static PyObject *
 run_eval_code_obj(PyThreadState *tstate, PyCodeObject *co, PyObject *globals, PyObject *locals)
 {
-    PyObject *v;
-    /*
-     * We explicitly re-initialize _Py_UnhandledKeyboardInterrupt every eval
-     * _just in case_ someone is calling into an embedded Python where they
-     * don't care about an uncaught KeyboardInterrupt exception (why didn't they
-     * leave config.install_signal_handlers set to 0?!?) but then later call
-     * Py_Main() itself (which _checks_ this flag and dies with a signal after
-     * its interpreter exits).  We don't want a previous embedded interpreter's
-     * uncaught exception to trigger an unexplained signal exit from a future
-     * Py_Main() based one.
-     */
-    // XXX Isn't this dealt with by the move to _PyRuntimeState?
-    _PyRuntime.signals.unhandled_keyboard_interrupt = 0;
-
     /* Set globals['__builtins__'] if it doesn't exist */
     if (!globals || !PyDict_Check(globals)) {
         PyErr_SetString(PyExc_SystemError, "globals must be a real dict");
@@ -1328,11 +1315,7 @@ run_eval_code_obj(PyThreadState *tstate, PyCodeObject *co, PyObject *globals, Py
         }
     }
 
-    v = PyEval_EvalCode((PyObject*)co, globals, locals);
-    if (!v && _PyErr_Occurred(tstate) == PyExc_KeyboardInterrupt) {
-        _PyRuntime.signals.unhandled_keyboard_interrupt = 1;
-    }
-    return v;
+    return PyEval_EvalCode((PyObject*)co, globals, locals);
 }
 
 static PyObject *

diff --git a/Tools/c-analyzer/TODO b/Tools/c-analyzer/TODO
index edd0c4bc7fdaa6cd23e67f5013f3bde3311a8037..d509489176b9453819330b818ccd5fc7061ddba0 100644 (file)
--- a/Tools/c-analyzer/TODO
+++ b/Tools/c-analyzer/TODO
@@ -532,7 +532,7 @@ Python/pythonrun.c:PyId_stdin                                    _Py_IDENTIFIER(
 Python/pythonrun.c:PyId_stdout                                   _Py_IDENTIFIER(stdout)
 Python/pythonrun.c:PyRun_InteractiveOneObjectEx():PyId___main__  _Py_IDENTIFIER(__main__)
 Python/pythonrun.c:PyRun_InteractiveOneObjectEx():PyId_encoding  _Py_IDENTIFIER(encoding)
-Python/pythonrun.c:_Py_HandleSystemExit():PyId_code              _Py_IDENTIFIER(code)
+Python/pythonrun.c:_Py_HandleSystemExitAndKeyboardInterrupt():PyId_code _Py_IDENTIFIER(code)
 Python/pythonrun.c:parse_syntax_error():PyId_filename            _Py_IDENTIFIER(filename)
 Python/pythonrun.c:parse_syntax_error():PyId_lineno              _Py_IDENTIFIER(lineno)
 Python/pythonrun.c:parse_syntax_error():PyId_msg                 _Py_IDENTIFIER(msg)