tests: the strand drain-race case's rendezvous depends on spinning, and starves under Valgrind

[christianparpart]

tests/test_strand_race.cpp's case "StrandExecutor keeps one strand per key when a post races the drain" (line 334) synchronises its threads by spinning.

• Four chaser threads spin on an atomic gate, yielding once every 4096 spins (around line 403).
• The producer yields in a loop on the completed counter (around line 434).
• Nothing blocks, so a round only advances when the scheduler happens to run the pool worker that executes that round's task.

That works on a native build with enough cores. Under Valgrind, which runs one thread at a time and by default lets a yielding thread take the lock straight back, it degrades badly:

Where	Time for this one case
Locally (WSL, valgrind 3.26, gcc-debug)	1982 s
master's CI Valgrind job (run 36115939775, 64d339a)	about 45 of the job's 54 minutes
#806 CI Valgrind job (e984ebe)	more than 3 hours; the job was cancelled as hung

The rest of morph_tests takes about 3 minutes under Valgrind.

#806 works around it in CI (c9394a0, ci: valgrind schedules spinning threads fairly): the Valgrind step now passes --fair-sched=yes. That brings the case to 63 s (4 of 4 runs) and the whole of morph_tests to 217 s with 0 errors. The test's own rendezvous still depends on how threads get scheduled.

Follow-up (low priority): make the rendezvous not depend on spinning. For example, chasers could block on a condition variable until the gate opens, and spin only across the few instructions the race window needs. It must keep the case's measured ability to catch the pre-fix drain race under TSan, which is why it was not changed in #806.

Note: the core-cpp 0.4.0 switch replaces morph's strand code, and this test file with it. If that lands first, this issue closes with it. The replacement tests must not spin-wait either.

[Yaraslaut]

Triaged. Verdict: valid (well-framed; the fix direction is sound and correctly scoped away from what #806 already changed).

What I checked

Code. tests/test_strand_race.cpp on master (1034681, this worktree): the 4-chaser spin on gate yielding every 4096 spins is at lines 401-406, and the producer's yield-loop on completed is at line 434 — matches the issue's description exactly.

The two cited numbers, against primary sources rather than the issue's paraphrase.

• Run 36115939775 / job "Valgrind memcheck" (ID 108010066205, commit 64d339a). Pulling the job's raw log directly: the last line before a gap is [ERROR] [strand] task threw unknown exception at 2026-09-25T09:15:48Z; the next line of any kind is at 2026-09-25T10:01:00Z — a silent 45m12s. That matches "about 45 of the job's 54 minutes." [slow] is the only other scheduling-sensitive tag under tests/, and its case carries a DrainWatchdog that prints every 10s by default — nothing from it appears anywhere in the log, so it wasn't the one stalling. The untagged drain-race case has no watchdog, which is consistent with 45 minutes of dead air.
• PR Move morph onto core-cpp v0.5.0: coroutine model handlers on core-cpp strands, CPM dependencies #806, commit e984ebe → run 36127304451. Its first attempt's Valgrind job ran 11:03:45Z → 14:14:16Z = 3h10m31s before conclusion: cancelled (checked via gh api .../jobs/<id>). That matches "more than 3 hours; the job was cancelled as hung" exactly. (A second attempt of the same run, after the --fair-sched=yes fix, finished the whole suite in 54m3s but was cancelled separately — noting this so the two attempts aren't conflated.)

Both citations check out against the actual API/logs, not just re-stated. I did not run this under Valgrind myself: this host is macOS/arm64, where Valgrind isn't practically usable, and a from-scratch repro would cost close to the 1982s the issue already measured for no new information — re-verifying the CI artifacts directly was the higher-value check here (per the skill: "does the number it quotes still hold").

The closing condition. Fetched pull/806/head. .github/workflows/ci.yml there does have valgrind --fair-sched=yes ... with a comment explicitly saying the drain-race case "removed with morph's own strand" — and indeed git ls-tree pr-806 shows tests/test_strand_race.cpp is gone on that branch, replaced by tests/test_strand.cpp, which does not reproduce this spin/yield rendezvous pattern. So the issue's own note ("if [core-cpp 0.4.0] lands first, this issue closes with it") is accurate — but #806 is still a draft with stated unresolved gaps, not close to landing, so this issue stands on its own until then.

Invariant check

This is entirely test code (tests/test_strand_race.cpp), not include/morph or src, and touches no model, no public API, no framework seam — none of the layering/composition/value-policy/template/spec-disagreement/per-call-site invariants apply. The one real constraint is whether the proposed remedy would weaken the case's detection power, and the issue already states it: the rendezvous fix "must keep the case's measured ability to catch the pre-fix drain race under TSan" — the same constraint that kept #806 from touching it.

Reading the case, the coarse per-round wait (chasers spinning on gate, ~600 times per iteration) and the fine-grained stagger loop that walks each post across the race window are two separate loops. Only the coarse one needs to become a blocking wait (e.g. a condition variable notified when the pilot advances gate); the stagger loop — the part that actually needs sub-microsecond precision — is untouched by that change. So the proposed direction is both correct and separable from the part that must keep spinning.

What would change this verdict: if a branch merges that drops tests/test_strand_race.cpp (per the issue's own note) before this is acted on, it closes with that landing — re-open only if whatever replaces it reintroduces the same spin-based rendezvous for an equivalent case.

[Yaraslaut]

Filed #833 for the same coarse per-round rendezvous shape in this file's third case ("StrandExecutor recycles a drained strand under the key that asked for it", tests/test_strand_race.cpp), which this fix does not touch -- it's outside what this issue and its triage scoped (only the "keeps one strand per key" case was named).