https://github.com/python/cpython/commit/b8998fe2d8249565bf30ce6075ed678e1643f2a4
commit: b8998fe2d8249565bf30ce6075ed678e1643f2a4
branch: main
author: Peter Bierma <zintensity...@gmail.com>
committer: ericsnowcurrently <ericsnowcurren...@gmail.com>
date: 2025-05-21T07:01:25-06:00
summary:
gh-131185: Use a proper thread-local for cached thread states (gh-132510)
Switches over to a _Py_thread_local in place of autoTssKey, and also fixes a
few other checks regarding PyGILState_Ensure after finalization.
Note that this doesn't fix concurrent use of PyGILState_Ensure with
Py_Finalize; I'm pretty sure zapthreads doesn't work at all, and that needs to
be fixed seperately.
files:
A Misc/NEWS.d/next/C_API/2025-04-14-07-41-28.gh-issue-131185.ZCjMHD.rst
M Include/internal/pycore_runtime_init.h
M Include/internal/pycore_runtime_structs.h
M Lib/test/test_embed.py
M Programs/_testembed.c
M Python/pystate.c
M Tools/c-analyzer/cpython/ignored.tsv
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 8e0e330f6605c9..577da65c7cdafa 100644
--- a/Programs/_testembed.c
+++ b/Programs/_testembed.c
@@ -8,6 +8,7 @@
#include <Python.h>
#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 <inttypes.h>
@@ -2312,6 +2313,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.
@@ -2402,7 +2429,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 4757a8c3d1476c..4144e6edefc073 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();
@@ -1669,7 +1560,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);
}
}
@@ -2095,7 +1986,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);
}
@@ -2563,7 +2454,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);
}
}
@@ -2765,7 +2656,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();
@@ -2801,7 +2692,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
}
@@ -2817,11 +2708,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
@@ -2832,16 +2719,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);
}
@@ -2856,12 +2739,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 */
@@ -2901,8 +2789,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
_______________________________________________
Python-checkins mailing list -- python-checkins@python.org
To unsubscribe send an email to python-checkins-le...@python.org
https://mail.python.org/mailman3//lists/python-checkins.python.org
Member address: arch...@mail-archive.com
