Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-126016: Force crossinterpreter thread states to get cleaned up #126026

Closed
wants to merge 11 commits into from
3 changes: 3 additions & 0 deletions Include/internal/pycore_pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
27 changes: 27 additions & 0 deletions Lib/test/test_interpreters/test_lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed crash upon using CTRL+C while joining a thread containing a subinterpreter.
ZeroIntensity marked this conversation as resolved.
Show resolved Hide resolved
23 changes: 22 additions & 1 deletion Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
16 changes: 16 additions & 0 deletions Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading