diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index ed42d7b64302f9..f3d16e4a2fc92a 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/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): 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 index 00000000000000..038dec2dbf90d1 --- /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 f99ebf0dde4f9e..b58c17260626c2 100644 --- 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 1ea1ad982a0ec9..ad7e082ce0d37e 100644 --- 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); }