From 6a6ca9e87b5db93e20fb5ac8a2f16914dc197aac Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 7 Mar 2024 22:34:55 +0000 Subject: [PATCH 1/3] gh-108724: Fix _PySemaphore_Wait call during thread deletion In general, when `_PyThreadState_GET()` is non-NULL then the current thread is "attached", but there is a small window during `PyThreadState_DeleteCurrent()` where that's not true: tstate_delete_common is called when the thread is detached, but before current_fast_clear(). This updates _PySemaphore_Wait() to handle that case. --- Python/parking_lot.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Python/parking_lot.c b/Python/parking_lot.c index 0a897f9952f648..085b0367bae17c 100644 --- a/Python/parking_lot.c +++ b/Python/parking_lot.c @@ -194,14 +194,17 @@ _PySemaphore_Wait(_PySemaphore *sema, PyTime_t timeout, int detach) PyThreadState *tstate = NULL; if (detach) { tstate = _PyThreadState_GET(); - if (tstate) { - PyEval_ReleaseThread(tstate); + // Only detach if we are attached + if (tstate && tstate->state != _Py_THREAD_ATTACHED) { + tstate = NULL; } } + if (tstate) { + PyEval_ReleaseThread(tstate); + } int res = _PySemaphore_PlatformWait(sema, timeout); - - if (detach && tstate) { + if (tstate) { PyEval_AcquireThread(tstate); } return res; From 341650be5b13162f05ec073b82dca8575d22bc5f Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Fri, 8 Mar 2024 14:52:44 -0500 Subject: [PATCH 2/3] Update Python/parking_lot.c Co-authored-by: Eric Snow --- Python/parking_lot.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Python/parking_lot.c b/Python/parking_lot.c index 085b0367bae17c..59daf2b3fbfb4b 100644 --- a/Python/parking_lot.c +++ b/Python/parking_lot.c @@ -198,10 +198,9 @@ _PySemaphore_Wait(_PySemaphore *sema, PyTime_t timeout, int detach) if (tstate && tstate->state != _Py_THREAD_ATTACHED) { tstate = NULL; } - } - - if (tstate) { - PyEval_ReleaseThread(tstate); + if (tstate) { + PyEval_ReleaseThread(tstate); + } } int res = _PySemaphore_PlatformWait(sema, timeout); if (tstate) { From 69dc393d8230b30a9e6425857ee8f9e2adf31bbb Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Fri, 8 Mar 2024 19:55:22 +0000 Subject: [PATCH 3/3] Use variant of Eric's second suggestion --- Python/parking_lot.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Python/parking_lot.c b/Python/parking_lot.c index 59daf2b3fbfb4b..d5877fef56e4d0 100644 --- a/Python/parking_lot.c +++ b/Python/parking_lot.c @@ -194,13 +194,13 @@ _PySemaphore_Wait(_PySemaphore *sema, PyTime_t timeout, int detach) PyThreadState *tstate = NULL; if (detach) { tstate = _PyThreadState_GET(); - // Only detach if we are attached - if (tstate && tstate->state != _Py_THREAD_ATTACHED) { - tstate = NULL; - } - if (tstate) { + if (tstate && tstate->state == _Py_THREAD_ATTACHED) { + // Only detach if we are attached PyEval_ReleaseThread(tstate); } + else { + tstate = NULL; + } } int res = _PySemaphore_PlatformWait(sema, timeout); if (tstate) {