https://github.com/python/cpython/commit/bcc1be39cb1d04ad9fc0bd1b9193d3972835a57c
commit: bcc1be39cb1d04ad9fc0bd1b9193d3972835a57c
branch: main
author: Sam Gross <[email protected]>
committer: colesbury <[email protected]>
date: 2024-05-31T10:50:52-04:00
summary:
gh-119585: Fix crash involving `PyGILState_Release()` and
`PyThreadState_Clear()` (#119753)
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.
files:
A Misc/NEWS.d/next/C API/2024-05-29-21-05-59.gh-issue-119585.Sn7JL3.rst
M Lib/test/test_capi/test_misc.py
M Modules/_testcapimodule.c
M Python/pystate.c
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);
}
_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: [email protected]
