From df63d15cf5ca3af06ef87bba488cf6c998afd125 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 25 Jun 2024 14:36:03 -0600 Subject: [PATCH 1/3] Factor out resolve_final_tstate() and _Py_Finalize(). --- Python/pylifecycle.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 9b599cba241445..3873ece12baabc 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1908,20 +1908,28 @@ finalize_interp_delete(PyInterpreterState *interp) } -int -Py_FinalizeEx(void) +static PyThreadState * +resolve_final_tstate(_PyRuntimeState *runtime) +{ + PyThreadState *tstate = _PyThreadState_GET(); + // XXX assert(_Py_IsMainInterpreter(tstate->interp)); + // XXX assert(_Py_IsMainThread()); + + return 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; @@ -2141,10 +2149,16 @@ Py_FinalizeEx(void) return status; } +int +Py_FinalizeEx(void) +{ + return _Py_Finalize(&_PyRuntime); +} + void Py_Finalize(void) { - Py_FinalizeEx(); + (void)_Py_Finalize(&_PyRuntime); } @@ -3217,7 +3231,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; } From f6df8bed90dff9f91aab476ea55091aab2662467 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 25 Jun 2024 14:36:44 -0600 Subject: [PATCH 2/3] Make sure the main interpreter is active in _Py_Finalize(). --- Python/pylifecycle.c | 51 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 3873ece12baabc..2b4c01b1817ee6 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1908,14 +1908,59 @@ finalize_interp_delete(PyInterpreterState *interp) } +/* 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(); - // XXX assert(_Py_IsMainInterpreter(tstate->interp)); - // XXX assert(_Py_IsMainThread()); + 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); - return tstate; + /* We might want to warn if main_tstate->current_frame != NULL. */ + + return main_tstate; } static int From 071d270ca09b69a6e1468cf173121ec146f37404 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 26 Jun 2024 14:09:40 -0600 Subject: [PATCH 3/3] Add a NEWS entry. --- .../2024-06-26-14-09-31.gh-issue-120838.nFeTL9.rst | 2 ++ 1 file changed, 2 insertions(+) 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-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 00000000000000..057d00aeeaba11 --- /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.