From 50ac0c041f1e688e99626f5c2fef7b4f82352908 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 16 Sep 2025 09:21:58 +0100 Subject: [PATCH 1/2] gh-137433: Fix deadlock with stop-the-world and daemon threads (gh-137735) There was a deadlock originally seen by Memray when a daemon thread enabled or disabled profiling while the interpreter was shutting down. I think this could also happen with garbage collection, but I haven't seen that in practice. The daemon thread could be hung while trying acquire the global rwmutex that prevents overlapping global and per-interpreter stop-the-world events. Since it already held the main interpreter's stop-the-world lock, it also deadlocked the main thread, which is trying to perform interpreter finalization. Swap the order of lock acquisition to prevent this deadlock. Additionally, refactor `_PyParkingLot_Park` so that the global buckets hashtable is left in a clean state if the thread is hung in `PyEval_AcquireThread`. (cherry picked from commit 90fe3250f82712b61630d636246c92df7c40c816) Co-authored-by: Sam Gross --- Include/internal/pycore_semaphore.h | 4 +- Lib/test/test_threading.py | 27 ++++++++++++ ...-08-13-13-39-02.gh-issue-137433.g6Atfz.rst | 3 ++ Python/lock.c | 2 +- Python/parking_lot.c | 44 ++++++++----------- Python/pystate.c | 6 ++- 6 files changed, 55 insertions(+), 31 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-08-13-13-39-02.gh-issue-137433.g6Atfz.rst diff --git a/Include/internal/pycore_semaphore.h b/Include/internal/pycore_semaphore.h index 269538384606ce..66b4939dcacf05 100644 --- a/Include/internal/pycore_semaphore.h +++ b/Include/internal/pycore_semaphore.h @@ -46,10 +46,8 @@ typedef struct _PySemaphore { } _PySemaphore; // Puts the current thread to sleep until _PySemaphore_Wakeup() is called. -// If `detach` is true, then the thread will detach/release the GIL while -// sleeping. PyAPI_FUNC(int) -_PySemaphore_Wait(_PySemaphore *sema, PyTime_t timeout_ns, int detach); +_PySemaphore_Wait(_PySemaphore *sema, PyTime_t timeout_ns); // Wakes up a single thread waiting on sema. Note that _PySemaphore_Wakeup() // can be called before _PySemaphore_Wait(). diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index e468fcd7601c15..960a0cf3a948f1 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -1381,6 +1381,33 @@ def test_native_id_after_fork(self): self.assertEqual(len(native_ids), 2) self.assertNotEqual(native_ids[0], native_ids[1]) + def test_stop_the_world_during_finalization(self): + # gh-137433: Test functions that trigger a stop-the-world in the free + # threading build concurrent with interpreter finalization. + script = """if True: + import gc + import sys + import threading + NUM_THREADS = 5 + b = threading.Barrier(NUM_THREADS + 1) + def run_in_bg(): + b.wait() + while True: + sys.setprofile(None) + gc.collect() + + for _ in range(NUM_THREADS): + t = threading.Thread(target=run_in_bg, daemon=True) + t.start() + + b.wait() + print("Exiting...") + """ + rc, out, err = assert_python_ok('-c', script) + self.assertEqual(rc, 0) + self.assertEqual(err, b"") + self.assertEqual(out.strip(), b"Exiting...") + class ThreadJoinOnShutdown(BaseTestCase): def _run_and_join(self, script): diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-08-13-13-39-02.gh-issue-137433.g6Atfz.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-08-13-13-39-02.gh-issue-137433.g6Atfz.rst new file mode 100644 index 00000000000000..0389cb0c735128 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-08-13-13-39-02.gh-issue-137433.g6Atfz.rst @@ -0,0 +1,3 @@ +Fix a potential deadlock in the :term:`free threading` build when daemon +threads enable or disable profiling or tracing while the main thread is +shutting down the interpreter. diff --git a/Python/lock.c b/Python/lock.c index ca269bd2cb4b51..e4b2899e352a55 100644 --- a/Python/lock.c +++ b/Python/lock.c @@ -212,7 +212,7 @@ _PyRawMutex_LockSlow(_PyRawMutex *m) // Wait for us to be woken up. Note that we still have to lock the // mutex ourselves: it is NOT handed off to us. - _PySemaphore_Wait(&waiter.sema, -1, /*detach=*/0); + _PySemaphore_Wait(&waiter.sema, -1); } _PySemaphore_Destroy(&waiter.sema); diff --git a/Python/parking_lot.c b/Python/parking_lot.c index e896dea02712f2..99c1ad848be795 100644 --- a/Python/parking_lot.c +++ b/Python/parking_lot.c @@ -91,8 +91,8 @@ _PySemaphore_Destroy(_PySemaphore *sema) #endif } -static int -_PySemaphore_PlatformWait(_PySemaphore *sema, PyTime_t timeout) +int +_PySemaphore_Wait(_PySemaphore *sema, PyTime_t timeout) { int res; #if defined(MS_WINDOWS) @@ -225,27 +225,6 @@ _PySemaphore_PlatformWait(_PySemaphore *sema, PyTime_t timeout) return res; } -int -_PySemaphore_Wait(_PySemaphore *sema, PyTime_t timeout, int detach) -{ - PyThreadState *tstate = NULL; - if (detach) { - tstate = _PyThreadState_GET(); - if (tstate && _PyThreadState_IsAttached(tstate)) { - // Only detach if we are attached - PyEval_ReleaseThread(tstate); - } - else { - tstate = NULL; - } - } - int res = _PySemaphore_PlatformWait(sema, timeout); - if (tstate) { - PyEval_AcquireThread(tstate); - } - return res; -} - void _PySemaphore_Wakeup(_PySemaphore *sema) { @@ -342,7 +321,19 @@ _PyParkingLot_Park(const void *addr, const void *expected, size_t size, enqueue(bucket, addr, &wait); _PyRawMutex_Unlock(&bucket->mutex); - int res = _PySemaphore_Wait(&wait.sema, timeout_ns, detach); + PyThreadState *tstate = NULL; + if (detach) { + tstate = _PyThreadState_GET(); + if (tstate && _PyThreadState_IsAttached(tstate)) { + // Only detach if we are attached + PyEval_ReleaseThread(tstate); + } + else { + tstate = NULL; + } + } + + int res = _PySemaphore_Wait(&wait.sema, timeout_ns); if (res == Py_PARK_OK) { goto done; } @@ -354,7 +345,7 @@ _PyParkingLot_Park(const void *addr, const void *expected, size_t size, // Another thread has started to unpark us. Wait until we process the // wakeup signal. do { - res = _PySemaphore_Wait(&wait.sema, -1, detach); + res = _PySemaphore_Wait(&wait.sema, -1); } while (res != Py_PARK_OK); goto done; } @@ -366,6 +357,9 @@ _PyParkingLot_Park(const void *addr, const void *expected, size_t size, done: _PySemaphore_Destroy(&wait.sema); + if (tstate) { + PyEval_AcquireThread(tstate); + } return res; } diff --git a/Python/pystate.c b/Python/pystate.c index 02621f36d91c8a..bda65d01022ce8 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -2364,13 +2364,15 @@ stop_the_world(struct _stoptheworld_state *stw) { _PyRuntimeState *runtime = &_PyRuntime; - PyMutex_Lock(&stw->mutex); + // gh-137433: Acquire the rwmutex first to avoid deadlocks with daemon + // threads that may hang when blocked on lock acquisition. if (stw->is_global) { _PyRWMutex_Lock(&runtime->stoptheworld_mutex); } else { _PyRWMutex_RLock(&runtime->stoptheworld_mutex); } + PyMutex_Lock(&stw->mutex); HEAD_LOCK(runtime); stw->requested = 1; @@ -2436,13 +2438,13 @@ start_the_world(struct _stoptheworld_state *stw) } stw->requester = NULL; HEAD_UNLOCK(runtime); + PyMutex_Unlock(&stw->mutex); if (stw->is_global) { _PyRWMutex_Unlock(&runtime->stoptheworld_mutex); } else { _PyRWMutex_RUnlock(&runtime->stoptheworld_mutex); } - PyMutex_Unlock(&stw->mutex); } #endif // Py_GIL_DISABLED From b3ae15a7a176654fe9c720cf8ce9b4de4f0c1958 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 16 Sep 2025 04:53:24 -0400 Subject: [PATCH 2/2] Avoid abi change --- Include/internal/pycore_semaphore.h | 4 +++- Python/lock.c | 2 +- Python/parking_lot.c | 29 +++++++++++++++++++++++++---- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/Include/internal/pycore_semaphore.h b/Include/internal/pycore_semaphore.h index 66b4939dcacf05..269538384606ce 100644 --- a/Include/internal/pycore_semaphore.h +++ b/Include/internal/pycore_semaphore.h @@ -46,8 +46,10 @@ typedef struct _PySemaphore { } _PySemaphore; // Puts the current thread to sleep until _PySemaphore_Wakeup() is called. +// If `detach` is true, then the thread will detach/release the GIL while +// sleeping. PyAPI_FUNC(int) -_PySemaphore_Wait(_PySemaphore *sema, PyTime_t timeout_ns); +_PySemaphore_Wait(_PySemaphore *sema, PyTime_t timeout_ns, int detach); // Wakes up a single thread waiting on sema. Note that _PySemaphore_Wakeup() // can be called before _PySemaphore_Wait(). diff --git a/Python/lock.c b/Python/lock.c index e4b2899e352a55..ca269bd2cb4b51 100644 --- a/Python/lock.c +++ b/Python/lock.c @@ -212,7 +212,7 @@ _PyRawMutex_LockSlow(_PyRawMutex *m) // Wait for us to be woken up. Note that we still have to lock the // mutex ourselves: it is NOT handed off to us. - _PySemaphore_Wait(&waiter.sema, -1); + _PySemaphore_Wait(&waiter.sema, -1, /*detach=*/0); } _PySemaphore_Destroy(&waiter.sema); diff --git a/Python/parking_lot.c b/Python/parking_lot.c index 99c1ad848be795..0ffea97b079943 100644 --- a/Python/parking_lot.c +++ b/Python/parking_lot.c @@ -91,8 +91,8 @@ _PySemaphore_Destroy(_PySemaphore *sema) #endif } -int -_PySemaphore_Wait(_PySemaphore *sema, PyTime_t timeout) +static int +_PySemaphore_PlatformWait(_PySemaphore *sema, PyTime_t timeout) { int res; #if defined(MS_WINDOWS) @@ -225,6 +225,27 @@ _PySemaphore_Wait(_PySemaphore *sema, PyTime_t timeout) return res; } +int +_PySemaphore_Wait(_PySemaphore *sema, PyTime_t timeout, int detach) +{ + PyThreadState *tstate = NULL; + if (detach) { + tstate = _PyThreadState_GET(); + if (tstate && _PyThreadState_IsAttached(tstate)) { + // Only detach if we are attached + PyEval_ReleaseThread(tstate); + } + else { + tstate = NULL; + } + } + int res = _PySemaphore_PlatformWait(sema, timeout); + if (tstate) { + PyEval_AcquireThread(tstate); + } + return res; +} + void _PySemaphore_Wakeup(_PySemaphore *sema) { @@ -333,7 +354,7 @@ _PyParkingLot_Park(const void *addr, const void *expected, size_t size, } } - int res = _PySemaphore_Wait(&wait.sema, timeout_ns); + int res = _PySemaphore_Wait(&wait.sema, timeout_ns, 0); if (res == Py_PARK_OK) { goto done; } @@ -345,7 +366,7 @@ _PyParkingLot_Park(const void *addr, const void *expected, size_t size, // Another thread has started to unpark us. Wait until we process the // wakeup signal. do { - res = _PySemaphore_Wait(&wait.sema, -1); + res = _PySemaphore_Wait(&wait.sema, -1, 0); } while (res != Py_PARK_OK); goto done; }