diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index fade55945b7dbf..f9dc61a1ababe8 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -244,6 +244,9 @@ extern PyThreadState * _PyThreadState_Swap( extern PyStatus _PyInterpreterState_Enable(_PyRuntimeState *runtime); +extern PyThreadState * +_PyThreadState_SwapAttached(PyThreadState *tstate); + #ifdef HAVE_FORK extern PyStatus _PyInterpreterState_DeleteExceptMain(_PyRuntimeState *runtime); extern void _PySignal_AfterFork(void); diff --git a/Lib/test/test_interpreters/test_lifecycle.py b/Lib/test/test_interpreters/test_lifecycle.py index ac24f6568acd95..7f511cdebce4a4 100644 --- a/Lib/test/test_interpreters/test_lifecycle.py +++ b/Lib/test/test_interpreters/test_lifecycle.py @@ -184,6 +184,33 @@ def test_gh_109793(self): print("------") self.assertEqual(proc.returncode, 1) + @support.requires_subprocess() + @unittest.skipIf(sys.platform == "win32", "SIGINT does not work on Windows") + def test_interrupt_thread_with_interpreter(self): + # See GH-126016: Subinterpreters that are created + # in another thread might not have their thread states + # cleaned up if the user interrupted joining with CTRL+C. + import subprocess + import signal + + with subprocess.Popen([ + sys.executable, "-c", + "import threading, _interpreters\n" + "def run_interp(): _interpreters.run_string(_interpreters.create()," + "'import time; print(1, flush=True); time.sleep(5)')\n" + "threading.Thread(target=run_interp).start()" + ], stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) as proc: + with proc.stdout: + # Make sure that the thread has actually started + self.assertEqual(proc.stdout.read(1), "1") + + # Send a KeyboardInterrupt to the process + proc.send_signal(signal.SIGINT) + with proc.stderr: + self.assertIn("KeyboardInterrupt", proc.stderr.read()) + proc.wait() + # Make sure it didn't segfault + self.assertEqual(proc.returncode, 0) if __name__ == '__main__': # Test needs to be a package, so we can do relative imports. diff --git a/Misc/NEWS.d/next/Library/2024-10-26-22-46-50.gh-issue-126016.gUWIIK.rst b/Misc/NEWS.d/next/Library/2024-10-26-22-46-50.gh-issue-126016.gUWIIK.rst new file mode 100644 index 00000000000000..43a0bdc30d52ac --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-10-26-22-46-50.gh-issue-126016.gUWIIK.rst @@ -0,0 +1 @@ +Fixed crash upon using CTRL+C while joining a thread containing a subinterpreter. diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 8f38fbedae9842..bbf9fa56f881bf 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -2477,7 +2477,28 @@ finalize_subinterpreters(void) /* Clean up all remaining subinterpreters. */ while (interp != NULL) { - assert(!_PyInterpreterState_IsRunningMain(interp)); + if (_PyInterpreterState_IsRunningMain(interp)) + { + /* + * GH-126016: If a subinterpreter was running in another + * thread, and the user interrupted the joining of that thread + * with CTRL+C, then a thread state for this interpreter is still + * active. We need to destroy it before proceeding. + */ + // XXX Are there more threads we have to worry about? + PyThreadState *to_destroy = interp->threads.main; + + // We have to use SwapAttached to attach to an attached thread state. + // If the thread were really running, this would be a thread safety headache. + // Luckily, we know it isn't. + _PyThreadState_SwapAttached(to_destroy); + _PyInterpreterState_SetNotRunningMain(interp); + PyThreadState_Clear(to_destroy); + + // Back to a NULL tstate + PyThreadState_Swap(NULL); + PyThreadState_Delete(to_destroy); + } /* Find the tstate to use for fini. We assume the interpreter will have at most one tstate at this point. */ diff --git a/Python/pystate.c b/Python/pystate.c index 36b31f3b9e4200..09708e64eadb97 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -2406,6 +2406,22 @@ PyThreadState_Swap(PyThreadState *newts) return _PyThreadState_Swap(&_PyRuntime, newts); } +/* + * Calls PyThreadState_Swap() on the a bound thread state. + * This breaks the GIL, so it should only be used if certain that + * it's impossible for the thread to be running code. + */ +PyThreadState * +_PyThreadState_SwapAttached(PyThreadState *tstate) +{ + // Evil thread state magic: we manually + // mark it as unbound so we can re-attach it + // to current thread. + tstate->_status.bound_gilstate = 0; + tstate->_status.holds_gil = 0; + tstate->_status.active = 0; + return PyThreadState_Swap(tstate); +} void _PyThreadState_Bind(PyThreadState *tstate)