Skip to content

test(sema): make contended semaphore poll/signal test deterministic#176

Merged
ran-j merged 1 commit into
ran-j:mainfrom
smmathews:feature/16-flaky-semaphore-contention-test
Jul 21, 2026
Merged

test(sema): make contended semaphore poll/signal test deterministic#176
ran-j merged 1 commit into
ran-j:mainfrom
smmathews:feature/16-flaky-semaphore-contention-test

Conversation

@smmathews

Copy link
Copy Markdown
Contributor

Problem

The ps2xTest case "Semaphore poll/signal remains stable under host-thread
contention" (ps2xTest/src/ps2_runtime_expansion_tests.cpp) is intermittently
flaky, failing the assertion "contended SignalSema should observe successful
releases".

Root cause is in the test, not the runtime. The test creates a semaphore that
is already full (init_count == max_count == 1) and then spawns a poller thread
and a signaler thread that each run 64 iterations with no coordination between
them. Because the semaphore starts full, SignalSema has no legal way to succeed
until a PollSema has first freed a slot — it returns KE_SEMA_OVF while the
count is at max. With no start barrier, under host-thread contention the
signaler can run through most or all of its 64 iterations before the poller
gets its first timeslice, so every SignalSema legitimately returns KE_SEMA_OVF
and signalOkCount ends at 0, failing the assertion even though nothing
malfunctioned. The poller side is effectively immune (a token is always
available at t=0), which is why only the signal assertion was ever seen to
fail.

That this is a genuine, recurring flake rather than a local artifact is confirmed by the
repository's own CI: this exact test and assertion failed the linux-gcc check on draft PR #174
(workflow run 29797114266), a change that touched only single-threaded memory-card code and
could not have affected semaphore behavior. Its failing on a wholly unrelated pull request is
direct evidence that the failure is scheduling-driven, not caused by the code under review.

Both syscalls serialize on the same per-semaphore mutex, so this is not a data
race — it is a scheduling-order dependency the test does not control. Locally the
failure rate is low and host-load-dependent: an earlier dedicated investigation
of the unfixed test observed 2 failures in 24 contended (8-way-parallel)
full-suite runs and none in 20 serial runs, while a later best-effort
re-reproduction of the unfixed test over 64 contended runs did not reproduce it —
consistent with a low, load-sensitive rate that CI happened to surface on PR #174.

Fix

  • Seed the semaphore with headroom: init_count = 1, max_count = 2. With a token
    present and headroom below max at the start, the first PollSema and the first
    SignalSema each succeed regardless of interleaving (the poller is the only
    decrementer, so count >= 1 at its first call; the signaler is the only
    incrementer, so count < max at its first call). This makes the test
    deterministic without serializing it.
  • Add a start barrier so both worker threads enter their loops together,
    maximizing real interleaving of the concurrent mutex acquisitions. Without
    it the seeding change alone could let the test pass while exercising zero
    contention — one worker running all 64 iterations before the other is
    scheduled — defeating what the test's name promises, so the barrier is here
    deliberately to keep the test meaningful, not as scope creep.
  • Widen the post-run count-range assertion to the new max.

No runtime or syscall code changes; the semaphore implementation is correct.

What the test still pins

  • No exceptions/crashes in concurrent PollSema/SignalSema.
  • ReferSemaStatus returns KE_OK after the contention.
  • The final semaphore count stays within [0, max]. In this test's shape — one
    guarded incrementer (the signaler) and one guarded decrementer (the poller),
    each enforcing its own bound under the per-semaphore mutex — that range is a
    boundary-guard check, not a mutex-integrity check. It would catch a guard
    regression that lets the count escape [0, max]: a SignalSema that stopped
    honoring max_count (count climbing toward 64) or a PollSema that stopped
    honoring the zero floor (count falling toward -64). It would not catch a
    broken or deleted mutex — with a single incrementer and a single decrementer
    on a naturally-aligned int the count does not tear, and a lost
    read-modify-write resolves in range and invisibly, so the bound would still
    hold. A crash or lock corruption from the concurrent access surfaces through
    the no-throw/no-crash assertions above — and, suite-wide, through no other
    test crashing — not through this count range.

The live regression signals are therefore the boundary-guard bound [0, max],
the no-throw/no-crash guarantee across concurrent PollSema/SignalSema, and the
post-contention ReferSemaStatus KE_OK; the two liveness assertions are now
true by construction of the seeding (init=1, max=2) and stand as documented
invariants that the workers actually exercised both call sites, not as
independent failure detectors.

The deterministic OVF/ZERO boundary behavior — PollSema returning KE_SEMA_ZERO
when the count is empty and SignalSema returning KE_SEMA_OVF at max — is pinned
separately by the single-threaded sibling test "semaphore EE layout covers poll,
signal overflow, and status" (ps2xTest/src/ps2_runtime_kernel_tests.cpp), which
exercises those guards deterministically; the widened [0, 2] range check here
re-covers the same bound under concurrency rather than serving as this test's
primary boundary assertion.

It can no longer produce the scheduling-dependent signalOkCount == 0 false
negative, which was never a real regression signal.

The "Semaphore poll/signal remains stable under host-thread contention"
test created the semaphore full (init == max == 1), so SignalSema could
only succeed after PollSema had already freed a slot. With no start
barrier, under host-thread contention the signaler thread could run all
64 of its iterations before the poller's first timeslice, making every
SignalSema legitimately return KE_SEMA_OVF and leaving signalOkCount at
0 — a false failure of "contended SignalSema should observe successful
releases". An earlier investigation of the unfixed test observed this
twice in 24 contended (8-way-parallel) full-suite runs and never in 20
serial runs; CI has independently hit the same assertion on the
unrelated draft PR ran-j#174, confirming the trigger is host scheduling, not
the code under review.

Seed the semaphore with headroom (init=1, max=2) so the first PollSema
and the first SignalSema each succeed regardless of scheduling order,
and add a start barrier so both workers start together, maximizing the
opportunity to interleave. Widen the final-count range check to the new
max. The semaphore implementation is unchanged; both threads still
contend concurrently on the same per-semaphore mutex.
@smmathews
smmathews marked this pull request as ready for review July 21, 2026 11:49
@smmathews

Copy link
Copy Markdown
Contributor Author

@ran-j , I'd suggest prioritizing this PR for review, as flakey tests make the CI pipeline and our agents less reliable

@ran-j
ran-j merged commit 1176609 into ran-j:main Jul 21, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants