From 4be1f37b20bd51498d3adf8ad603095c0f38d6e5 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 26 Jun 2024 15:17:26 -0600 Subject: [PATCH] gh-113433: Automatically Clean Up Subinterpreters in Py_Finalize() (gh-121060) This change makes things a little less painful for some users. It also fixes a failing assert (gh-120765), by making sure all subinterpreters are destroyed before the main interpreter. As part of that, we make sure Py_Finalize() always runs with the main interpreter active. --- ...-06-26-13-42-36.gh-issue-113433.xKAtLB.rst | 2 + ...-06-26-14-09-31.gh-issue-120838.nFeTL9.rst | 2 + Python/pylifecycle.c | 156 +++++++++++++++++- 3 files changed, 151 insertions(+), 9 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-06-26-13-42-36.gh-issue-113433.xKAtLB.rst create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-06-26-14-09-31.gh-issue-120838.nFeTL9.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-06-26-13-42-36.gh-issue-113433.xKAtLB.rst b/Misc/NEWS.d/next/Core and Builtins/2024-06-26-13-42-36.gh-issue-113433.xKAtLB.rst new file mode 100644 index 00000000000..bf8377ac488 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-06-26-13-42-36.gh-issue-113433.xKAtLB.rst @@ -0,0 +1,2 @@ +Subinterpreters now get cleaned up automatically during runtime +finalization. diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-06-26-14-09-31.gh-issue-120838.nFeTL9.rst b/Misc/NEWS.d/next/Core and Builtins/2024-06-26-14-09-31.gh-issue-120838.nFeTL9.rst new file mode 100644 index 00000000000..057d00aeeab --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-06-26-14-09-31.gh-issue-120838.nFeTL9.rst @@ -0,0 +1,2 @@ +:c:func:`Py_Finalize()` and :c:func:`Py_FinalizeEx()` now always run with +the main interpreter active. diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index cae1f8b61c8..9bbfa831db3 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -73,6 +73,7 @@ static PyStatus init_sys_streams(PyThreadState *tstate); static PyStatus init_android_streams(PyThreadState *tstate); #endif static void wait_for_thread_shutdown(PyThreadState *tstate); +static void finalize_subinterpreters(void); static void call_ll_exitfuncs(_PyRuntimeState *runtime); /* The following places the `_PyRuntime` structure in a location that can be @@ -1907,20 +1908,73 @@ finalize_interp_delete(PyInterpreterState *interp) } -int -Py_FinalizeEx(void) +/* Conceptually, there isn't a good reason for Py_Finalize() + to be called in any other thread than the one where Py_Initialize() + was called. Consequently, it would make sense to fail if the thread + or thread state (or interpreter) don't match. However, such + constraints have never been enforced, and, as unlikely as it may be, + there may be users relying on the unconstrained behavior. Thus, + we do our best here to accommodate that possibility. */ + +static PyThreadState * +resolve_final_tstate(_PyRuntimeState *runtime) +{ + PyThreadState *main_tstate = runtime->main_tstate; + assert(main_tstate != NULL); + assert(main_tstate->thread_id == runtime->main_thread); + PyInterpreterState *main_interp = _PyInterpreterState_Main(); + assert(main_tstate->interp == main_interp); + + PyThreadState *tstate = _PyThreadState_GET(); + if (_Py_IsMainThread()) { + if (tstate != main_tstate) { + /* This implies that Py_Finalize() was called while + a non-main interpreter was active or while the main + tstate was temporarily swapped out with another. + Neither case should be allowed, but, until we get around + to fixing that (and Py_Exit()), we're letting it go. */ + (void)PyThreadState_Swap(main_tstate); + } + } + else { + /* This is another unfortunate case where Py_Finalize() was + called when it shouldn't have been. We can't simply switch + over to the main thread. At the least, however, we can make + sure the main interpreter is active. */ + if (!_Py_IsMainInterpreter(tstate->interp)) { + /* We don't go to the trouble of updating runtime->main_tstate + since it will be dead soon anyway. */ + main_tstate = + _PyThreadState_New(main_interp, _PyThreadState_WHENCE_FINI); + if (main_tstate != NULL) { + _PyThreadState_Bind(main_tstate); + (void)PyThreadState_Swap(main_tstate); + } + else { + /* Fall back to the current tstate. It's better than nothing. */ + main_tstate = tstate; + } + } + } + assert(main_tstate != NULL); + + /* We might want to warn if main_tstate->current_frame != NULL. */ + + return main_tstate; +} + +static int +_Py_Finalize(_PyRuntimeState *runtime) { int status = 0; - _PyRuntimeState *runtime = &_PyRuntime; + /* Bail out early if already finalized (or never initialized). */ if (!runtime->initialized) { return status; } - /* Get current thread state and interpreter pointer */ - PyThreadState *tstate = _PyThreadState_GET(); - // XXX assert(_Py_IsMainInterpreter(tstate->interp)); - // XXX assert(_Py_IsMainThread()); + /* Get final thread state pointer. */ + PyThreadState *tstate = resolve_final_tstate(runtime); // Block some operations. tstate->interp->finalizing = 1; @@ -1943,6 +1997,8 @@ Py_FinalizeEx(void) _PyAtExit_Call(tstate->interp); + assert(_PyThreadState_GET() == tstate); + /* Copy the core config, PyInterpreterState_Delete() free the core config memory */ #ifdef Py_REF_DEBUG @@ -2023,6 +2079,9 @@ Py_FinalizeEx(void) _PyImport_FiniExternal(tstate->interp); finalize_modules(tstate); + /* Clean up any lingering subinterpreters. */ + finalize_subinterpreters(); + /* Print debug stats if any */ _PyEval_Fini(); @@ -2140,10 +2199,16 @@ Py_FinalizeEx(void) return status; } +int +Py_FinalizeEx(void) +{ + return _Py_Finalize(&_PyRuntime); +} + void Py_Finalize(void) { - Py_FinalizeEx(); + (void)_Py_Finalize(&_PyRuntime); } @@ -2355,6 +2420,79 @@ _Py_IsInterpreterFinalizing(PyInterpreterState *interp) return finalizing != NULL; } +static void +finalize_subinterpreters(void) +{ + PyThreadState *final_tstate = _PyThreadState_GET(); + PyInterpreterState *main_interp = _PyInterpreterState_Main(); + assert(final_tstate->interp == main_interp); + _PyRuntimeState *runtime = main_interp->runtime; + struct pyinterpreters *interpreters = &runtime->interpreters; + + /* Get the first interpreter in the list. */ + HEAD_LOCK(runtime); + PyInterpreterState *interp = interpreters->head; + if (interp == main_interp) { + interp = interp->next; + } + HEAD_UNLOCK(runtime); + + /* Bail out if there are no subinterpreters left. */ + if (interp == NULL) { + return; + } + + /* Warn the user if they forgot to clean up subinterpreters. */ + (void)PyErr_WarnEx( + PyExc_RuntimeWarning, + "remaining subinterpreters; " + "destroy them with _interpreters.destroy()", + 0); + + /* Swap out the current tstate, which we know must belong + to the main interpreter. */ + _PyThreadState_Detach(final_tstate); + + /* Clean up all remaining subinterpreters. */ + while (interp != NULL) { + assert(!_PyInterpreterState_IsRunningMain(interp)); + + /* Find the tstate to use for fini. We assume the interpreter + will have at most one tstate at this point. */ + PyThreadState *tstate = interp->threads.head; + if (tstate != NULL) { + /* Ideally we would be able to use tstate as-is, and rely + on it being in a ready state: no exception set, not + running anything (tstate->current_frame), matching the + current thread ID (tstate->thread_id). To play it safe, + we always delete it and use a fresh tstate instead. */ + assert(tstate != final_tstate); + _PyThreadState_Attach(tstate); + PyThreadState_Clear(tstate); + _PyThreadState_Detach(tstate); + PyThreadState_Delete(tstate); + } + tstate = _PyThreadState_NewBound(interp, _PyThreadState_WHENCE_FINI); + + /* Destroy the subinterpreter. */ + _PyThreadState_Attach(tstate); + Py_EndInterpreter(tstate); + assert(_PyThreadState_GET() == NULL); + + /* Advance to the next interpreter. */ + HEAD_LOCK(runtime); + interp = interpreters->head; + if (interp == main_interp) { + interp = interp->next; + } + HEAD_UNLOCK(runtime); + } + + /* Switch back to the main interpreter. */ + _PyThreadState_Attach(final_tstate); +} + + /* Add the __main__ module */ static PyStatus @@ -3216,7 +3354,7 @@ Py_Exit(int sts) if (tstate != NULL && _PyThreadState_IsRunningMain(tstate)) { _PyInterpreterState_SetNotRunningMain(tstate->interp); } - if (Py_FinalizeEx() < 0) { + if (_Py_Finalize(&_PyRuntime) < 0) { sts = 120; }