[Python-checkins] GH-96071: fix deadlock in PyGILState_Ensure (GH-96124)
miss-islington
webhook-mailer at python.org
Fri Aug 19 15:43:08 EDT 2022
https://github.com/python/cpython/commit/e0d54a4a799dae4ebdd72a16bcf287ed62ae2972
commit: e0d54a4a799dae4ebdd72a16bcf287ed62ae2972
branch: main
author: Kumar Aditya <59607654+kumaraditya303 at users.noreply.github.com>
committer: miss-islington <31488909+miss-islington at users.noreply.github.com>
date: 2022-08-19T12:43:00-07:00
summary:
GH-96071: fix deadlock in PyGILState_Ensure (GH-96124)
Alternative of #96107
files:
A Misc/NEWS.d/next/Core and Builtins/2022-08-19-06-51-17.gh-issue-96071.mVgPAo.rst
M Python/pystate.c
diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-08-19-06-51-17.gh-issue-96071.mVgPAo.rst b/Misc/NEWS.d/next/Core and Builtins/2022-08-19-06-51-17.gh-issue-96071.mVgPAo.rst
new file mode 100644
index 000000000000..37653ffac124
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2022-08-19-06-51-17.gh-issue-96071.mVgPAo.rst
@@ -0,0 +1 @@
+Fix a deadlock in :c:func:`PyGILState_Ensure` when allocating new thread state. Patch by Kumar Aditya.
diff --git a/Python/pystate.c b/Python/pystate.c
index 642d680ba20b..a11f1622ecd4 100644
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -810,7 +810,15 @@ new_threadstate(PyInterpreterState *interp)
{
PyThreadState *tstate;
_PyRuntimeState *runtime = interp->runtime;
-
+ // We don't need to allocate a thread state for the main interpreter
+ // (the common case), but doing it later for the other case revealed a
+ // reentrancy problem (deadlock). So for now we always allocate before
+ // taking the interpreters lock. See GH-96071.
+ PyThreadState *new_tstate = alloc_threadstate();
+ int used_newtstate;
+ if (new_tstate == NULL) {
+ return NULL;
+ }
/* We serialize concurrent creation to protect global state. */
HEAD_LOCK(runtime);
@@ -822,18 +830,15 @@ new_threadstate(PyInterpreterState *interp)
if (old_head == NULL) {
// It's the interpreter's initial thread state.
assert(id == 1);
-
+ used_newtstate = 0;
tstate = &interp->_initial_thread;
}
else {
// Every valid interpreter must have at least one thread.
assert(id > 1);
assert(old_head->prev == NULL);
-
- tstate = alloc_threadstate();
- if (tstate == NULL) {
- goto error;
- }
+ used_newtstate = 1;
+ tstate = new_tstate;
// Set to _PyThreadState_INIT.
memcpy(tstate,
&initial._main_interpreter._initial_thread,
@@ -844,11 +849,11 @@ new_threadstate(PyInterpreterState *interp)
init_threadstate(tstate, interp, id, old_head);
HEAD_UNLOCK(runtime);
+ if (!used_newtstate) {
+ // Must be called with lock unlocked to avoid re-entrancy deadlock.
+ PyMem_RawFree(new_tstate);
+ }
return tstate;
-
-error:
- HEAD_UNLOCK(runtime);
- return NULL;
}
PyThreadState *
More information about the Python-checkins
mailing list