pythongh-119585: Fix crash involving PyGILState_Release() and `PyTh…

…readState_Clear()`

Don't decrement `gilstate_counter` in `PyGILState_Release()` until after
`PyThreadState_Clear()` is called. A destructor called from
`PyThreadState_Clear()` may call back into `PyGILState_Ensure()` and
PyGILState_Release()` and if `gilstate_counter` is zero, it will try to
create a new thread state before the current active thread state is
destroyed.

Lib/test/test_capi/test_misc.py

@@ -2888,6 +2888,22 @@ def callback():
         t.start()
         t.join()
 
+    @threading_helper.reap_threads
+    @threading_helper.requires_working_threading()
+    def test_thread_gilstate_in_clear(self):
+        # See https://github.com/python/cpython/issues/119585
+        class C:
+            def __del__(self):
+                _testcapi.gilstate_ensure_release()
+
+        # Thread-local variables are destroyed in `PyThreadState_Clear()`.
+        local_var = threading.local()
+
+        def callback():
+            local_var.x = C()
+
+        _testcapi._test_thread_state(callback)
+
     @threading_helper.reap_threads
     @threading_helper.requires_working_threading()
     def test_gilstate_ensure_no_deadlock(self):

Misc/NEWS.d/next/C API/2024-05-29-21-05-59.gh-issue-119585.Sn7JL3.rst

@@ -0,0 +1,5 @@
+Fix crash when a thread state that was created by :c:func:`PyGILState_Ensure`
+calls a destructor that during :c:func:`PyThreadState_Clear` that
+calls back into :c:func:`PyGILState_Ensure` and :c:func:`PyGILState_Release`.
+This might occur when in the free-threaded build or when using thread-local
+variables whose destructors call :c:func:`PyGILState_Ensure`.

Modules/_testcapimodule.c

@@ -764,6 +764,14 @@ test_thread_state(PyObject *self, PyObject *args)
     Py_RETURN_NONE;
 }
 
+static PyObject *
+gilstate_ensure_release(PyObject *module, PyObject *Py_UNUSED(ignored))
+{
+    PyGILState_STATE state = PyGILState_Ensure();
+    PyGILState_Release(state);
+    Py_RETURN_NONE;
+}
+
 #ifndef MS_WINDOWS
 static PyThread_type_lock wait_done = NULL;
 
@@ -3351,6 +3359,7 @@ static PyMethodDef TestMethods[] = {
     {"test_get_type_dict",        test_get_type_dict,            METH_NOARGS},
     {"test_reftracer",          test_reftracer,                  METH_NOARGS},
     {"_test_thread_state",      test_thread_state,               METH_VARARGS},
+    {"gilstate_ensure_release", gilstate_ensure_release,         METH_NOARGS},
 #ifndef MS_WINDOWS
     {"_spawn_pthread_waiter",   spawn_pthread_waiter,            METH_NOARGS},
     {"_end_spawned_pthread",    end_spawned_pthread,             METH_NOARGS},

Python/pystate.c

@@ -2798,17 +2798,20 @@ PyGILState_Release(PyGILState_STATE oldstate)
                              tstate);
     }
     assert(holds_gil(tstate));
-    --tstate->gilstate_counter;
-    assert(tstate->gilstate_counter >= 0); /* illegal counter value */
+    assert(tstate->gilstate_counter >= 1); /* illegal counter value */
 
     /* If we're going to destroy this thread-state, we must
      * clear it while the GIL is held, as destructors may run.
      */
-    if (tstate->gilstate_counter == 0) {
+    if (tstate->gilstate_counter == 1) {
         /* can't have been locked when we created it */
         assert(oldstate == PyGILState_UNLOCKED);
         // XXX Unbind tstate here.
         PyThreadState_Clear(tstate);
+        // gh-119585: decrement gilstate_counter after `PyThreadState_Clear()`
+        // because it may call destructors that themselves use
+        // PyGILState_Ensure and PyGILState_Release.
+        --tstate->gilstate_counter;
         /* Delete the thread-state.  Note this releases the GIL too!
          * It's vital that the GIL be held here, to avoid shutdown
          * races; see bugs 225673 and 1061968 (that nasty bug has a
@@ -2818,8 +2821,11 @@ PyGILState_Release(PyGILState_STATE oldstate)
         _PyThreadState_DeleteCurrent(tstate);
     }
     /* Release the lock if necessary */
-    else if (oldstate == PyGILState_UNLOCKED) {
-        PyEval_SaveThread();
+    else {
+        --tstate->gilstate_counter;
+        if (oldstate == PyGILState_UNLOCKED) {
+            PyEval_SaveThread();
+        }
     }
 }