From 8a2d1cda25b9083b8119d2037ea330be777d3f37 Mon Sep 17 00:00:00 2001 From: xalestar Date: Sat, 10 Oct 2026 21:25:54 +0800 Subject: [PATCH 1/3] Set FUTEX_WAITERS under the bucket lock in LOCK_PI futex_lock_pi_inner set FUTEX_WAITERS before it took the bucket lock and then, under the lock, only checked that the word was still owned. The loop that sets the bit also left without setting it when it read an unowned word. An owner that released and retook the lock in guest code between those reads left the waiter parked on an owned word with the bit clear. The owner's next release was then the guest-side CAS from its tid to 0, which makes no syscall, so FUTEX_UNLOCK_PI never ran and the waiter polled forever. Rosetta guards its runtime with such a lock. pthread-tls-probe hung in 6 of 300 runs with the main thread parked in FUTEX_LOCK_PI on that lock, before it reached pthread_join; a trace of three hangs shows the fast-path CAS reading 0x2, the loop reading 0, and the enqueue reading 0x2 again with no FUTEX_UNLOCK_PI after it. The bit is now set and the waiter enqueued under one hold of the bucket lock. With the bit set the owner can only release through FUTEX_UNLOCK_PI, which takes the same lock after its release and so finds the waiter. tests/test-futex-pi.c gains one case: two threads take and release one PI lock, entering the kernel only on contention, until 2000 acquisitions have gone through FUTEX_LOCK_PI. That call carries a 2 s deadline so that a lost wake is reported and not hung on, and the case fails when it cannot collect all 2000 within 10 s, so it cannot pass without contention. It reported a lost wake in 20 of 20 runs before the change and in 0 of 20 after it. Verified: the loop from the issue ran 1000 times with 0 timeouts, make test-rosetta-glibc passed 5 times in a row, test-futex-pi passed 5 of 5 runs on Linux 6.18 under QEMU with 4 CPUs, and make check is green. Fixes #413 --- src/runtime/futex.c | 57 +++++++------- tests/test-futex-pi.c | 174 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 202 insertions(+), 29 deletions(-) diff --git a/src/runtime/futex.c b/src/runtime/futex.c index 6f93835c2..16bdcc093 100644 --- a/src/runtime/futex.c +++ b/src/runtime/futex.c @@ -1547,43 +1547,42 @@ static int64_t futex_lock_pi_inner(guest_t *g, if (owner_tid != 0 && !thread_find((int64_t) owner_tid)) return -LINUX_ESRCH; - /* Set the WAITERS bit so the owner takes the kernel-mediated unlock - * path. Retry the CAS in a loop since the owner may release - * concurrently. + /* Set FUTEX_WAITERS and enqueue under one hold of the bucket lock. + * + * The bit is what makes the owner release through FUTEX_UNLOCK_PI: its + * guest-side CAS(TID->0) fails while the bit is set. Parking on an + * owned word without it is therefore a lost wake, since that CAS + * succeeds and makes no syscall. Setting the bit before taking the + * lock, and then only checking that the word was still owned, allowed + * exactly that: the owner could release and retake the lock in guest + * code in between, and the word read as owned with the bit gone. + * + * Under the lock the two cannot come apart. Once the word is seen (or + * made) owned with the bit set, the owner can only release through + * FUTEX_UNLOCK_PI, which takes this bucket lock after its release and + * so finds the waiter enqueued below. */ + pthread_mutex_lock(&b->lock); + + uint32_t cur; for (;;) { - uint32_t cur; - if (!futex_word_load(word, &cur)) + if (!futex_word_load(word, &cur)) { + pthread_mutex_unlock(&b->lock); return -LINUX_EFAULT; - if (futex_pi_unowned(cur)) - break; /* Owner released; retry outer loop */ - if (futex_pi_has_waiters(cur)) - break; /* Already set by another waiter */ - uint32_t desired = futex_pi_set_waiters(cur); + } + if (futex_pi_unowned(cur) || futex_pi_has_waiters(cur)) + break; bool marked; - if (!futex_word_cas(word, &cur, desired, &marked)) + if (!futex_word_cas(word, &cur, futex_pi_set_waiters(cur), + &marked)) { + pthread_mutex_unlock(&b->lock); return -LINUX_EFAULT; + } if (marked) - break; /* WAITERS bit set */ + break; } - /* Re-check after WAITERS bit: if lock is now free, retry */ - uint32_t cur; - if (!futex_word_load(word, &cur)) - return -LINUX_EFAULT; - if (futex_pi_unowned(cur)) - continue; - - /* Enqueue and block */ - pthread_mutex_lock(&b->lock); - - /* Double-check under bucket lock: owner may have released and called - * UNLOCK_PI between the current WAITERS set and lock. - */ - if (!futex_word_load(word, &cur)) { - pthread_mutex_unlock(&b->lock); - return -LINUX_EFAULT; - } + /* Owner released first: retry the acquisition. */ if (futex_pi_unowned(cur)) { pthread_mutex_unlock(&b->lock); continue; diff --git a/tests/test-futex-pi.c b/tests/test-futex-pi.c index e9c62258a..c89b446a3 100644 --- a/tests/test-futex-pi.c +++ b/tests/test-futex-pi.c @@ -25,11 +25,19 @@ * must only return when a real wake arrives or a signal is * genuinely queued for the thread. * + * 4. PI waiter against an owner that retakes the lock in guest code: two + * threads take and release one PI lock in a tight loop, entering the + * kernel only on contention, until 2000 acquisitions have gone through + * FUTEX_LOCK_PI. A waiter that parks while the word has no FUTEX_WAITERS + * is never woken, because the owner's next release is a guest-side CAS + * that makes no syscall. + * * Syscalls exercised: futex(98), clone(220), gettid(178), exit(93) */ #include #include +#include #include #include #include @@ -252,6 +260,171 @@ static void test_pi_owner_died_recover(void) PASS(); } +/* Test: a PI waiter is woken when the owner retakes the lock in guest code */ + +#define RELOCK_CONTENDED 2000 /* FUTEX_LOCK_PI acquisitions to collect */ +#define RELOCK_BUDGET_SEC 10 /* Give up collecting after this long */ +#define RELOCK_DEADLINE_SEC 2 /* On one FUTEX_LOCK_PI call */ + +static volatile uint32_t relock_word __attribute__((aligned(4))) = 0; +static volatile long relock_count = 0; /* Guarded by relock_word */ +static volatile long relock_iters = 0; /* Loop passes of both threads */ +static volatile long relock_contended = 0; /* FUTEX_LOCK_PI calls that won */ +static volatile long relock_failure = 0; /* First failed acquisition */ +static char relock_stack_buf[8192] __attribute__((aligned(16))); + +static long relock_clock(int clock, struct timespec *ts) +{ + return raw_syscall6(__NR_clock_gettime, clock, (long) ts, 0, 0, 0, 0); +} + +/* The protocol glibc and Rosetta both use: take the lock with a guest-side CAS + * and enter the kernel only when that fails. The kernel call carries a deadline + * so that a waiter nobody wakes reports ETIMEDOUT rather than hanging the + * suite; with the lock changing hands every few microseconds, two seconds is + * never reached by a waiter that is woken. + */ +static long relock_acquire(uint32_t tid) +{ + uint32_t expected = 0; + if (__atomic_compare_exchange_n((uint32_t *) &relock_word, &expected, tid, + 0, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST)) + return 0; + + /* FUTEX_LOCK_PI takes an absolute CLOCK_REALTIME deadline. */ + struct timespec deadline; + long r = relock_clock(CLOCK_REALTIME, &deadline); + if (r != 0) + return r; + deadline.tv_sec += RELOCK_DEADLINE_SEC; + r = raw_syscall6(__NR_futex, (long) &relock_word, + FUTEX_LOCK_PI | FUTEX_PRIVATE, 0, (long) &deadline, 0, 0); + if (r == 0) + __atomic_fetch_add((long *) &relock_contended, 1, __ATOMIC_SEQ_CST); + return r; +} + +static void relock_release(uint32_t tid) +{ + /* Fails exactly when FUTEX_WAITERS is set, which is the kernel's way of + * asking for the syscall. + */ + uint32_t expected = tid; + if (__atomic_compare_exchange_n((uint32_t *) &relock_word, &expected, 0, 0, + __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST)) + return; + raw_futex_unlock_pi((uint32_t *) &relock_word); +} + +/* Runs on both threads until they have collected RELOCK_CONTENDED kernel + * acquisitions between them. A thread running alone collects none, so the two + * need no start signal: whichever starts first keeps going until the other + * arrives. Raw syscalls only: the clone child has no TLS. + */ +static void relock_loop(void) +{ + uint32_t tid = (uint32_t) raw_gettid(); + struct timespec now; + long stop_sec = 0; + long iters = 0; + + if (relock_clock(CLOCK_MONOTONIC, &now) == 0) + stop_sec = now.tv_sec + RELOCK_BUDGET_SEC; + + for (;;) { + if (__atomic_load_n((long *) &relock_failure, __ATOMIC_SEQ_CST) != 0 || + __atomic_load_n((long *) &relock_contended, __ATOMIC_SEQ_CST) >= + RELOCK_CONTENDED) + break; + + /* The budget ends a run that cannot contend, such as one CPU; the + * caller then fails on the count. + */ + if ((iters & 1023) == 0 && + (relock_clock(CLOCK_MONOTONIC, &now) != 0 || now.tv_sec > stop_sec)) + break; + + long r = relock_acquire(tid); + if (r != 0) { + long none = 0; + __atomic_compare_exchange_n((long *) &relock_failure, &none, r, 0, + __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST); + break; + } + relock_count = relock_count + 1; + relock_release(tid); + iters++; + } + __atomic_fetch_add((long *) &relock_iters, iters, __ATOMIC_SEQ_CST); +} + +static void test_pi_owner_relock(void) +{ + TEST("PI waiter woken across relock"); + + relock_word = 0; + relock_count = 0; + relock_iters = 0; + relock_contended = 0; + relock_failure = 0; + + void *stack_top = relock_stack_buf + sizeof(relock_stack_buf); + int child_tid_val = 0; + long ret = raw_clone(0x7d0f00, stack_top, &child_tid_val, 0, + (int *) &child_tid_val); + if (ret < 0) { + FAIL("clone failed"); + return; + } + if (ret == 0) { + relock_loop(); + raw_exit(0); + } + + relock_loop(); + + /* The child is at most one deadline plus one budget behind. */ + for (int i = 0; i < (RELOCK_DEADLINE_SEC + RELOCK_BUDGET_SEC + 1) * 100; + i++) { + if (__atomic_load_n(&child_tid_val, __ATOMIC_SEQ_CST) == 0) + break; + usleep(10000); + } + if (__atomic_load_n(&child_tid_val, __ATOMIC_SEQ_CST) != 0) { + FAIL("relock child did not exit"); + return; + } + + long failure = __atomic_load_n((long *) &relock_failure, __ATOMIC_SEQ_CST); + if (failure != 0) { + printf( + "FAIL: lock acquisition returned %ld (-110: a parked waiter " + "was never woken)\n", + failure); + fails++; + return; + } + + /* Fewer means the run could not contend enough to show a lost wake, and a + * pass would then say nothing about the kernel path. + */ + long contended = + __atomic_load_n((long *) &relock_contended, __ATOMIC_SEQ_CST); + if (contended < RELOCK_CONTENDED) { + printf("FAIL: %ld of %d FUTEX_LOCK_PI acquisitions in %d s\n", + contended, RELOCK_CONTENDED, RELOCK_BUDGET_SEC); + fails++; + return; + } + if (relock_count != relock_iters) { + printf("FAIL: %ld increments under the lock, expected %ld\n", + relock_count, relock_iters); + fails++; + return; + } + PASS(); +} + /* Test 3: futex_wait without a signal blocks until woken */ /* Sibling that waits ~1.2 s, flips the futex word, and issues FUTEX_WAKE on the @@ -389,6 +562,7 @@ int main(void) test_futex_eintr(); test_futex_unaligned(); test_pi_owner_died_recover(); + test_pi_owner_relock(); test_pi_dead_owner(); /* Last: uses CLONE_THREAD which may hang on x64 */ SUMMARY("test-futex-pi"); From 097546bdc383a49533bb3b9efb8f3a3dc1921541 Mon Sep 17 00:00:00 2001 From: xalestar Date: Fri, 16 Oct 2026 00:14:41 +0800 Subject: [PATCH 2/3] Clear the PI waiters bit under the bucket lock The FUTEX_LOCK_PI timeout path looked for other waiters under the bucket lock but cleared FUTEX_WAITERS only after dropping it. A waiter that took the lock in that gap read the bit as set, enqueued without setting it, and parked. The clear then left it on an owned word with no bit, so the owner's next release was the guest-side CAS and nothing woke the waiter. Clear the bit before the unlock, so the check and the clear happen under one hold of the lock. --- src/runtime/futex.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/runtime/futex.c b/src/runtime/futex.c index 16bdcc093..6e684190f 100644 --- a/src/runtime/futex.c +++ b/src/runtime/futex.c @@ -392,6 +392,16 @@ static futex_bucket_t *futex_waiter_follow(futex_bucket_t *b, } } +/* Whether any waiter on @uaddr is queued in @b. Caller must hold b->lock. */ +static bool bucket_has_uaddr_locked(const futex_bucket_t *b, uint64_t uaddr) +{ + for (const futex_waiter_t *w = b->head; w; w = w->next) { + if (w->uaddr == uaddr) + return true; + } + return false; +} + /* Unlink the waiter at *pp from its bucket list and wake it. The bucket lock * must be held. Unlinking before the store/signal keeps a woken waiter from * observing itself still queued; the release store pairs with the waiter's @@ -1655,18 +1665,14 @@ static int64_t futex_lock_pi_inner(guest_t *g, memory_order_acquire)) { /* Timeout: dequeue and return */ bucket_unlink_locked(b, &waiter); - /* Only clear WAITERS bit if no waiters for this address */ - bool has_waiters = false; - for (futex_waiter_t *w = b->head; w; w = w->next) { - if (w->uaddr == uaddr) { - has_waiters = true; - break; - } - } + + /* Clear under the lock: a waiter that enqueues once it is + * dropped relies on the bit it reads staying set. + */ + if (!bucket_has_uaddr_locked(b, uaddr)) + futex_clear_waiters_bit(word); pthread_mutex_unlock(&b->lock); pthread_cond_destroy(&waiter.cond); - if (!has_waiters) - futex_clear_waiters_bit(word); return -LINUX_ETIMEDOUT; } } else { From edfd4ea893735c9686ead38afac5e9d1f4da40dc Mon Sep 17 00:00:00 2001 From: xalestar Date: Fri, 16 Oct 2026 00:14:41 +0800 Subject: [PATCH 3/3] Take a PI word that has waiters but no owner futex_pi_unowned looks only at the TID field, so FUTEX_LOCK_PI on a word that holds FUTEX_WAITERS with a TID of 0 went back to the CAS(0->TID) fast path. That CAS never matches such a word, so the call spun in the host and never checked for exit_group. Linux takes the word over in futex_lock_pi_atomic. Take it under the bucket lock, and keep the bit while another waiter is queued on the address. --- src/runtime/futex.c | 21 ++++++++++++++++++++- tests/test-futex-pi.c | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/runtime/futex.c b/src/runtime/futex.c index 6e684190f..e8faff0d5 100644 --- a/src/runtime/futex.c +++ b/src/runtime/futex.c @@ -1592,8 +1592,27 @@ static int64_t futex_lock_pi_inner(guest_t *g, break; } - /* Owner released first: retry the acquisition. */ if (futex_pi_unowned(cur)) { + /* FUTEX_WAITERS with no TID never matches the fast path's + * CAS(0->TID), so take the word here, as futex_lock_pi_atomic does + * on Linux. The bit stays while another waiter is queued. + * FUTEX_OWNER_DIED is left to the recovery at the top. + */ + if (cur != 0 && !futex_pi_owner_died(cur)) { + uint32_t want = tid; + if (bucket_has_uaddr_locked(b, uaddr)) + want = futex_pi_set_waiters(tid); + bool taken; + if (!futex_word_cas(word, &cur, want, &taken)) { + pthread_mutex_unlock(&b->lock); + return -LINUX_EFAULT; + } + if (taken) { + pthread_mutex_unlock(&b->lock); + return 0; + } + } + /* Owner released first, or the word moved: retry. */ pthread_mutex_unlock(&b->lock); continue; } diff --git a/tests/test-futex-pi.c b/tests/test-futex-pi.c index c89b446a3..2093549a9 100644 --- a/tests/test-futex-pi.c +++ b/tests/test-futex-pi.c @@ -15,6 +15,9 @@ * post-robust-walk state (FUTEX_OWNER_DIED set, TID cleared) must take * the lock over and return 0, not spin on the CAS(0->TID) fast path. * + * 1c. FUTEX_LOCK_PI on a word with FUTEX_WAITERS set and no TID takes the + * lock over, as Linux does, for the same reason. + * * 2. FUTEX_LOCK_PI + FUTEX_UNLOCK_PI round-trip: acquire and * release a PI lock from the same thread. * @@ -260,6 +263,35 @@ static void test_pi_owner_died_recover(void) PASS(); } +/* Test: LOCK_PI takes a word that has FUTEX_WAITERS but no owner */ + +static void test_pi_waiters_no_owner(void) +{ + TEST("PI LOCK_PI takes a WAITERS-only word"); + + /* Linux takes this word over (futex_lock_pi_atomic). The CAS(0->TID) fast + * path never matches it, so a retry loop without a takeover spins forever. + */ + pi_lock = FUTEX_WAITERS; + + long r = raw_futex_lock_pi((uint32_t *) &pi_lock); + if (r != 0) { + printf("FAIL: LOCK_PI on a WAITERS-only word expected 0, got %ld\n", r); + fails++; + return; + } + + uint32_t tid = (uint32_t) raw_gettid(); + uint32_t val = __atomic_load_n(&pi_lock, __ATOMIC_SEQ_CST); + if ((val & FUTEX_TID_MASK) != tid) { + FAIL("lock word doesn't contain our TID after takeover"); + return; + } + + raw_futex_unlock_pi((uint32_t *) &pi_lock); + PASS(); +} + /* Test: a PI waiter is woken when the owner retakes the lock in guest code */ #define RELOCK_CONTENDED 2000 /* FUTEX_LOCK_PI acquisitions to collect */ @@ -562,6 +594,7 @@ int main(void) test_futex_eintr(); test_futex_unaligned(); test_pi_owner_died_recover(); + test_pi_waiters_no_owner(); test_pi_owner_relock(); test_pi_dead_owner(); /* Last: uses CLONE_THREAD which may hang on x64 */