]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-59956: Partial Fix for GILState API Compatibility with Subinterpreters (gh-101431)
authorEric Snow <ericsnowcurrently@gmail.com>
Mon, 6 Feb 2023 21:39:25 +0000 (14:39 -0700)
committerGitHub <noreply@github.com>
Mon, 6 Feb 2023 21:39:25 +0000 (14:39 -0700)
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.

https://github.com/python/cpython/issues/59956

Misc/NEWS.d/next/Core and Builtins/2023-01-30-11-56-09.gh-issue-59956.7xqnC_.rst [new file with mode: 0644]
Python/pystate.c

diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-01-30-11-56-09.gh-issue-59956.7xqnC_.rst b/Misc/NEWS.d/next/Core and Builtins/2023-01-30-11-56-09.gh-issue-59956.7xqnC_.rst
new file mode 100644 (file)
index 0000000..b3c1896
--- /dev/null
@@ -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.
index 8bb49d954a81b6e74ee0ff0a360532a002df2e82..ed8c2e212a55391f366f0a52c9cf63b40c904f95 100644 (file)
@@ -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
@@ -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);
@@ -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;