gh-119585: Fix crash involving `PyGILState_Release()` and `PyThreadState_Clear()...

Make sure that `gilstate_counter` is not zero in when calling
`PyThreadState_Clear()`. A destructor called from `PyThreadState_Clear()` may
call back into `PyGILState_Ensure()` and `PyGILState_Release()`. If
`gilstate_counter` is zero, it will try to create a new thread state before
the current active thread state is destroyed, leading to an assertion failure
or crash.

diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py
index ed42d7b64302f9301299ec7ca5a17cb844eed66b..f3d16e4a2fc92a3c4661974ee2904b07b42a621b 100644 (file)
--- a/Lib/test/test_capi/test_misc.py
+++ b/Lib/test/test_capi/test_misc.py
@@ -2888,6 +2888,22 @@ class TestThreadState(unittest.TestCase):
         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):

diff --git a/Misc/NEWS.d/next/C API/2024-05-29-21-05-59.gh-issue-119585.Sn7JL3.rst b/Misc/NEWS.d/next/C API/2024-05-29-21-05-59.gh-issue-119585.Sn7JL3.rst
new file mode 100644 (file)
index 0000000..038dec2
--- /dev/null
+++ b/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`.

diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index f99ebf0dde4f9e4a5ccdf0bddc4c412f3911f53e..b58c17260626c2ab96575d447d156bcccb8daf26 100644 (file)
--- a/Modules/_testcapimodule.c
+++ b/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},

diff --git a/Python/pystate.c b/Python/pystate.c
index 1ea1ad982a0ec99cf071ba0a357950a49e8f93d4..ad7e082ce0d37e4c5b4658e57e081f4fe0d288eb 100644 (file)
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -2808,12 +2808,18 @@ PyGILState_Release(PyGILState_STATE oldstate)
         /* can't have been locked when we created it */
         assert(oldstate == PyGILState_UNLOCKED);
         // XXX Unbind tstate here.
+        // gh-119585: `PyThreadState_Clear()` may call destructors that
+        // themselves use PyGILState_Ensure and PyGILState_Release, so make
+        // sure that gilstate_counter is not zero when calling it.
+        ++tstate->gilstate_counter;
         PyThreadState_Clear(tstate);
+        --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
          * habit of coming back).
          */
+        assert(tstate->gilstate_counter == 0);
         assert(current_fast_get() == tstate);
         _PyThreadState_DeleteCurrent(tstate);
     }