diff --git a/Include/internal/pycore_runtime_init.h b/Include/internal/pycore_runtime_init.h index 4200d91a2fcd9d..b182f7825a2326 100644 --- a/Include/internal/pycore_runtime_init.h +++ b/Include/internal/pycore_runtime_init.h @@ -61,9 +61,6 @@ extern PyTypeObject _PyExc_MemoryError; }, \ }, \ }, \ - /* A TSS key must be initialized with Py_tss_NEEDS_INIT \ - in accordance with the specification. */ \ - .autoTSSkey = Py_tss_NEEDS_INIT, \ .parser = _parser_runtime_state_INIT, \ .ceval = { \ .pending_mainthread = { \ @@ -236,4 +233,4 @@ extern PyTypeObject _PyExc_MemoryError; #ifdef __cplusplus } #endif -#endif /* !Py_INTERNAL_RUNTIME_INIT_H */ +#endif /* !Py_INTERNAL_RUNTIME_INIT_H */ \ No newline at end of file diff --git a/Include/internal/pycore_runtime_structs.h b/Include/internal/pycore_runtime_structs.h index 6bf3aae7175a97..12164c7fdd9425 100644 --- a/Include/internal/pycore_runtime_structs.h +++ b/Include/internal/pycore_runtime_structs.h @@ -223,9 +223,6 @@ struct pyruntimestate { struct _pythread_runtime_state threads; struct _signals_runtime_state signals; - /* Used for the thread state bound to the current thread. */ - Py_tss_t autoTSSkey; - /* Used instead of PyThreadState.trash when there is not current tstate. */ Py_tss_t trashTSSkey; diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py index 46222e521aead8..89f4aebe28f4a1 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -1915,6 +1915,10 @@ def test_get_incomplete_frame(self): self.run_embedded_interpreter("test_get_incomplete_frame") + def test_gilstate_after_finalization(self): + self.run_embedded_interpreter("test_gilstate_after_finalization") + + class MiscTests(EmbeddingTestsMixin, unittest.TestCase): def test_unicode_id_init(self): # bpo-42882: Test that _PyUnicode_FromId() works diff --git a/Misc/NEWS.d/next/C_API/2025-04-14-07-41-28.gh-issue-131185.ZCjMHD.rst b/Misc/NEWS.d/next/C_API/2025-04-14-07-41-28.gh-issue-131185.ZCjMHD.rst new file mode 100644 index 00000000000000..aa0e8bca93b46f --- /dev/null +++ b/Misc/NEWS.d/next/C_API/2025-04-14-07-41-28.gh-issue-131185.ZCjMHD.rst @@ -0,0 +1,2 @@ +:c:func:`PyGILState_Ensure` no longer crashes when called after interpreter +finalization. diff --git a/Programs/_testembed.c b/Programs/_testembed.c index 0a19d1126e246e..79c3a3f6facf92 100644 --- a/Programs/_testembed.c +++ b/Programs/_testembed.c @@ -8,6 +8,7 @@ #include #include "pycore_initconfig.h" // _PyConfig_InitCompatConfig() #include "pycore_runtime.h" // _PyRuntime +#include "pycore_lock.h" // PyEvent #include "pycore_pythread.h" // PyThread_start_joinable_thread() #include "pycore_import.h" // _PyImport_FrozenBootstrap #include @@ -2315,6 +2316,32 @@ test_get_incomplete_frame(void) return result; } +static void +do_gilstate_ensure(void *event_ptr) +{ + PyEvent *event = (PyEvent *)event_ptr; + // Signal to the calling thread that we've started + _PyEvent_Notify(event); + PyGILState_Ensure(); // This should hang + assert(NULL); +} + +static int +test_gilstate_after_finalization(void) +{ + _testembed_initialize(); + Py_Finalize(); + PyThread_handle_t handle; + PyThread_ident_t ident; + PyEvent event = {0}; + if (PyThread_start_joinable_thread(&do_gilstate_ensure, &event, &ident, &handle) < 0) { + return -1; + } + PyEvent_Wait(&event); + // We're now pretty confident that the thread went for + // PyGILState_Ensure(), but that means it got hung. + return PyThread_detach_thread(handle); +} /* ********************************************************* * List of test cases and the function that implements it. @@ -2405,7 +2432,7 @@ static struct TestCase TestCases[] = { {"test_frozenmain", test_frozenmain}, #endif {"test_get_incomplete_frame", test_get_incomplete_frame}, - + {"test_gilstate_after_finalization", test_gilstate_after_finalization}, {NULL, NULL} }; diff --git a/Python/pystate.c b/Python/pystate.c index 14ae2748b0bc99..3657d958ce12d6 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -69,7 +69,12 @@ to avoid the expense of doing their own locking). #ifdef HAVE_THREAD_LOCAL +/* The attached thread state for the current thread. */ _Py_thread_local PyThreadState *_Py_tss_tstate = NULL; + +/* The "bound" thread state used by PyGILState_Ensure(), + also known as a "gilstate." */ +_Py_thread_local PyThreadState *_Py_tss_gilstate = NULL; #endif static inline PyThreadState * @@ -118,79 +123,9 @@ _PyThreadState_GetCurrent(void) } -//------------------------------------------------ -// the thread state bound to the current OS thread -//------------------------------------------------ - -static inline int -tstate_tss_initialized(Py_tss_t *key) -{ - return PyThread_tss_is_created(key); -} - -static inline int -tstate_tss_init(Py_tss_t *key) -{ - assert(!tstate_tss_initialized(key)); - return PyThread_tss_create(key); -} - -static inline void -tstate_tss_fini(Py_tss_t *key) -{ - assert(tstate_tss_initialized(key)); - PyThread_tss_delete(key); -} - -static inline PyThreadState * -tstate_tss_get(Py_tss_t *key) -{ - assert(tstate_tss_initialized(key)); - return (PyThreadState *)PyThread_tss_get(key); -} - -static inline int -tstate_tss_set(Py_tss_t *key, PyThreadState *tstate) -{ - assert(tstate != NULL); - assert(tstate_tss_initialized(key)); - return PyThread_tss_set(key, (void *)tstate); -} - -static inline int -tstate_tss_clear(Py_tss_t *key) -{ - assert(tstate_tss_initialized(key)); - return PyThread_tss_set(key, (void *)NULL); -} - -#ifdef HAVE_FORK -/* Reset the TSS key - called by PyOS_AfterFork_Child(). - * This should not be necessary, but some - buggy - pthread implementations - * don't reset TSS upon fork(), see issue #10517. - */ -static PyStatus -tstate_tss_reinit(Py_tss_t *key) -{ - if (!tstate_tss_initialized(key)) { - return _PyStatus_OK(); - } - PyThreadState *tstate = tstate_tss_get(key); - - tstate_tss_fini(key); - if (tstate_tss_init(key) != 0) { - return _PyStatus_NO_MEMORY(); - } - - /* If the thread had an associated auto thread state, reassociate it with - * the new key. */ - if (tstate && tstate_tss_set(key, tstate) != 0) { - return _PyStatus_ERR("failed to re-set autoTSSkey"); - } - return _PyStatus_OK(); -} -#endif - +//--------------------------------------------- +// The thread state used by PyGILState_Ensure() +//--------------------------------------------- /* The stored thread state is set by bind_tstate() (AKA PyThreadState_Bind(). @@ -198,36 +133,23 @@ tstate_tss_reinit(Py_tss_t *key) The GIL does no need to be held for these. */ -#define gilstate_tss_initialized(runtime) \ - tstate_tss_initialized(&(runtime)->autoTSSkey) -#define gilstate_tss_init(runtime) \ - tstate_tss_init(&(runtime)->autoTSSkey) -#define gilstate_tss_fini(runtime) \ - tstate_tss_fini(&(runtime)->autoTSSkey) -#define gilstate_tss_get(runtime) \ - tstate_tss_get(&(runtime)->autoTSSkey) -#define _gilstate_tss_set(runtime, tstate) \ - tstate_tss_set(&(runtime)->autoTSSkey, tstate) -#define _gilstate_tss_clear(runtime) \ - tstate_tss_clear(&(runtime)->autoTSSkey) -#define gilstate_tss_reinit(runtime) \ - tstate_tss_reinit(&(runtime)->autoTSSkey) +static inline PyThreadState * +gilstate_get(void) +{ + return _Py_tss_gilstate; +} static inline void -gilstate_tss_set(_PyRuntimeState *runtime, PyThreadState *tstate) +gilstate_set(PyThreadState *tstate) { - assert(tstate != NULL && tstate->interp->runtime == runtime); - if (_gilstate_tss_set(runtime, tstate) != 0) { - Py_FatalError("failed to set current tstate (TSS)"); - } + assert(tstate != NULL); + _Py_tss_gilstate = tstate; } static inline void -gilstate_tss_clear(_PyRuntimeState *runtime) +gilstate_clear(void) { - if (_gilstate_tss_clear(runtime) != 0) { - Py_FatalError("failed to clear current tstate (TSS)"); - } + _Py_tss_gilstate = NULL; } @@ -253,7 +175,7 @@ bind_tstate(PyThreadState *tstate) assert(tstate_is_alive(tstate) && !tstate->_status.bound); assert(!tstate->_status.unbound); // just in case assert(!tstate->_status.bound_gilstate); - assert(tstate != gilstate_tss_get(tstate->interp->runtime)); + assert(tstate != gilstate_get()); assert(!tstate->_status.active); assert(tstate->thread_id == 0); assert(tstate->native_thread_id == 0); @@ -328,14 +250,13 @@ bind_gilstate_tstate(PyThreadState *tstate) // XXX assert(!tstate->_status.active); assert(!tstate->_status.bound_gilstate); - _PyRuntimeState *runtime = tstate->interp->runtime; - PyThreadState *tcur = gilstate_tss_get(runtime); + PyThreadState *tcur = gilstate_get(); assert(tstate != tcur); if (tcur != NULL) { tcur->_status.bound_gilstate = 0; } - gilstate_tss_set(runtime, tstate); + gilstate_set(tstate); tstate->_status.bound_gilstate = 1; } @@ -347,9 +268,8 @@ unbind_gilstate_tstate(PyThreadState *tstate) assert(tstate_is_bound(tstate)); // XXX assert(!tstate->_status.active); assert(tstate->_status.bound_gilstate); - assert(tstate == gilstate_tss_get(tstate->interp->runtime)); - - gilstate_tss_clear(tstate->interp->runtime); + assert(tstate == gilstate_get()); + gilstate_clear(); tstate->_status.bound_gilstate = 0; } @@ -373,7 +293,7 @@ holds_gil(PyThreadState *tstate) // (and tstate->interp->runtime->ceval.gil.locked). assert(tstate != NULL); /* Must be the tstate for this thread */ - assert(tstate == gilstate_tss_get(tstate->interp->runtime)); + assert(tstate == gilstate_get()); return tstate == current_fast_get(); } @@ -469,16 +389,6 @@ _PyRuntimeState_Init(_PyRuntimeState *runtime) return status; } - if (gilstate_tss_init(runtime) != 0) { - _PyRuntimeState_Fini(runtime); - return _PyStatus_NO_MEMORY(); - } - - if (PyThread_tss_create(&runtime->trashTSSkey) != 0) { - _PyRuntimeState_Fini(runtime); - return _PyStatus_NO_MEMORY(); - } - init_runtime(runtime, open_code_hook, open_code_userdata, audit_hook_head, unicode_next_index); @@ -492,14 +402,7 @@ _PyRuntimeState_Fini(_PyRuntimeState *runtime) /* The count is cleared by _Py_FinalizeRefTotal(). */ assert(runtime->object_state.interpreter_leaks == 0); #endif - - if (gilstate_tss_initialized(runtime)) { - gilstate_tss_fini(runtime); - } - - if (PyThread_tss_is_created(&runtime->trashTSSkey)) { - PyThread_tss_delete(&runtime->trashTSSkey); - } + gilstate_clear(); } #ifdef HAVE_FORK @@ -532,18 +435,6 @@ _PyRuntimeState_ReInitThreads(_PyRuntimeState *runtime) _PyTypes_AfterFork(); - PyStatus status = gilstate_tss_reinit(runtime); - if (_PyStatus_EXCEPTION(status)) { - return status; - } - - if (PyThread_tss_is_created(&runtime->trashTSSkey)) { - PyThread_tss_delete(&runtime->trashTSSkey); - } - if (PyThread_tss_create(&runtime->trashTSSkey) != 0) { - return _PyStatus_NO_MEMORY(); - } - _PyThread_AfterFork(&runtime->threads); return _PyStatus_OK(); @@ -1671,7 +1562,7 @@ _PyThreadState_NewBound(PyInterpreterState *interp, int whence) bind_tstate(tstate); // This makes sure there's a gilstate tstate bound // as soon as possible. - if (gilstate_tss_get(tstate->interp->runtime) == NULL) { + if (gilstate_get() == NULL) { bind_gilstate_tstate(tstate); } } @@ -2097,7 +1988,7 @@ tstate_activate(PyThreadState *tstate) assert(!tstate->_status.active); assert(!tstate->_status.bound_gilstate || - tstate == gilstate_tss_get((tstate->interp->runtime))); + tstate == gilstate_get()); if (!tstate->_status.bound_gilstate) { bind_gilstate_tstate(tstate); } @@ -2565,7 +2456,7 @@ _PyThreadState_Bind(PyThreadState *tstate) bind_tstate(tstate); // This makes sure there's a gilstate tstate bound // as soon as possible. - if (gilstate_tss_get(tstate->interp->runtime) == NULL) { + if (gilstate_get() == NULL) { bind_gilstate_tstate(tstate); } } @@ -2767,7 +2658,7 @@ _PyGILState_Init(PyInterpreterState *interp) return _PyStatus_OK(); } _PyRuntimeState *runtime = interp->runtime; - assert(gilstate_tss_get(runtime) == NULL); + assert(gilstate_get() == NULL); assert(runtime->gilstate.autoInterpreterState == NULL); runtime->gilstate.autoInterpreterState = interp; return _PyStatus_OK(); @@ -2803,7 +2694,7 @@ _PyGILState_SetTstate(PyThreadState *tstate) _PyRuntimeState *runtime = tstate->interp->runtime; assert(runtime->gilstate.autoInterpreterState == tstate->interp); - assert(gilstate_tss_get(runtime) == tstate); + assert(gilstate_get() == tstate); assert(tstate->gilstate_counter == 1); #endif } @@ -2819,11 +2710,7 @@ _PyGILState_GetInterpreterStateUnsafe(void) PyThreadState * PyGILState_GetThisThreadState(void) { - _PyRuntimeState *runtime = &_PyRuntime; - if (!gilstate_tss_initialized(runtime)) { - return NULL; - } - return gilstate_tss_get(runtime); + return gilstate_get(); } int @@ -2834,16 +2721,12 @@ PyGILState_Check(void) return 1; } - if (!gilstate_tss_initialized(runtime)) { - return 1; - } - PyThreadState *tstate = current_fast_get(); if (tstate == NULL) { return 0; } - PyThreadState *tcur = gilstate_tss_get(runtime); + PyThreadState *tcur = gilstate_get(); return (tstate == tcur); } @@ -2858,12 +2741,17 @@ PyGILState_Ensure(void) called Py_Initialize(). */ /* Ensure that _PyEval_InitThreads() and _PyGILState_Init() have been - called by Py_Initialize() */ - assert(_PyEval_ThreadsInitialized()); - assert(gilstate_tss_initialized(runtime)); - assert(runtime->gilstate.autoInterpreterState != NULL); + called by Py_Initialize() - PyThreadState *tcur = gilstate_tss_get(runtime); + TODO: This isn't thread-safe. There's no protection here against + concurrent finalization of the interpreter; it's simply a guard + for *after* the interpreter has finalized. + */ + if (!_PyEval_ThreadsInitialized() || runtime->gilstate.autoInterpreterState == NULL) { + PyThread_hang_thread(); + } + + PyThreadState *tcur = gilstate_get(); int has_gil; if (tcur == NULL) { /* Create a new Python thread state for this thread */ @@ -2903,8 +2791,7 @@ PyGILState_Ensure(void) void PyGILState_Release(PyGILState_STATE oldstate) { - _PyRuntimeState *runtime = &_PyRuntime; - PyThreadState *tstate = gilstate_tss_get(runtime); + PyThreadState *tstate = gilstate_get(); if (tstate == NULL) { Py_FatalError("auto-releasing thread-state, " "but no thread-state for this thread"); diff --git a/Tools/c-analyzer/cpython/ignored.tsv b/Tools/c-analyzer/cpython/ignored.tsv index b128abca39fb41..15b18f5286b399 100644 --- a/Tools/c-analyzer/cpython/ignored.tsv +++ b/Tools/c-analyzer/cpython/ignored.tsv @@ -191,6 +191,7 @@ Python/pyfpe.c - PyFPE_counter - Python/import.c - pkgcontext - Python/pystate.c - _Py_tss_tstate - +Python/pystate.c - _Py_tss_gilstate - ##----------------------- ## should be const