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.

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 (file)
index 0000000..611ab94
--- /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.c b/Python/ceval_gil.c
index a67908667667954dc24727f1344e8097b322ff7c..fd737b5738e8896eb5806b5e743dc8ddbeed3d27 100644 (file)
--- a/Python/ceval_gil.c
+++ b/Python/ceval_gil.c
@@ -369,6 +369,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;
 
@@ -384,11 +385,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;
         }
     }