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.
This commit is contained in:
Peter Bierma 2025-05-21 09:01:25 -04:00 committed by GitHub
parent dcfc91e4e5
commit b8998fe2d8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 81 additions and 166 deletions

View file

@ -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 */

View file

@ -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;

View file

@ -1915,6 +1915,10 @@ class AuditingTests(EmbeddingTestsMixin, unittest.TestCase):
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

View file

@ -0,0 +1,2 @@
:c:func:`PyGILState_Ensure` no longer crashes when called after interpreter
finalization.

View file

@ -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}
};

View file

@ -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 void
gilstate_tss_set(_PyRuntimeState *runtime, PyThreadState *tstate)
static inline PyThreadState *
gilstate_get(void)
{
assert(tstate != NULL && tstate->interp->runtime == runtime);
if (_gilstate_tss_set(runtime, tstate) != 0) {
Py_FatalError("failed to set current tstate (TSS)");
}
return _Py_tss_gilstate;
}
static inline void
gilstate_tss_clear(_PyRuntimeState *runtime)
gilstate_set(PyThreadState *tstate)
{
if (_gilstate_tss_clear(runtime) != 0) {
Py_FatalError("failed to clear current tstate (TSS)");
}
assert(tstate != NULL);
_Py_tss_gilstate = tstate;
}
static inline void
gilstate_clear(void)
{
_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");

View file

@ -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

Can't render this file because it has a wrong number of fields in line 4.