From 978a6b431e3c4900793a4c142a9e0a222fb430cc Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 20 Sep 2022 00:13:56 +0200 Subject: [PATCH] gh-96387: take_gil() resets drop request before exit (#96869) At Python exit, sometimes a thread holding the GIL can wait forever for a thread (usually a daemon thread) which requested to drop the GIL, whereas the thread already exited. To fix the race condition, the thread which requested the GIL drop now resets its request before exiting. take_gil() now calls RESET_GIL_DROP_REQUEST() before PyThread_exit_thread() if it called SET_GIL_DROP_REQUEST to fix a race condition with drop_gil(). Issue discovered and analyzed by Mingliang ZHAO. (cherry picked from commit 04f4977f508583954ad7b9cb09076ee1e57461f8) --- .../2022-09-16-16-54-35.gh-issue-96387.GRzewg.rst | 5 +++++ Python/ceval_gil.h | 11 +++++++++++ 2 files changed, 16 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-09-16-16-54-35.gh-issue-96387.GRzewg.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-09-16-16-54-35.gh-issue-96387.GRzewg.rst b/Misc/NEWS.d/next/Core and Builtins/2022-09-16-16-54-35.gh-issue-96387.GRzewg.rst new file mode 100644 index 00000000000000..611ab94bc636a6 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-09-16-16-54-35.gh-issue-96387.GRzewg.rst @@ -0,0 +1,5 @@ +At Python exit, sometimes a thread holding the GIL can wait forever for a +thread (usually a daemon thread) which requested to drop the GIL, whereas +the thread already exited. To fix the race condition, the thread which +requested the GIL drop now resets its request before exiting. Issue +discovered and analyzed by Mingliang ZHAO. Patch by Victor Stinner. diff --git a/Python/ceval_gil.h b/Python/ceval_gil.h index 1b2dc7f8e1dc31..23f6fb26580ece 100644 --- a/Python/ceval_gil.h +++ b/Python/ceval_gil.h @@ -239,6 +239,7 @@ take_gil(PyThreadState *tstate) goto _ready; } + int drop_requested = 0; while (_Py_atomic_load_relaxed(&gil->locked)) { unsigned long saved_switchnum = gil->switch_number; @@ -254,11 +255,21 @@ take_gil(PyThreadState *tstate) { if (tstate_must_exit(tstate)) { MUTEX_UNLOCK(gil->mutex); + // gh-96387: If the loop requested a drop request in a previous + // iteration, reset the request. Otherwise, drop_gil() can + // block forever waiting for the thread which exited. Drop + // requests made by other threads are also reset: these threads + // may have to request again a drop request (iterate one more + // time). + if (drop_requested) { + RESET_GIL_DROP_REQUEST(interp); + } PyThread_exit_thread(); } assert(is_tstate_valid(tstate)); SET_GIL_DROP_REQUEST(interp); + drop_requested = 1; } }