Skip to content

gh-137433: Fix deadlock with stop-the-world and daemon threads #137735

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions Include/internal/pycore_semaphore.h
Original file line number Diff line number Diff line change
Expand Up @@ -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().
Expand Down
27 changes: 27 additions & 0 deletions Lib/test/test_threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -1430,6 +1430,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):
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 1 addition & 1 deletion Python/lock.c
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,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);
Expand Down
44 changes: 19 additions & 25 deletions Python/parking_lot.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;

}
Expand Down
6 changes: 4 additions & 2 deletions Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -2245,13 +2245,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;
Expand Down Expand Up @@ -2317,13 +2319,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

Expand Down
Loading