Skip to content

Commit

Permalink
gh-59956: Partial Fix for GILState API Compatibility with Subinterpre…
Browse files Browse the repository at this point in the history
…ters (gh-101431)

The GILState API (PEP 311) implementation from 2003 made the assumption that only one thread state would ever be used for any given OS thread, explicitly disregarding the case of subinterpreters.  However, PyThreadState_Swap() still facilitated switching between subinterpreters, meaning the "current" thread state (holding the GIL), and the GILState thread state could end up out of sync, causing problems (including crashes).

This change addresses the issue by keeping the two in sync in PyThreadState_Swap().  I verified the fix against gh-99040.

Note that the other GILState-subinterpreter incompatibility (with autoInterpreterState) is not resolved here.

#59956
  • Loading branch information
ericsnowcurrently authored Feb 6, 2023
1 parent 262003f commit 132b3f8
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 21 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
The GILState API is now partially compatible with subinterpreters.
Previously, ``PyThreadState_GET()`` and ``PyGILState_GetThisThreadState()``
would get out of sync, causing inconsistent behavior and crashes.
25 changes: 4 additions & 21 deletions Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -266,10 +266,10 @@ unbind_tstate(PyThreadState *tstate)
thread state for an OS level thread is when there are multiple
interpreters.
The PyGILState_*() APIs don't work with multiple
interpreters (see bpo-10915 and bpo-15751), so this function
sets TSS only once. Thus, the first thread state created for that
given OS level thread will "win", which seems reasonable behaviour.
Before 3.12, the PyGILState_*() APIs didn't work with multiple
interpreters (see bpo-10915 and bpo-15751), so this function used
to set TSS only once. Thus, the first thread state created for that
given OS level thread would "win", which seemed reasonable behaviour.
*/

static void
Expand All @@ -286,10 +286,6 @@ bind_gilstate_tstate(PyThreadState *tstate)
assert(tstate != tcur);

if (tcur != NULL) {
// The original gilstate implementation only respects the
// first thread state set.
// XXX Skipping like this does not play nice with multiple interpreters.
return;
tcur->_status.bound_gilstate = 0;
}
gilstate_tss_set(runtime, tstate);
Expand Down Expand Up @@ -1738,20 +1734,7 @@ _PyThreadState_Swap(_PyRuntimeState *runtime, PyThreadState *newts)
tstate_activate(newts);
}

/* It should not be possible for more than one thread state
to be used for a thread. Check this the best we can in debug
builds.
*/
// XXX The above isn't true when multiple interpreters are involved.
#if defined(Py_DEBUG)
if (newts && gilstate_tss_initialized(runtime)) {
PyThreadState *check = gilstate_tss_get(runtime);
if (check != newts) {
if (check && check->interp == newts->interp) {
Py_FatalError("Invalid thread state for this thread");
}
}
}
errno = err;
#endif
return oldts;
Expand Down

0 comments on commit 132b3f8

Please sign in to comment.