Repository navigation
Set FUTEX_WAITERS under the bucket lock in LOCK_PI - #434
Conversation
| * 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 |
There was a problem hiding this comment.
The timeout exit still breaks this invariant: it unlinks the waiter, drops b->lock, and only then calls futex_clear_waiters_bit. A second waiter that takes the lock in that gap sees FUTEX_WAITERS already set, takes the futex_pi_has_waiters break and parks. The bit is then cleared under it, and the owner's next release is a guest-side CAS that makes no syscall, which is the same lost wake this change fixes. Moving the futex_clear_waiters_bit(word) call above pthread_mutex_unlock(&b->lock) in that branch is safe for the same reason the marking loop is safe under the lock, and it would make this comment true. Otherwise, qualify the comment and keep it on the follow-up list the description already carries.
There was a problem hiding this comment.
Moved futex_clear_waiters_bit(word) above pthread_mutex_unlock(&b->lock) in 097546b, so the check for other waiters and the clear happen under one hold of the bucket lock, and the comment holds as written. There is no test for this path: a stress case that raced an expired FUTEX_LOCK_PI against a parking one passed 16 of 16 runs with only this change reverted, because the gap between the unlock and the clear is a few instructions.
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 sysprog21#413
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.
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.
47cfc1e to
edfd4ea
Compare
|
Thank @xalestar for contributing! |
Fixes #413.
pthread-tls-probehung because the guest main thread was parked inFUTEX_LOCK_PIand nothing woke it. It never reachedpthread_join. The lock is one that Rosetta's runtime takes around its own work: the futex word is at 0x8000001a2440, inside the translator's range, and the worker thread takes and releases it 13 times before it exits.futex_lock_pi_innersetFUTEX_WAITERSbefore it took the bucket lock, and under the lock it 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, soFUTEX_UNLOCK_PInever ran. The waiter's 100 ms poll leaves only forexit_group, a signal,FUTEX_OWNER_DIED, or a dead owner tid, and a word of 0 is none of those.A temporary trace, not part of this change, showed the same three reads in each of three hangs: the fast-path CAS saw 0x2, the loop read 0, and the read under the bucket lock saw 0x2 again, with no
FUTEX_UNLOCK_PIafter 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.cgains one case: two threads take and release one PI lock and enter the kernel only on contention, until 2000 acquisitions have gone throughFUTEX_LOCK_PI. That call carries a 2 s deadline, so a lost wake is reported as a failure and does not hang the suite. The case also fails when no acquisition needed the kernel, so it cannot pass without contention.Based on
mainat b57f367. Measured on an Apple M5 Pro, macOS 27.0.1, with no other elfuse test running:main; 0 of 1000 timed out with this change.make test-rosetta-glibc: 5 runs in a row pass, 7 of 7 probes each.test-futex-pi: reported a lost wake in 20 of 20 runs onmain, and in 0 of 20 runs with this change.make check: all sections pass.make test-matrix-elfuse-aarch64: 296 passed, 1 failed, 10 skipped. The failure istest-sigioon the TCP out-of-band SIGURG case, which is intermittent on both trees: standalone it failed 9 of 20 runs on unmodifiedmainand 10 of 20 runs on this branch.make test-matrix-elfuse-x86_64: 77 passed, 1 failed, 1 skipped. The failure isaudit-known-limitations(rt_sigreturnassertion inside Rosetta, rc=133). It fails 3 of 3 runs on unmodifiedmainand 3 of 3 runs on this branch.make lint,make analyze,make check-format,.ci/check-cppcheck.shandmake verify-futexdeadlinepass.make analyzereports nothing insrc/runtime/futex.c.Two more commits close gaps found in review:
FUTEX_LOCK_PIclearedFUTEX_WAITERSafter it dropped the bucket lock, so a waiter that enqueued in that gap could park on an owned word without the bit. It now clears under the lock. This has no test: a stress case of 2000 rounds passed 16 of 16 runs with only this fix reverted, 6 of them with every core loaded, because the gap is a few instructions long.FUTEX_LOCK_PIon a word holdingFUTEX_WAITERSwith a TID of 0 spun in the host, since theCAS(0->TID)fast path never matches it. It now takes the word over, as Linux does infutex_lock_pi_atomic. The newtest-futex-picase hangs on the previous head and passes with the change.After these,
make check,make lint,make check-format,.ci/check-cppcheck.shandmake verify-futexdeadlinepass again.Not changed here, and left for follow-up pull requests because none of them is needed for this hang:
FUTEX_UNLOCK_PIwrites 0 and wakes one waiter; it does not hand the lock to that waiter. With two or more waiters the others stay parked on a word withoutFUTEX_WAITERS.Summary by cubic
Fixes #413 by setting
FUTEX_WAITERSand enqueuing the waiter under one hold of the bucket lock inFUTEX_LOCK_PI. Previously a waiter could park on an owned word with the bit clear when the owner released and retook the lock in guest code between the bit set and the enqueue; the owner's next release then made no syscall, soFUTEX_UNLOCK_PInever ran and the waiter hung.pthread-tls-probehit this.Two related lost-wake paths are closed the same way:
FUTEX_LOCK_PItimeout path clears the bit under the bucket lock, so a waiter that enqueues after reading it can't be left parked without it.FUTEX_LOCK_PItakes over a word that hasFUTEX_WAITERSbut no owner, as Linux does, instead of spinning on theCAS(0->TID)fast path.Adds cases to
tests/test-futex-pi.c: one runs two threads through 2000 contended PI acquisitions with a 2 s deadline on eachFUTEX_LOCK_PIcall, requiring all 2000 to have contended so a lost wake reports a failure instead of hanging the suite; a second covers takeover of aFUTEX_WAITERS-only word.Written for commit edfd4ea. Summary will update on new commits.