From: Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com> Date: Fri, 19 Aug 2022 21:11:31 +0000 (-0700) Subject: GH-96071: fix deadlock in PyGILState_Ensure (GH-96124) (#96129) X-Git-Tag: v3.11.0rc2~89 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=1fa9644c23632f8ffc3f59e7f574b92185c371e4;p=thirdparty%2FPython%2Fcpython.git GH-96071: fix deadlock in PyGILState_Ensure (GH-96124) (#96129) Alternative of GH-96107 (cherry picked from commit e0d54a4a799dae4ebdd72a16bcf287ed62ae2972) Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> --- 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 2e4585688ea8..a0c12bac9d14 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -795,7 +795,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); @@ -807,18 +815,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, @@ -829,11 +834,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 *