Skip to content

Commit

Permalink
gh-59956: Clarify GILState-related Code (gh-101161)
Browse files Browse the repository at this point in the history
The objective of this change is to help make the GILState-related code easier to understand.  This mostly involves moving code around and some semantically equivalent refactors.  However, there are a also a small number of slight changes in structure and behavior:

* tstate_current is moved out of _PyRuntimeState.gilstate
* autoTSSkey is moved out of _PyRuntimeState.gilstate
* autoTSSkey is initialized earlier
* autoTSSkey is re-initialized (after fork) earlier

#59956
  • Loading branch information
ericsnowcurrently authored Jan 19, 2023
1 parent 8a2d4f4 commit 6036c3e
Show file tree
Hide file tree
Showing 10 changed files with 373 additions and 250 deletions.
6 changes: 1 addition & 5 deletions Include/cpython/pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,7 @@ struct _ts {
PyThreadState *next;
PyInterpreterState *interp;

/* Has been initialized to a safe state.
In order to be effective, this must be set to 0 during or right
after allocation. */
int _initialized;
int _status;

int py_recursion_remaining;
int py_recursion_limit;
Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_pylifecycle.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ extern void _PyThread_FiniType(PyInterpreterState *interp);
extern void _Py_Deepfreeze_Fini(void);
extern void _PyArg_Fini(void);

extern PyStatus _PyGILState_Init(_PyRuntimeState *runtime);
extern PyStatus _PyGILState_Init(PyInterpreterState *interp);
extern PyStatus _PyGILState_SetTstate(PyThreadState *tstate);
extern void _PyGILState_Fini(PyInterpreterState *interp);

Expand Down
21 changes: 16 additions & 5 deletions Include/internal/pycore_pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,12 @@ _Py_ThreadCanHandlePendingCalls(void)
static inline PyThreadState*
_PyRuntimeState_GetThreadState(_PyRuntimeState *runtime)
{
return (PyThreadState*)_Py_atomic_load_relaxed(&runtime->gilstate.tstate_current);
return (PyThreadState*)_Py_atomic_load_relaxed(&runtime->tstate_current);
}

/* Get the current Python thread state.
Efficient macro reading directly the 'gilstate.tstate_current' atomic
Efficient macro reading directly the 'tstate_current' atomic
variable. The macro is unsafe: it does not check for error and it can
return NULL.
Expand Down Expand Up @@ -120,7 +120,7 @@ static inline PyInterpreterState* _PyInterpreterState_GET(void) {

// PyThreadState functions

PyAPI_FUNC(void) _PyThreadState_SetCurrent(PyThreadState *tstate);
PyAPI_FUNC(void) _PyThreadState_Bind(PyThreadState *tstate);
// We keep this around exclusively for stable ABI compatibility.
PyAPI_FUNC(void) _PyThreadState_Init(
PyThreadState *tstate);
Expand All @@ -139,17 +139,28 @@ _PyThreadState_UpdateTracingState(PyThreadState *tstate)
}


/* PyThreadState status */

#define PyThreadState_UNINITIALIZED 0
/* Has been initialized to a safe state.
In order to be effective, this must be set to 0 during or right
after allocation. */
#define PyThreadState_INITIALIZED 1
#define PyThreadState_BOUND 2
#define PyThreadState_UNBOUND 3


/* Other */

PyAPI_FUNC(PyThreadState *) _PyThreadState_Swap(
struct _gilstate_runtime_state *gilstate,
_PyRuntimeState *runtime,
PyThreadState *newts);

PyAPI_FUNC(PyStatus) _PyInterpreterState_Enable(_PyRuntimeState *runtime);

#ifdef HAVE_FORK
extern PyStatus _PyInterpreterState_DeleteExceptMain(_PyRuntimeState *runtime);
extern PyStatus _PyGILState_Reinit(_PyRuntimeState *runtime);
extern void _PySignal_AfterFork(void);
#endif

Expand Down
10 changes: 6 additions & 4 deletions Include/internal/pycore_runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,11 @@ struct _gilstate_runtime_state {
/* bpo-26558: Flag to disable PyGILState_Check().
If set to non-zero, PyGILState_Check() always return 1. */
int check_enabled;
/* Assuming the current thread holds the GIL, this is the
PyThreadState for the current thread. */
_Py_atomic_address tstate_current;
/* The single PyInterpreterState used by this process'
GILState implementation
*/
/* TODO: Given interp_main, it may be possible to kill this ref */
PyInterpreterState *autoInterpreterState;
Py_tss_t autoTSSkey;
};

/* Runtime audit hook state */
Expand Down Expand Up @@ -124,6 +120,12 @@ typedef struct pyruntimestate {

unsigned long main_thread;

/* Assuming the current thread holds the GIL, this is the
PyThreadState for the current thread. */
_Py_atomic_address tstate_current;
/* Used for the thread state bound to the current thread. */
Py_tss_t autoTSSkey;

PyWideStringList orig_argv;

struct _parser_runtime_state parser;
Expand Down
6 changes: 3 additions & 3 deletions Include/internal/pycore_runtime_init.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ extern "C" {
until _PyInterpreterState_Enable() is called. */ \
.next_id = -1, \
}, \
/* 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, \
.imports = { \
.lock = { \
Expand All @@ -49,9 +52,6 @@ extern "C" {
}, \
.gilstate = { \
.check_enabled = 1, \
/* A TSS key must be initialized with Py_tss_NEEDS_INIT \
in accordance with the specification. */ \
.autoTSSkey = Py_tss_NEEDS_INIT, \
}, \
.dtoa = _dtoa_runtime_state_INIT(runtime), \
.fileutils = { \
Expand Down
8 changes: 1 addition & 7 deletions Modules/_threadmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -1074,13 +1074,7 @@ thread_run(void *boot_raw)
PyThreadState *tstate;

tstate = boot->tstate;
tstate->thread_id = PyThread_get_thread_ident();
#ifdef PY_HAVE_THREAD_NATIVE_ID
tstate->native_thread_id = PyThread_get_thread_native_id();
#else
tstate->native_thread_id = 0;
#endif
_PyThreadState_SetCurrent(tstate);
_PyThreadState_Bind(tstate);
PyEval_AcquireThread(tstate);
tstate->interp->threads.count++;

Expand Down
7 changes: 1 addition & 6 deletions Modules/posixmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ PyOS_AfterFork_Child(void)
PyStatus status;
_PyRuntimeState *runtime = &_PyRuntime;

status = _PyGILState_Reinit(runtime);
status = _PyRuntimeState_ReInitThreads(runtime);
if (_PyStatus_EXCEPTION(status)) {
goto fatal_error;
}
Expand All @@ -611,11 +611,6 @@ PyOS_AfterFork_Child(void)

_PySignal_AfterFork();

status = _PyRuntimeState_ReInitThreads(runtime);
if (_PyStatus_EXCEPTION(status)) {
goto fatal_error;
}

status = _PyInterpreterState_DeleteExceptMain(runtime);
if (_PyStatus_EXCEPTION(status)) {
goto fatal_error;
Expand Down
14 changes: 6 additions & 8 deletions Python/ceval_gil.c
Original file line number Diff line number Diff line change
Expand Up @@ -581,8 +581,7 @@ PyEval_AcquireThread(PyThreadState *tstate)

take_gil(tstate);

struct _gilstate_runtime_state *gilstate = &tstate->interp->runtime->gilstate;
if (_PyThreadState_Swap(gilstate, tstate) != NULL) {
if (_PyThreadState_Swap(tstate->interp->runtime, tstate) != NULL) {
Py_FatalError("non-NULL old thread state");
}
}
Expand All @@ -593,7 +592,7 @@ PyEval_ReleaseThread(PyThreadState *tstate)
assert(is_tstate_valid(tstate));

_PyRuntimeState *runtime = tstate->interp->runtime;
PyThreadState *new_tstate = _PyThreadState_Swap(&runtime->gilstate, NULL);
PyThreadState *new_tstate = _PyThreadState_Swap(runtime, NULL);
if (new_tstate != tstate) {
Py_FatalError("wrong thread state");
}
Expand Down Expand Up @@ -643,7 +642,7 @@ PyThreadState *
PyEval_SaveThread(void)
{
_PyRuntimeState *runtime = &_PyRuntime;
PyThreadState *tstate = _PyThreadState_Swap(&runtime->gilstate, NULL);
PyThreadState *tstate = _PyThreadState_Swap(runtime, NULL);
_Py_EnsureTstateNotNULL(tstate);

struct _ceval_runtime_state *ceval = &runtime->ceval;
Expand All @@ -660,8 +659,7 @@ PyEval_RestoreThread(PyThreadState *tstate)

take_gil(tstate);

struct _gilstate_runtime_state *gilstate = &tstate->interp->runtime->gilstate;
_PyThreadState_Swap(gilstate, tstate);
_PyThreadState_Swap(tstate->interp->runtime, tstate);
}


Expand Down Expand Up @@ -965,7 +963,7 @@ _Py_HandlePending(PyThreadState *tstate)
/* GIL drop request */
if (_Py_atomic_load_relaxed_int32(&interp_ceval_state->gil_drop_request)) {
/* Give another thread a chance */
if (_PyThreadState_Swap(&runtime->gilstate, NULL) != tstate) {
if (_PyThreadState_Swap(runtime, NULL) != tstate) {
Py_FatalError("tstate mix-up");
}
drop_gil(ceval, interp_ceval_state, tstate);
Expand All @@ -974,7 +972,7 @@ _Py_HandlePending(PyThreadState *tstate)

take_gil(tstate);

if (_PyThreadState_Swap(&runtime->gilstate, tstate) != NULL) {
if (_PyThreadState_Swap(runtime, tstate) != NULL) {
Py_FatalError("orphan tstate");
}
}
Expand Down
19 changes: 9 additions & 10 deletions Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -675,12 +675,7 @@ pycore_create_interpreter(_PyRuntimeState *runtime,
const PyConfig *src_config,
PyThreadState **tstate_p)
{
/* Auto-thread-state API */
PyStatus status = _PyGILState_Init(runtime);
if (_PyStatus_EXCEPTION(status)) {
return status;
}

PyStatus status;
PyInterpreterState *interp = PyInterpreterState_New();
if (interp == NULL) {
return _PyStatus_ERR("can't make main interpreter");
Expand All @@ -692,6 +687,12 @@ pycore_create_interpreter(_PyRuntimeState *runtime,
return status;
}

/* Auto-thread-state API */
status = _PyGILState_Init(interp);
if (_PyStatus_EXCEPTION(status)) {
return status;
}

const _PyInterpreterConfig config = _PyInterpreterConfig_LEGACY_INIT;
init_interp_settings(interp, &config);

Expand Down Expand Up @@ -1795,10 +1796,8 @@ finalize_interp_clear(PyThreadState *tstate)
static void
finalize_interp_delete(PyInterpreterState *interp)
{
if (_Py_IsMainInterpreter(interp)) {
/* Cleanup auto-thread-state */
_PyGILState_Fini(interp);
}
/* Cleanup auto-thread-state */
_PyGILState_Fini(interp);

/* We can't call _PyEval_FiniGIL() here because destroying the GIL lock can
fail when it is being awaited by another running daemon thread (see
Expand Down
Loading

0 comments on commit 6036c3e

Please sign in to comment.