Skip to content

Commit dcff50a

Browse files
gh-96387: take_gil() resets drop request before exit (GH-96869) (GH-96941)
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 04f4977) (cherry picked from commit 6ff5471) Co-authored-by: Victor Stinner <[email protected]>
1 parent 88a3f18 commit dcff50a

File tree

2 files changed

+16
-0
lines changed

2 files changed

+16
-0
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
At Python exit, sometimes a thread holding the GIL can wait forever for a
2+
thread (usually a daemon thread) which requested to drop the GIL, whereas
3+
the thread already exited. To fix the race condition, the thread which
4+
requested the GIL drop now resets its request before exiting. Issue
5+
discovered and analyzed by Mingliang ZHAO. Patch by Victor Stinner.

Python/ceval_gil.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ take_gil(PyThreadState *tstate)
247247
goto _ready;
248248
}
249249

250+
int drop_requested = 0;
250251
while (_Py_atomic_load_relaxed(&gil->locked)) {
251252
unsigned long saved_switchnum = gil->switch_number;
252253

@@ -262,11 +263,21 @@ take_gil(PyThreadState *tstate)
262263
{
263264
if (tstate_must_exit(tstate)) {
264265
MUTEX_UNLOCK(gil->mutex);
266+
// gh-96387: If the loop requested a drop request in a previous
267+
// iteration, reset the request. Otherwise, drop_gil() can
268+
// block forever waiting for the thread which exited. Drop
269+
// requests made by other threads are also reset: these threads
270+
// may have to request again a drop request (iterate one more
271+
// time).
272+
if (drop_requested) {
273+
RESET_GIL_DROP_REQUEST(interp);
274+
}
265275
PyThread_exit_thread();
266276
}
267277
assert(is_tstate_valid(tstate));
268278

269279
SET_GIL_DROP_REQUEST(interp);
280+
drop_requested = 1;
270281
}
271282
}
272283

0 commit comments

Comments
 (0)