fix(tests): make the lock-registry absolute-deadline fixture deterministic - #2043
Merged
Merged
Conversation
…istic lock_registry_absolute_deadline_survives_repeated_wakes has failed on the test-unix (macos-15-intel) leg of three unrelated pull requests in one week -- #1342 (08-28), #1811 (09-02) and #1819 (09-03, run 33799475629, job 100823626965) -- always with the same signature: FAIL tests/test_lock_registry.c:1153: ASSERT(tail_queued) 7648 passed / 1 failed The registry is not racy. cbm_lock_registry_acquire enqueues the waiter synchronously under the registry mutex before any wait, so waiter_count and the attempting count are exact. The defect is in the fixture: it raced two wall-clock windows against each other, both anchored to a timestamp the observer thread took before the tail thread had even been scheduled. deadline_start = cbm_now_ms(); tail.deadline_ms = deadline_start + 200; /* the tail's acquire deadline */ queue_deadline = deadline_start + 100; /* the observer's budget */ Because both windows start before the tail runs, a loaded runner breaks the fixture two different ways: * the tail is scheduled inside its deadline but after the observer's 100 ms budget has expired -- the queued state existed and was simply no longer being looked at; or * the tail is scheduled more than 200 ms late, in which case its deadline has already passed when it finally calls acquire, the pre-registration deadline check in lock_registry_acquire_internal returns BUSY immediately, and the tail never enqueues at all -- the asserted state can then never occur, however long the observer waits. lock_registry is in the parallel wave of run-tests-parallel.sh, not the serial tail, so on a small CI runner it competes with a full wave of sanitized suites -- exactly the scheduling delay both paths need. Widening the observer's budget would only paper over the first path, and the state it waits for is transient by construction: it exists only between the tail's enqueue and the tail's own deadline. So the fixture is rebuilt to observe states that cannot evaporate. * The tail anchors its absolute deadline itself, in its own thread, immediately before the acquire it bounds. Scheduling delay can no longer consume the deadline before the call starts, so the enqueue is unconditional, and elapsed is measured from the tail's own anchor -- it now times the registry instead of timing the scheduler. * Enqueue is exposed as a monotonic counter, cbm_lock_registry_waiter_enqueue_count_for_test, next to the existing test_condition_wait_calls counter it is modelled on. Because the count only ever grows, the observer reads it once after the tail has returned rather than trying to catch a live queue depth: the polling loop, and with it the window, is gone. * The fixture's remaining 500 ms and 600 ms budgets become the file's LOCK_REGISTRY_TEST_TIMEOUT_MS backstop, and each loop exits on the state it waits for instead of on the clock, so the backstop only fires when the product is actually broken. The contract is unchanged: the tail must still return at its absolute deadline (150 <= elapsed < 350 ms for a 200 ms deadline) despite ~40 unrelated cancel broadcasts, still with BUSY and no lease, with the head still holding the attempt. The broadcast loop now starts immediately after the tail is released, so it overlaps the tail's wait at least as much as it did before. Verification, all on macOS arm64 with the sanitized runner: * The mechanism was reproduced locally by delaying only the tail thread after its start gate, with production untouched. A 120 ms delay (the state exists, outside the observer's budget) and a 250 ms delay (the tail never enqueues) each produced exactly one failure, ASSERT(tail_queued) -- the CI signature. * After the rebuild the same injections are green at 120, 250, 400 and 900 ms: an arbitrary scheduling delay no longer decides the verdict. * lock_registry 30/30 green plain and 30/30 green under four CPU hogs; 16/16 inside the saturated 18-job parallel wave. * private_file_lock, lock_registry, daemon, project_lock and the daemon_* suites: 222 passed, 1 skipped. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
DeusData
force-pushed
the
fix/lock-registry-deadline-test-deterministic
branch
from
September 4, 2026 13:42
7109e5e to
7e47043
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The flake
lock_registry_absolute_deadline_survives_repeated_wakeshas failed on thetest-unix (macos-15-intel)leg of three unrelated pull requests in one week,always with the same signature:
test-unix (macos-15-intel)test-unix (macos-15-intel)33799475629, job100823626965Attribution: the registry is fine, the fixture is not
cbm_lock_registry_acquireenqueues the waiter synchronously under the registrymutex, before any wait (
lock_registry_waiter_pushinlock_registry_acquire_internal), sowaiter_countand the attempting count areexact. There is no production race here.
The defect is that the fixture raced two wall-clock windows against each other,
both anchored to a timestamp the observer thread took before the tail thread had
even been scheduled:
Neither window is anchored to the tail's actual registration, so a loaded runner
breaks the test two different ways:
its own deadline but after the observer's 100 ms budget has expired.
late; its absolute deadline has already passed when it finally calls acquire, so
the pre-registration deadline check returns
BUSYimmediately and the tail neverenqueues. No amount of extra observer patience would help.
lock_registrysits in the parallel wave ofrun-tests-parallel.sh, not theserial tail, so on a small CI runner it competes with a full wave of sanitized
suites — exactly the scheduling delay both paths need.
RED first, with production untouched
Delaying only the tail thread after its start gate — a faithful model of "this
thread did not get scheduled promptly", with production untouched — reproduces the
CI signature exactly, one failure and nothing else:
(120 ms exercises path 1, 250 ms exercises path 2; line 1160 rather than 1153
because the temporary delay hook adds seven lines. The hook is not part of this PR.)
The rebuild: wait for states that cannot evaporate
Widening the observer's budget would only paper over path 1, and the state it waits
for is transient by construction — it exists only between the tail's enqueue and the
tail's own deadline. So the fixture now observes states that cannot disappear:
before the acquire it bounds. Scheduling delay can no longer consume the deadline
before the call starts, so the enqueue is unconditional — and
elapsedis measuredfrom the tail's own anchor, so it times the registry instead of timing the scheduler.
cbm_lock_registry_waiter_enqueue_count_for_test, modelled on the existingtest_condition_wait_callscounter right next to it. Because the count only evergrows, the observer reads it once, after the tail has returned, instead of
polling for a live queue depth. The polling loop — and with it the window — is gone.
LOCK_REGISTRY_TEST_TIMEOUT_MSbackstop, and every loop exits on the state it iswaiting for rather than on the clock, so the backstop only fires when the product
is actually broken.
No budget was tuned, and no assertion was relaxed. The contract still holds: the tail
must return at its absolute deadline (
150 <= elapsed < 350ms for a 200 msdeadline) despite ~40 unrelated cancel broadcasts, still
BUSY, still no lease, withthe head still holding the attempt. The broadcast loop now starts immediately after
the tail is released, so it overlaps the tail's wait at least as much as before.
Production changes are three lines and one accessor: a relaxed atomic increment under
a mutex already held, a scrub on retirement, and the
_for_testreader. No behaviourchanges.
Verification (macOS arm64, sanitized runner)
ASSERT(tail_queued), 1 failure eachlock_registryx30, plainlock_registryx30, under 4 CPU hogslock_registryin the saturated 18-job waverc=0 pass=16 fail=0private_file_lock lock_registry daemon project_lock daemon_*run-tests-parallel.sh, 141 suites, 18 jobsmake -f Makefile.cbm lint-ci