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.
This commit is contained in:
Sam Gross 2025-02-13 12:29:03 -05:00 committed by GitHub
parent aa28423201
commit 451f291baa
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 13 additions and 36 deletions

View file

@ -75,7 +75,7 @@ extern PyStatus _Py_PreInitializeFromConfig(
extern wchar_t * _Py_GetStdlibDir(void); 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); extern PyObject* _PyErr_WriteUnraisableDefaultHook(PyObject *unraisable);

View file

@ -99,7 +99,7 @@ static int
pymain_err_print(int *exitcode_p) pymain_err_print(int *exitcode_p)
{ {
int exitcode; int exitcode;
if (_Py_HandleSystemExit(&exitcode)) { if (_Py_HandleSystemExitAndKeyboardInterrupt(&exitcode)) {
*exitcode_p = exitcode; *exitcode_p = exitcode;
return 1; return 1;
} }
@ -292,11 +292,7 @@ pymain_start_pyrepl_no_main(void)
goto done; goto done;
} }
if (!PyDict_SetItemString(kwargs, "pythonstartup", _PyLong_GetOne())) { if (!PyDict_SetItemString(kwargs, "pythonstartup", _PyLong_GetOne())) {
_PyRuntime.signals.unhandled_keyboard_interrupt = 0;
console_result = PyObject_Call(console, empty_tuple, kwargs); 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) { if (console_result == NULL) {
res = pymain_exit_err_print(); res = pymain_exit_err_print();
} }
@ -338,11 +334,7 @@ pymain_run_module(const wchar_t *modname, int set_argv0)
Py_DECREF(module); Py_DECREF(module);
return pymain_exit_err_print(); return pymain_exit_err_print();
} }
_PyRuntime.signals.unhandled_keyboard_interrupt = 0;
result = PyObject_Call(runmodule, runargs, NULL); result = PyObject_Call(runmodule, runargs, NULL);
if (!result && PyErr_Occurred() == PyExc_KeyboardInterrupt) {
_PyRuntime.signals.unhandled_keyboard_interrupt = 1;
}
Py_DECREF(runmodule); Py_DECREF(runmodule);
Py_DECREF(module); Py_DECREF(module);
Py_DECREF(runargs); Py_DECREF(runargs);
@ -763,6 +755,8 @@ Py_RunMain(void)
{ {
int exitcode = 0; int exitcode = 0;
_PyRuntime.signals.unhandled_keyboard_interrupt = 0;
pymain_run_python(&exitcode); pymain_run_python(&exitcode);
if (Py_FinalizeEx() < 0) { if (Py_FinalizeEx() < 0) {

View file

@ -589,8 +589,13 @@ parse_exit_code(PyObject *code, int *exitcode_p)
} }
int 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; int inspect = _Py_GetConfig()->inspect;
if (inspect) { if (inspect) {
/* Don't exit if -i flag was given. This flag is set to 0 /* Don't exit if -i flag was given. This flag is set to 0
@ -646,7 +651,7 @@ static void
handle_system_exit(void) handle_system_exit(void)
{ {
int exitcode; int exitcode;
if (_Py_HandleSystemExit(&exitcode)) { if (_Py_HandleSystemExitAndKeyboardInterrupt(&exitcode)) {
Py_Exit(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 // Try first with the stdlib traceback module
PyObject *print_exception_fn = PyImport_ImportModuleAttrString( PyObject *print_exception_fn = PyImport_ImportModuleAttrString(
"traceback", "traceback",
@ -1120,11 +1123,9 @@ _PyErr_Display(PyObject *file, PyObject *unused, PyObject *value, PyObject *tb)
Py_XDECREF(print_exception_fn); Py_XDECREF(print_exception_fn);
if (result) { if (result) {
Py_DECREF(result); Py_DECREF(result);
_PyRuntime.signals.unhandled_keyboard_interrupt = unhandled_keyboard_interrupt;
return; return;
} }
fallback: fallback:
_PyRuntime.signals.unhandled_keyboard_interrupt = unhandled_keyboard_interrupt;
#ifdef Py_DEBUG #ifdef Py_DEBUG
if (PyErr_Occurred()) { if (PyErr_Occurred()) {
PyErr_FormatUnraisable( PyErr_FormatUnraisable(
@ -1297,20 +1298,6 @@ flush_io(void)
static PyObject * static PyObject *
run_eval_code_obj(PyThreadState *tstate, PyCodeObject *co, PyObject *globals, PyObject *locals) 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 */ /* Set globals['__builtins__'] if it doesn't exist */
if (!globals || !PyDict_Check(globals)) { if (!globals || !PyDict_Check(globals)) {
PyErr_SetString(PyExc_SystemError, "globals must be a real dict"); 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); return PyEval_EvalCode((PyObject*)co, globals, locals);
if (!v && _PyErr_Occurred(tstate) == PyExc_KeyboardInterrupt) {
_PyRuntime.signals.unhandled_keyboard_interrupt = 1;
}
return v;
} }
static PyObject * static PyObject *

View file

@ -532,7 +532,7 @@ Python/pythonrun.c:PyId_stdin _Py_IDENTIFIER(
Python/pythonrun.c:PyId_stdout _Py_IDENTIFIER(stdout) Python/pythonrun.c:PyId_stdout _Py_IDENTIFIER(stdout)
Python/pythonrun.c:PyRun_InteractiveOneObjectEx():PyId___main__ _Py_IDENTIFIER(__main__) Python/pythonrun.c:PyRun_InteractiveOneObjectEx():PyId___main__ _Py_IDENTIFIER(__main__)
Python/pythonrun.c:PyRun_InteractiveOneObjectEx():PyId_encoding _Py_IDENTIFIER(encoding) 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_filename _Py_IDENTIFIER(filename)
Python/pythonrun.c:parse_syntax_error():PyId_lineno _Py_IDENTIFIER(lineno) Python/pythonrun.c:parse_syntax_error():PyId_lineno _Py_IDENTIFIER(lineno)
Python/pythonrun.c:parse_syntax_error():PyId_msg _Py_IDENTIFIER(msg) Python/pythonrun.c:parse_syntax_error():PyId_msg _Py_IDENTIFIER(msg)