fix(websocket): apply pause/resume under the lock that decides them, and lift a stale pause (celeris#667, celeris#672) - #671
Conversation
ca4ad57 to
3f2dcde
Compare
Not merging this alone: measured, it makes #672 worse at the production defaultThe corrected A/B answered the question it was rebuilt to answer, and the answer blocks the Throughput: inconclusive, and the old headline is withdrawn. At round-level analysis The wedge rate is the decisive result, and it goes against this PR. Measured directly per
Connections wedged at cap256: 556/640 → 615/640. Median ops-to-first-wedge: 285,066 → 170,726. And the remedy is already measured. Arm C — this fix plus a #672 stale-pause re-check inside So #667 and #672 should land together. Fixing the reordering while leaving the stale-pause hole Also to address before this merges
Credit where due: M1–M5 are genuinely resolved, and both re-reviewers re-derived the numbers |
…the lock that decides it
chanReader decides to pause under pausedMu and applies the decision to the
engine after releasing it (engineread.go:230-232), and Read's resume branch
does the same in reverse (engineread.go:311-313). The engine's closures are
Swap-based and ignore the previous value (engine/iouring/worker.go:2200-2231),
so whichever callback reaches the engine last wins outright and nothing
reconciles.
Two failing-first tests, both engine-free and both deterministic:
TestChanReaderLostResumeConverges - the appender parks inside pause() after
unlocking; the handler drains to lowWater, clears pausedState and applies
resume() into an engine that is not paused yet; the pause then lands. The
engine's recv ends up PAUSED while the reader believes it is RUNNING, and
the reader is edge-triggered, so it never resumes that connection again.
TestChanReaderParkedResumeConverges - the symmetric direction. The handler
parks inside resume(); a later pause lands first and the resume overwrites
it. The engine keeps delivering while the reader believes it is paused, so
backpressure is silently gone.
No timing on the failing path: the order of the two applications is fixed by a
channel close that strictly happens-before the parked callback's Swap, so the
outcome is forced rather than sampled. Both tests carry anti-vacuity guards
(exact pause/resume callback counts, and proof that the park actually
happened), so a run in which the watermark crossing never fired fails as a
harness error instead of passing quietly.
The park is scheduled by a TryLock probe of pausedMu, not asserted on, so the
tests do not mandate one particular fix and cannot deadlock on a build that
closes the window: if a callback is ever invoked with the deciding lock held,
the park is skipped and the same convergence assertion runs.
Measured on 6136554: 40/40 FAIL over 20 iterations of each test under
-race, 0 passes. With r.pause()/r.resume() moved inside pausedMu: 40/40 PASS.
The fix itself is deliberately NOT included here - issue #667 requires the
contention it adds to be A/B measured before it can be merged.
…em (celeris#667)
chanReader decided to pause under pausedMu and applied the decision to the
engine after releasing it (engineread.go:225-232), and Read's resume branch did
the same in reverse (engineread.go:309-316). The engines' PauseRecv/ResumeRecv
closures are Swap-based and ignore the previous value -- their early return
skips only the eventfd wakeup, never the state write
(engine/iouring/worker.go:2200-2231, engine/epoll/loop.go:1672-1703) -- so
whichever callback reached the engine LAST won outright and nothing reconciled
the two. A resume could be applied before the pause it was meant to cancel,
leaving the engine's recv PAUSED while the reader believed it was RUNNING. The
reader is edge-triggered, so it never asked again: that connection stopped
delivering inbound data for the rest of its life.
Both callbacks are now invoked with pausedMu held, so the order the engine
observes equals the order of the pausedState transitions.
The fix is engine-agnostic. chanReader is the only implementation of this
decide-then-apply pair (`git grep pausedMu` finds engineread.go and tests only),
so there is no epoll-side copy to change; both engines' closures already have
the identical Swap shape and are unmodified here.
LOCK ORDER: pausedMu -> detachQMu, documented on requestPause. It cannot cycle.
pausedMu is an unexported field of an unexported type in this package and is
taken in exactly two places, both here; the engine packages do not import
middleware/websocket, so no detachQMu holder can reach either. All 24 detachQMu
critical sections in both engines (23 in engine/{iouring,epoll}, 1 in a test)
were read: every one is a straight-line queue append plus an atomic store with
no call out of the engine, and both drainDetachQueue implementations release
the lock before their loop. The eventfd write is on an EFD_NONBLOCK descriptor
and happens after detachQMu is released, so it cannot block under pausedMu.
pausedMu never nests with spillMu either: refillFromSpill returns before the
resume branch, spillChunk returns before requestPause, and hasSpill is atomic.
Measured, in Docker on linux/arm64, arms interleaved round by round, 20 samples
each, medians with a Mann-Whitney U test and an A/A control for the noise floor
(scripts and raw logs: wf-celeris-667-logs/ab, regenerate with report.sh):
- End to end (8 WS clients, MaxBackpressureBuffer 16): epoll is unchanged at
both memlocks (-0.5%/+0.4%, p>=0.40) and io_uring with 4 workers is
unchanged (+0.5%, p=0.67). io_uring capped to ONE worker -- the contended
shape -- is +16.7% ns/op / -14.4% MB/s, p=0.13 against an A/A floor of ~6%:
suggestive, not significant.
- Isolated chanReader: where the watermarks are crossed every few chunks the
cost is real and significant (+45% like-for-like, p<0.002 at both memlocks);
at the 256-chunk production default +5.4% (p=0.001) and +25.1% (p=0.0003)
in two batches whose A/A floors were +0.2% and +2.3%.
- The no-edge control, where the fix cannot matter, shows no fix-specific
difference.
This is a correct-but-slower change on the contended io_uring shape and is
deliberately NOT presented as free.
The A/B also measured the bug itself: with the watermarks crossed every few
chunks, 9 of 20 (8 MiB) and 13 of 20 (128 MiB) BASE samples lost a pause/resume
mid-run and degenerated -- their edge rate collapsed ~40x. 0 of 40 fix samples
degenerated.
Adds middleware/websocket/engineread_bench_linux_test.go, the benchmarks the
A/B is built on. They report their own edge counts so the two arms can be shown
to have run the same trigger, and the no-edge control asserts that no callback
fired.
…(celeris#667) The two celeris#667 regression tests ran in no CI job, and neither did the other ~14 chanReader unit tests: the root race job excludes ./middleware/websocket, and the one websocket step selected `-run '^TestBackpressure'`, which matches neither of them. Widen that step to `^(TestBackpressure|TestChanReader)` and add the interlock the `adaptive` (#660) and `iouring` (#664) jobs established. `-run` is a regex, so a rename would select nothing while `go test` still exits 0; the tally requires exactly the two celeris#667 tests to report PASS, and a skipped test prints `--- SKIP`, never `--- PASS`, so a skip cannot satisfy it either. Unlike those two jobs this needs no raised privileges and has no environment skip to forbid: the tests are in-process over newChanReader, with no sockets, no engine and no io_uring ring. `-v` is required for the tally, and the budget goes to 300s because the step now runs two suites with per-test output; the measured wall time of this exact shape is ~50s. Proved in the CI shape (golang:1.27, -race, memlock 8 MiB, the step body extracted verbatim from ci.yml and run as `bash --noprofile --norc -eo pipefail`): fix rc=0 with passed=2; base rc=1 with both tests FAIL; both tests renamed out of the namespace rc=1 on the interlock; both tests skipped rc=1 on the interlock.
… drained to empty TestChanReaderStalePauseConverges lays out the celeris#672 interleaving in one goroutine, so the scheduler decides nothing: the crossing append's channel send and depth snapshot, then the handler draining to empty through the real Read, then the crossing append's requestPause applying the now-stale decision. The oracle is the quiesced state: nothing is buffered, so the engine must not be left paused, the reader's view must agree with the engine's, and the next delivered chunk must reach the next Read. It fails at every capacity swept (1, 8, 16, 256) on this commit, which already carries the celeris#667 fix: #672 is a stale decision applied in the right order, not two applications reordered.
…eleris#672) requestPause applies a decision Append took from a depth snapshot before pausedMu was held. If the handler drained to empty in between, every resume check it made saw pausedState == false, the pause then landed on an empty buffer, and Read, which re-evaluates the resume only after a successful dequeue, could never lift it: the connection stopped delivering for good. After applying the pause, requestPause now re-checks the watermark under the same lock and resumes at once if the depth is already at or below lowWater with nothing spilled. Once pausedState is true under the lock, every later dequeue is followed by a resume check that sees it, so the pause cannot go stale again. This is arm C of the PR #671 A/B, byte-identical in code to the measured variant (sha256 123165ca...); only its comment changed. It wedged 0/640 connections at cap16 and 0/640 at cap256 there, against 615/640 for the celeris#667 fix alone at cap256.
… panics Since celeris#667 both engine callbacks run with pausedMu held, and the lock is released by a plain Unlock after them. A panic inside a callback skips the Unlock, so when a caller recovers it the reader keeps pausedMu locked for good: every later resume check in Read blocks, and so does the next high-water crossing in Append, which runs on the engine worker thread. TestChanReaderCallbackPanicReleasesPausedMu panics once inside each of the three callback sites (the pause in requestPause, the resume in Read, the celeris#672 re-check's resume in requestPause), recovers, and probes the lock with TryLock, so a regression fails instead of hanging. All three fail on this commit.
…nnot keep it Both critical sections that apply an engine callback under pausedMu now release it with defer: requestPause, and Read's resume branch, which moves into resumeIfDrained so the deferred unlock ends with the decision instead of extending the hold over Read's copy. A callback that panics no longer leaves pausedMu held when a caller recovers, which would have blocked every later resume check in Read and the next high-water crossing in Append on the engine worker thread. No behaviour change when nothing panics: the decisions, their order and the set of statements under the lock are the same.
The #667 comments pointed at engine/iouring/worker.go:2200-2231 and engine/epoll/loop.go:1672-1703 for the PauseRecv/ResumeRecv closures; the rebase onto the celeris#657 work moved both, and any later engine change would move them again. Name the closures instead, and mark the listings of the pre-fix code as what engineread.go was at 6136554. The benchmark's escape-hatch comment now says what the hatch is for on a tree that carries the celeris#672 fix.
TestMeasureWedgeRate is the harness behind the PR #671 wedge numbers. Until now it lived only with the measurement logs, so the result that decides this PR could not be reproduced from the tree. It removes the benchmark's escape hatch, drives each connection to its first wedge or its op budget, detects a wedge structurally (engine paused, buffer empty, no reader progress across a bounded number of yields) and prints one WEDGE_CONN line per connection and a WEDGE_SUMMARY per run, with per-connection anti-vacuity checks. The code is the measured harness unchanged; only its header comment is new: it now says that this tree is the negative control, how to build the base and #667-only arms from git, and the command for one observation. It skips unless WS667_WEDGE is set, and its name stays outside ^TestChanReader so the CI step that selects the chanReader tests never sweeps it in.
…nterlock The websocket step's exact-PASS tally now covers four regression tests instead of two -- the two celeris#667 orderings, the celeris#672 stale pause, and the pausedMu panic release -- and each of the seven subtests of the last two, so a rename, a skip, or a capacity or callback site dropped from a sweep fails the step instead of passing it quietly. The PASS patterns end at ` (`, never at the test name.
3f2dcde to
d3472e7
Compare
Update: #672 now lands in this PR, and every point above is addressedFollowing up on the comment above. The branch is rebased onto main
Also new since the last revision:
CI on |
…he callbacks tryEngineUpgrade calls SetPauser after Detach and after the 101 is written, on the goroutine running the upgrade, while the engine worker may already be appending the peer's first frames. An Append that crosses highWater reads r.pause in requestPause, and since celeris#672 r.resume in its re-check. Nothing orders those reads with SetPauser's writes: detachQMu, the only lock the upgrade shares with the worker, is released before SetPauser runs. TestChanReaderSetPauserOrderedWithAppend starts the two sides together with no synchronisation between them. Under -race it fails on this tree at both reads (engineread.go requestPause's nil check and the re-check); without -race it skips, since there is no wrong value to observe.
SetPauser now writes r.pause and r.resume under pausedMu, and requestPause reads r.pause under it (the nil check moves below the Lock; the celeris#672 re-check's read of r.resume was already under it). That is the happens-before edge the upgrade goroutine and the engine worker lacked. Nothing changes in steady state: with the callbacks installed, requestPause took pausedMu anyway. Only a reader with no callbacks now takes the lock before returning. Read's reads of r.resume stay unlocked, because they run on the handler goroutine, which the upgrade starts after SetPauser returns.
…#666 lock order) celeris#666, which the rebase brought under this branch, made the engines' pause/resume callbacks wake the loop through wakefd.WakeFD.Signal, which takes a sync.RWMutex read lock. Since celeris#667 those callbacks run with pausedMu held, so pausedMu now nests that read lock, and sync.RWMutex queues a new reader behind a waiting writer. The lock-order note on requestPause predated that and said nothing about it. The note is re-derived: under pausedMu the callbacks take detachQMu and then, after releasing it, the WakeFD read lock. Both are leaves. WakeFD's writers, Set and Close, run only on the loop's own thread, are never reached from the callbacks, and hold the lock only across fcntl, close(2) and atomic stores, so a writer never waits on pausedMu, holding the write lock or queued for it. TestChanReaderWakeFDWritersNeverWaitOnPausedMu forces the three ways a writer and a callback holding pausedMu can meet, with the production WakeFD and callbacks built like the engines' own: Close while a pause holds pausedMu, Set while a resume holds it, and a pause callback queued behind a Close that is itself queued behind another producer's in-flight write(2). Each must complete while pausedMu is held; a watchdog turns a deadlock into a failure.
…sured arm, 3b6c500 The header told a reader to build the #667-only arm by deleting the celeris#672 re-check from this tree's engineread.go. That is not the arm PR #671's wedge A/B measured: it ran 3b6c500's engineread.go (sha256 2b64141c...), which also predates the deferred unlocks and resumeIfDrained. The recipe is now the git show that reproduces the measured arm.
…anReader interlock The websocket step's exact-PASS interlock now requires six regression tests and ten subtests: the four it had, TestChanReaderSetPauserOrderedWithAppend (which the step's -race makes observable) and TestChanReaderWakeFDWritersNeverWaitOnPausedMu with its three forced interleavings.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe WebSocket reader now applies pause and resume callbacks under ChangesWebSocket reader pause synchronization
Priority: ⬆️ High Estimated code review effort: 4 (Complex) | ~60 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The CI change adds checks that the named WebSocket regression tests run and pass. No issue identified here prevents merging after normal checks. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 70.37% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 27 functions across 5 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Round 3: every review point, what I did, and the proofHead
Where this leaves the PR. Every point is closed except throughput, which I could not measure on this host. The throughput measurement is pre-registered and scripted ( Before the push (RULE 21): the whole |
Round 4: throughput measured, stall wording correctedThe head is unchanged at
Conduct of the run. The timing lock was held only while celeris#674's lane did not want it. The matrix waited until #674's B1 was terminated at 08:44:40Z. It paused once at a block boundary, 09:03:53–09:15:24Z, when the orchestrator asked for the lock for #674. No round was unusable or interrupted, and no process of the user or of another lane was touched. Foreign load, disclosed: another lane's native Where this leaves the PR. By the pre-registered rules the throughput condition is met: no cell is slower than its floor, and the replication of the borderline cell passes. The residual risk is a cost of up to about 2.5% on io_uring with four workers, which this n cannot exclude. If that cost is real, the likely source is holding |
Round 5: the tail in full, every number traced to a scriptThe head is unchanged at The body is updated. Evidence is in
One precision on the suggested wording. epoll runs four loops in both shapes: every round of both shapes logs Also found while tracing. The W2X rig in the same timing rounds stalled in 23 of 48 #667-only rounds and in none of the others. The body now reports this as descriptive, not pooled. |
Resolve the one conflict in .github/workflows/ci.yml: main renamed the root race step to "(excluding test/ + adaptive/ + websocket + engine/iouring)" (celeris#662 r6), and this branch added two comment lines above that step. Keep both: this branch's comment and main's step name. The websocket step is still the last step of the unit job, so the comment still holds.
celeris#705: a chunk that spills after the handler has drained the channel is never promoted. TestChanReaderLateSpillReachesParkedRead lays out the interleaving the issue reports (Append's select finds the channel full, the handler drains it and parks in Read, then spillChunk and requestPause run) and requires the parked Read to get the chunk. TestChanReaderSpillPublishedAfterDrain covers the other order: the handler drains while spillChunk is queuing the chunk, before spillLen is published; the next Read, after the publication, after a close, or already waiting in the window, must still get it. Its sequential cases also require the stale-pause re-check to keep a pause while a chunk is spilled, which the #671 review's mutant M5 does not (celeris#716, item 1). celeris#706: startNativeServerWithHandle never read Start's error. TestStartNativeServerFailsFastOnStartError starts a server whose Start fails at once (an engine type no factory knows) and requires the helper to fail with that error, at once, not with "server not ready within timeout" after 30 s. All three fail on main.
…716 item 1) The #671 review found that no test kills mutant M5, requestPause's celeris#672 re-check without its !r.hasSpill() term, and that #705's repro cannot. TestChanReaderSpillPublishedAfterDrain's sequential cases (from the test commit) require the pause to be kept while a chunk is spilled behind an empty channel. TestChanReaderPauseRecheckHoldsWhileSpilled adds the case where the guard is all that stands between the engine and a resume above lowWater: the re-check runs with the channel at lowWater and a full spill behind it (MaxBackpressureBuffer 2, BackpressureHighPct 100, BackpressureLowPct 50, chunks delivered before SetPauser, a Read between spillChunk and requestPause), and M5 resumes the engine with three chunks buffered against lowWater 1. The test passes on main's engineread.go as well as on the #705 fix; it fails only without the guard.
Summary
This PR fixes celeris#667 and celeris#672 together, because the measurement says they must ship together: the #667 fix alone makes #672's wedge more likely.
chanReaderdecided pause/resume underpausedMuand applied the decision to the engine after releasing it, so the appender and the drainer could reach the engine in the opposite order from the one in which they decided. Both callbacks now run withpausedMuheld.requestPauseapplies a decisionAppendtook from a depth snapshot beforepausedMuwas held. If the handler drained to empty in between, the pause landed on an empty buffer and nothing could ever lift it.requestPausenow re-checks the watermark under the same lock after applying the pause, and resumes at once if the pause is already stale (arm C of the earlier A/B, code-identical to the measured variant).pausedMuis released bydefer, so a callback that panics cannot leave it held.SetPauseris ordered with the engine worker (round 3): it writes the callbacks underpausedMu, andrequestPausereads them under it. That closes a pre-existing data race, which the middleware/websocket: a pause decided on a stale depth snapshot wedges the connection permanently when the handler drains to empty first (distinct from #667) #672 re-check had widened.WakeFDwriter can wait onpausedMu.Closes #667
Closes #672
Refs #705
celeris#705 is not fixed here. It is the stranded spill: a chunk spilled after the handler drained the channel is never promoted. It predates this PR, and mutant M5 survives because of it. It will be fixed on top of this PR.
Measured end to end on real sockets at
371c76a(pre-registered; 192 rounds, one container each; main, an A/A floor, the #667 fix alone and this PR; the CI shape at memlock 8 MiB and at 128 MiB; the backpressure and echo tests under-race, plus the earlier A/B's echo rig):engineread.goagain. The middleware/websocket: chanReader applies pause/resume to the engine outside the lock that decides them, so a resume can be lost and the connection stops delivering (found by reading, not reproduced) #667 fix alone: 9/48. The stall class was defined after the rounds ran, so its p-values are post hoc (middleware/websocket: chanReader applies pause/resume to the engine outside the lock that decides them, so a resume can be lost and the connection stops delivering (found by reading, not reproduced) #667 alone vs main, p = 0.0013). The pre-registered contrast is round failures: middleware/websocket: chanReader applies pause/resume to the engine outside the lock that decides them, so a resume can be lost and the connection stops delivering (found by reading, not reproduced) #667 alone 9/48 vs main 2/48, one-sided p = 0.025. It replicates the earlier A/B's finding that middleware/websocket: chanReader applies pause/resume to the engine outside the lock that decides them, so a resume can be lost and the connection stops delivering (found by reading, not reproduced) #667 alone raises failures. Every failure of this PR was an io_uring engine start at 8 MiB, before any upgrade (celeris#706).pausedMuabout once per 440–650 ops, for 80–180 µs on average. That is about ten times as often as on main, where most such waits last a few µs. It costs 140–375 ns per op, 0.02–0.04% of an op.Measured at the unit level (round 2, unchanged): the #672 wedge rate was 0/640 connections at cap256 and 0/640 at cap16, against main's 560/640 and 515/640. For the #667 fix alone it was x3.25 and x14 main's per-edge rate.
What this revision changes (round 5)
No code change and no new measurement. The head is still
371c76a, and its CI (run 36284528042) is unchanged. This round corrects the reporting (the re-review of round 4, two MINOR points):round5/51-STALLS.txt;52-TAIL.txt;53-MDE-CONFIRM.txt. This corrects12-CONFIRM.md, which said the replication detects +2.3% with about 80% power. The saved computation gives 0.93.What this revision changes (round 4)
No code change. The head is still
371c76a, and its CI (run 36284528042) is unchanged. This round adds:What this revision changes (round 3)
Fast-forward from
d3472e7to371c76a:9122766TestChanReaderSetPauserOrderedWithAppend, failing-first (review NIT: the SetPauser race)0a3492dSetPauserwrites, andrequestPausereads, the callbacks underpausedMu651d895TestChanReaderWakeFDWritersNeverWaitOnPausedMu, and the lock-order note re-derived (review MINOR: celeris#666)69bcd1c3b6c500, the arm actually measured (review NIT)371c76aThe rest of the review is answered in this body:
-vcaveat (NIT).What the rebase changed
Rebased from
d4bc70bonto main9f4d89b(7 commits: #666, #676, #677, #678, #680, #681, #687). None of them touchesmiddleware/websocket;git range-diffshows the three original commits patch-identical (=),engineread.goat the #667 fix commit is still sha2562b64141c…, and the only file both sides changed isci.yml, where main added the celeris#657 steps above this PR's step without a conflict. Build,go vetandgo test -cof./middleware/websocket,./engine/epolland./engine/iouringpass forlinux/amd64andlinux/arm64;gofmtandgolangci-lint(v2.13) report nothing.What the rebase did move is the engine's
PauseRecv/ResumeRecvclosures (review point): the comments citedengine/iouring/worker.go:2200-2231andengine/epoll/loop.go:1672-1703, which are now2342/2356and1770/1784. The comments now name the closures instead of lines, so the next engine change cannot stale them again.One commit in that range does bear on this PR: celeris#666 made the engines' pause/resume callbacks wake the loop through
wakefd.WakeFD.Signal, which takes an RWMutex read lock, and those callbacks run underpausedMu. The round-2 lock-order note predated it; it is re-derived below.celeris#672: mechanism and fix
Appendsends a chunk, then decides fromlen(r.ch) >= highWaterthat the engine should pause, andrequestPauseapplies that decision after takingpausedMu. If the handler drains the channel to empty in that gap, each of its resume checks runs whilepausedStateis still false and does nothing; then the pause lands on an empty buffer.Readre-evaluates the resume only after a successful dequeue, and none can happen again: the engine is paused, so it delivers nothing, and nothing is buffered.The fix, at the end of
requestPause, under the lock that applied the pause:Once
pausedStateis true under the lock, every later dequeue is followed by a resume check inReadthat sees it, so the pause cannot go stale again after this critical section ends.SetPauser is now ordered with the engine worker (review point)
tryEngineUpgraderegistersAppendas the data sink beforeDetach, and callsSetPauseronly afterDetachand after the 101 is written, on the goroutine running the upgrade. On an async-mode connection that goroutine is not the engine worker, and from the moment the 101 is on the wire the worker may be appending. AnAppendthat crosses highWater readr.pauseinrequestPausewithout a lock, and the #672 re-check added a read ofr.resumethere. Nothing ordered those reads withSetPauser's writes:detachQMu, the only lock the upgrade shares with the worker, is released beforeSetPauserruns. The race predates this PR (main readsr.pausethe same way); the re-check widened it by one field.The fix is local (
0a3492d).SetPauserwrites both callbacks underpausedMu, andrequestPausereadsr.pauseunder it: the nil check moves below theLock, and the re-check was already under it. With the callbacks installed,requestPausetookpausedMuanyway, so nothing changes in steady state.Read's reads ofr.resumestay unlocked, because they run on the handler goroutine, which the upgrade starts afterSetPauserreturns.TestChanReaderSetPauserOrderedWithAppend(9122766, one commit before the fix) starts the two sides together, with nothing synchronising them. Under-raceit fails on the commit before the fix at both reads (engineread.go:252forr.pauseand:283forr.resume, each againstSetPauser), and on main'sengineread.goat ther.pauseread. It passes on the fix. Without-raceit skips rather than passing vacuously; CI runs it under-race. Mutants M10 (unlockedSetPauser) and M11 (nil check back above theLock) are both killed by it.Lock order after celeris#666 (review point)
The previous revision's safety case for holding
pausedMuacross the engine callbacks predated celeris#666, which the rebase brought under this branch. #666 made the callbacks wake the loop throughwakefd.WakeFD.Signal, andSignaltakes async.RWMutexread lock. SopausedMunow nests a read lock of an RWMutex whose writers run on the loop thread, andsync.RWMutexqueues a new reader behind a waiting writer. Re-derived at371c76a:Locks taken under
pausedMu. The callbacks are the engines'PauseRecv/ResumeRecvclosures (engine/epoll/loop.go,engine/iouring/worker.go). They take two locks, one after the other and never nested:detachQMu, then, only when their append took the detach queue from empty to non-empty and afterdetachQMuis released,WakeFD.mufor reading. So the edges out ofpausedMuarepausedMu → detachQMuandpausedMu → WakeFD.mu (R).SetPausernow also takespausedMu(previous section), but it takes nothing under it.Neither lock ever waits on anything that could lead back, so no cycle can pass through
pausedMu, whatever a caller holds when it takes it:detachQMu(both engines)loop.go, 10 in io_uringworker.go, 1 intransplant_source.go; each is a queue append or slice swap plus an atomic store/swap;Signalis always called afterUnlockWakeFD.mu, read side (Signal)write(2)on a descriptorNew/Setforce toO_NONBLOCKEAGAINat worst)WakeFD.mu, write side (Set,Close)fcntl, atomic stores,close(2)write(2)The writers run only on the loop's own thread:
Setinrun()at start (epollloop.go:391, io_uringworker.go:992) and when epoll creates its eventfd lazily (loop.go:1807inOnDetach,:2415indrainDetachQueue);Closeat shutdown (loop.go:3076,worker.go:5413) and inreleaseFailedInit(worker.go:5455). None of them is reachable from a pause/resume callback, and the loop thread holdspausedMuonly insiderequestPause, whose body calls nothing but the callback, and insideSetPauserwhen a sync-mode upgrade runs on it, which calls nothing at all. So a writer of the RWMutex never waits onpausedMu, holding the write lock or queued for it. A callback'sSignalcan wait behind aClose, but thatClosewaits only for theSignals already inwrite(2).pausedMuitself is an unexported field of an unexported type, acquired only inengineread.go(requestPause,resumeIfDrained,SetPauser), and the engine packages do not importmiddleware/websocket.The forced-interleaving test (
TestChanReaderWakeFDWritersNeverWaitOnPausedMu, Linux, in CI) uses the productionWakeFDand callbacks built like the engines' own, parks a callback while it holdspausedMubetweendetachQMu's release andSignal, and forces the three ways a writer and that callback can meet:Closewhile a pause holdspausedMu:Closemust complete withpausedMustill held, and theSignalafter it is a no-op.Setwhile a resume holdspausedMu(epoll's lazy eventfd):Setmust complete withpausedMustill held, and theSignalafter it lands on the new descriptor.Signal'swrite(2)holding the read lock,Closeis queued for the write lock behind it, and only then does the pause callback, holdingpausedMu, callSignal, whichsync.RWMutexqueues behind the waitingClose. The goroutine dump confirms each state before the next step. Every lock in the chain is then held or awaited at once, and it must drain once the in-flight write completes. To hold a reader inwrite(2)for as long as the test needs, the WakeFD gets a full pipe whoseO_NONBLOCKthe test clears afterNewset it, which is a longer read hold than production can produce.A watchdog turns a deadlock into a failure. Control: a mutant in which both writers wait on
pausedMuwhile holding the write lock (a hook right afterw.mu.Lock()inSetandClose) fails all three cases on the watchdog instead of hanging the binary (round3/05-lockorder-control/).Failing-first evidence
Each test lands one commit before its fix.
go test -race -count=1 -v -run '^TestChanReader', strict tally of--- PASS/FAIL/SKIPlines (subtests included). Round 2's rows ran natively; round 3's rows ran on Linux (Dockerlinux/arm64,golang:1.27, memlock 8 MiB), because the lock-order test is Linux-only (round3/03-failing-first.txt,scripts/failing-first3.sh):7911b9bTestChanReaderStalePauseConverges+ all 4 capacities, each withceleris#672: engine recv paused with nothing buffered …d94aa538bf80f0TestChanReaderCallbackPanicReleasesPausedMu+ all 3 callback sites, each withpausedMu is still held after the callback panicked …a14ebbed3472e79122766TestChanReaderSetPauserOrderedWithAppend: 2 data races,r.pause(engineread.go:252) andr.resume(:283) againstSetPauser(:140/:141)0a3492d651d89569bcd1c,371c76aengineread.goSetPauserOrderedWithAppend(2 races)engineread.goSetPauserOrderedWithAppend(1 race: main'sr.pauseread,:222); and the lock-order test, whose harness precondition fails (pausedMu is free with the … callback parked), since main applies the callbacks outsidepausedMuand the question it asks does not exist thereTestChanReaderStalePauseConvergeslays the #672 interleaving out in one goroutine, in the order the race produces it, so the scheduler decides nothing: the crossing append's channel send and depth snapshot (the two statementsAppendruns beforerequestPause), the handler draining to empty through the realRead, then that append'srequestPause. Only the gap is manufactured; every statement on either side of it is the production path. The oracle is the quiesced state: nothing buffered, so the engine must not be left paused; the reader's view must match the engine's; and the next delivered chunk must reach the nextRead. Anti-vacuity: exactly one pause callback, an empty buffer and zero resumes before the stale pause. It sweeps capacity 1 (lowWater 0), 8, 16 and 256.TestChanReaderCallbackPanicReleasesPausedMupanics once inside each callback site (the pause inrequestPause, the resume inRead, the #672 re-check's resume), recovers as a caller would, requires that it recovered its own panic, and probes the lock withTryLock, so a regression fails instead of hanging.Mutants
Eleven single-site mutants of the head's
engineread.go, each run throughgo test -race -count=1 -v -run '^TestChanReader'(native, like round 2's;round3/04-mutants.txt,scripts/mutants3.py 371c76a). ABSENT counts the head's result lines that a mutant's run does not print:StalePause,CallbackPanicpausedStatetrueStalePause(engine/reader disagreement)pausedStatewithout resumingStalePause,CallbackPanic<instead of<=StalePause/cap1(lowWater 0)UnlockinrequestPauseCallbackPanicUnlockinresumeIfDrainedCallbackPanicLostResumeConverges, then the binary abortsParkedResumeConverges, then the binary abortsSetPauserwrites withoutpausedMu(new)SetPauserOrderedWithAppend(2 data races)requestPause's nil check back above theLock(new)SetPauserOrderedWithAppend(1 data race)M8 and M9 abort the test binary (review point; the previous table did not say so). Each unlocks and relocks
pausedMuaround the callback inside a critical section whose unlock is deferred. WhenTestChanReaderCallbackPanicReleasesPausedMumakes that callback panic, the relock is skipped and the deferredUnlockruns on an unlocked mutex:fatal error: sync: unlock of unlocked mutex, which norecovercan catch (M8 inpause-in-requestPause, M9 inresume-in-Read). Every test after that point never ran: 5 of the 26 result lines the head prints in this native run are absent (the panic test, its three subtests, andSetPauserOrderedWithAppend). On d3472e7 it was 4 of 25. Both kills stand, becauseLostResumeConvergesandParkedResumeConvergesfailed before the abort. But in these two runs nothing after the abort was tested.M5 survives because of celeris#705, which this PR does not fix. No test reaches the re-check with chunks spilled and the channel at or below lowWater. The only interleaving I found that does is #705's stranded spill: a chunk spills after the handler has drained the whole channel, and it is never promoted, because only a successful dequeue promotes spill. The
!hasSpill()guard cannot fix that in either direction. #705 predates this PR: its single-goroutine repro leavesdepth=0 spill=1with the engine paused, on main9f4d89bexactly as on this head. The previous revision said it was "reported separately"; it had not been filed. It is now celeris#705 (v1.6.0), to be fixed on top of this PR with that repro as its failing-first test, which is what will kill M5.CI gate
The websocket step's exact-PASS interlock now covers six tests (the two #667 orderings, the #672 stale pause, the panic release, the SetPauser ordering and the WakeFD lock order) and the ten subtests of the stale pause, the panic release and the lock order. A rename, a skip, or a capacity, callback site or interleaving dropped from a sweep turns the step red. The PASS patterns end at
(, never at the name.TestChanReaderSetPauserOrderedWithAppendskips without-race, and the step runs-race.Proved in the CI shape on
371c76a(round3/06-gate-proof.txt,scripts/prove-gate3.sh): the step body is extracted verbatim fromci.ymland run the way GitHub runsshell: bash(bash --noprofile --norc -eo pipefail), withgolang:1.27,-race, memlock 8 MiB and the step'sWS484_*env. One container per arm:371c76aengineread.go(SetPauser unordered)SetPauserOrderedWithAppend(race detected)t.Skipt.Skip)Round 2's seven arms (main's
engineread.go, the #667 fix alone, arm C without the deferred unlocks, and the rename/skip/sweep arms against the four-test interlock) are in round 2's06-gate-proof.txt.TestMeasureWedgeRateis outside^TestChanReader, so the step never sweeps the harness in; withoutWS667_WEDGEit skips.End-to-end A/B on real sockets (review MAJOR 2)
celeris#667 makes an end-to-end A/B a merge precondition, and the previous revision's own real-socket A/B found the #667 half raising round failures. So the shipped head is now measured end to end. The design was pre-registered before the seed was drawn and before any analysed round ran:
round3/ab/10-PREREGISTRATION.md, sha25653fe5e80…, hashed 00:42:07Z.mkschedule-ab3.pyrefuses to draw the seed unless the file still matches that hash. One round is one container and gives one observation per workload.Arms. Every arm is the head tree
371c76awith onlyengineread.goreplaced:9f4d89b;3b6c500, the earlier A/B's arm;371c76a.The only non-test file main and the head differ in is
engineread.go, so this compares main's production code with the head's under identical tests and rig. The head arm is371c76arather thand3472e7because this revision changesengineread.go(the SetPauser ordering). The point is to measure the head that ships.Shapes. The CI shape is memlock 8 MiB,
--cpus 4,GOMAXPROCS=4, where io_uring gets one worker. The second shape is memlock 128 MiB, where it gets four. Both run Dockerlinux/arm64withgolang:1.27.W1 is the backpressure and echo tests in CI's step shape (
-race, CI'sWS484_*env):TestBackpressurePauseDoesNotCancelInflightSend(96 connections per engine),TestBackpressureInboundSequenceIntegrity(16 per engine variant),TestNativeEngineEchoandTestNativeEngineBackpressure.W2 is the earlier A/B's rig, unchanged:
BenchmarkWSEngineBackpressureEcho,-count=3 -benchtime=200x, both engines.n and order. 24 rounds per arm per shape (48 per arm pooled), 192 rounds in all. They run in 24 counterbalanced blocks of 8, each a seeded permutation of the four arms followed by its mirror, with blocks alternating shapes. Power (one-sided Fisher at α 0.05, 48 v 48) is 0.76 against the earlier A/B's middleware/websocket: chanReader applies pause/resume to the engine outside the lock that decides them, so a resume can be lost and the connection stops delivering (found by reading, not reproduced) #667-only effect (1.7% → 16.7% of rounds).
Decision rule, per workload. Compare this PR with main on rounds failed, pooled over shapes, with a one-sided Fisher test. BLOCK if p < 0.05. VOID if the A/A moves (two-sided p < 0.05).
9f4d89b3b6c500371c76aVerdicts (pre-registered rules): W1: PASS (no increase in round failures that this n resolves); W2: PASS (no increase in round failures that this n resolves).
W1 tests that failed, rounds per arm (pooled over shapes):
9f4d89b: noneTestBackpressureInboundSequenceIntegrity1/48;TestBackpressureInboundSequenceIntegrity/io_uring/multishot_recv1/483b6c500:TestBackpressureInboundSequenceIntegrity1/48;TestBackpressureInboundSequenceIntegrity/io_uring/multishot_recv1/48371c76a: noneW2 failures per arm (pooled): rounds failed by engine, failed reps, and the failed rounds by kind (POST HOC,
32-SENSITIVITY.txt: start =server not ready within timeout, stall =client N: … i/o timeout):9f4d89b: epoll 0/48, io_uring 2/48 rounds; failed reps epoll 0, io_uring 5; failed rounds: start 2, stall 03b6c500: epoll 6/48, io_uring 3/48 rounds; failed reps epoll 6, io_uring 3; failed rounds: start 0, stall 9371c76a: epoll 0/48, io_uring 1/48 rounds; failed reps epoll 0, io_uring 3; failed rounds: start 1, stall 0Per-connection failure classes (W1), every class with a non-zero count: per-round median [min-max], total over connections, two-sided Mann-Whitney U over rounds vs main
9f4d89b3b6c500371c76aClass totals over all rounds, engines and shapes (connections): main
9f4d89b: pause_cancel 9216 conns, clientCloseFail 0; inbound 2304 conns, no failure class non-zero; A/A (main again): pause_cancel 9216 conns, clientCloseFail 3; inbound 2288 conns, no failure class non-zero; #667 only3b6c500: pause_cancel 9216 conns, clientCloseFail 5; inbound 2288 conns, no failure class non-zero; this PR371c76a: pause_cancel 9216 conns, clientCloseFail 20; inbound 2304 conns, no failure class non-zero.Rounds that started with another lane's container running: 177 of 192. First/last round start: ['2026-09-27T01:06:10Z', '2026-09-27T05:04:18Z'].
Rounds per arm and shape, and how many started with another lane's container running: base/m8 24 (23); base/m128 24 (22); aa/m8 24 (23); aa/m128 24 (22); fix/m8 24 (22); fix/m128 24 (21); head/m8 24 (22); head/m128 24 (22).
Unusable rounds (kept, listed, re-run once):
ab3-105-m8-head.log: W2 ran no io_uring (engine excluded?);ab3-128-m8-base.log: W2 ran no io_uring (engine excluded?);ab3-156-m8-fix.log: W2 ran no io_uring (engine excluded?). In all three the io_uring sub-benchmark had failed to start before it could log its workers, so these are start failures that the rule excluded; see the sensitivity view above.Pre-registered verdicts: W1 PASS, W2 PASS. This PR fails no more rounds than main: W1 0/48 vs 0/48, W2 1/48 vs 2/48 (one-sided p = 0.88). The A/A floor is quiet in both (p = 1). A PASS means no increase this n can resolve. At main's observed 2-4%, power reaches 80% only for a head rate of roughly 18-24% (
00-power.txt).The earlier A/B's finding replicates, and this PR removes it. The #667 fix alone fails 9/48 W2 rounds against main's 2/48 (one-sided p = 0.025; the earlier A/B found 20/120 vs 2/120). This PR fails 1/48 (two-sided p = 0.015 against #667 alone).
What the failures are (POST HOC,
32-SENSITIVITY.txt). Read from each failure's own line, every failed round is one of two kinds:client N: … i/o timeout. W2's 60 s read deadline expired on an echo that stopped, which is the wedge. Stalled rounds: main 0/48 and A/A 1/48 (ab3-026-m8-aa.log,client 0: … i/o timeout), so main's code stalled in 1/96; middleware/websocket: chanReader applies pause/resume to the engine outside the lock that decides them, so a resume can be lost and the connection stops delivering (found by reading, not reproduced) #667 alone 9/48; this PR 0/48. These p-values are POST HOC, because the stall class was defined after the rounds ran (ab/32-SENSITIVITY.txt; all of them, the main's-code one included, are inround5/51-STALLS.txt). middleware/websocket: chanReader applies pause/resume to the engine outside the lock that decides them, so a resume can be lost and the connection stops delivering (found by reading, not reproduced) #667 alone vs main gives p = 0.0013, and vs main's code (1/96) p = 0.0002. This PR vs middleware/websocket: chanReader applies pause/resume to the engine outside the lock that decides them, so a resume can be lost and the connection stops delivering (found by reading, not reproduced) #667 alone gives p = 0.0026. The pre-registered contrast is the round-failure one above: middleware/websocket: chanReader applies pause/resume to the engine outside the lock that decides them, so a resume can be lost and the connection stops delivering (found by reading, not reproduced) #667 alone 9/48 vs main 2/48, p = 0.025. All 9 of the middleware/websocket: chanReader applies pause/resume to the engine outside the lock that decides them, so a resume can be lost and the connection stops delivering (found by reading, not reproduced) #667-only arm's W2 failures are stalls, and none of this PR's is. Round 4's throughput rounds add more of the same (see Throughput).server not ready within timeout. This is an io_uring engine start at memlock 8 MiB that logs its worker cap and never reports listening. It happens before any connection is upgraded, so before achanReaderexists.startNativeServerWithHandle, which both W1 and W2 use) never readsStart's error, so the log cannot say why.A deviation the analysis exposed. The usability rule "W2 logged
workers=for both engines" marked 3 rounds unusable (105 this PR, 128 main, 156 #667 alone; all at 8 MiB), and they were re-run once, as pre-registered. In all three the io_uring sub-benchmark had failed to start before it logged its worker count, so the rule excluded real failures. Round 105 also carried a W1 start failure. Counting the three originals instead of their re-runs (POST HOC) gives:No conclusion changes, and the stall counts are the same in both views.
Per-connection failure classes.
seqGaps,parseErr,overflowErr,protocolErrors, RSTs andcloseTimeoutin every arm.closeTimeout,otherWriteErr,protoErr,ecanceledandclientRSTin every arm.clientCloseFail: the client's close handshake failed, which is the counter celeris#623 says is never asserted. Over 9,216 pause_cancel connections per arm: main 0, A/A 3, middleware/websocket: chanReader applies pause/resume to the engine outside the lock that decides them, so a resume can be lost and the connection stops delivering (found by reading, not reproduced) #667 alone 5, this PR 20.ab3-011-m8-head.log, io_uring at 8 MiB, where the test passed). Rounds with anyclientCloseFailon io_uring at 8 MiB: this PR 3/24, A/A 3/24, middleware/websocket: chanReader applies pause/resume to the engine outside the lock that decides them, so a resume can be lost and the connection stops delivering (found by reading, not reproduced) #667 alone 2/24, main 0/24. The Mann-Whitney p against main is 0.077 for this PR and 0.077 for the A/A, so this n cannot tell this PR's count from the floor.Conditions.
round3/MANIFEST.txt).Throughput (review MAJOR 2, round 4): no loss above the floor
Why the gate changed, and when. The pre-registered load gate (
load1 < 1.5) could not open on this host. Over 31 minutes with no container of any lane running,load1read 4.3–8.8, fromdasdand the maintainer's interactive apps, which no lane may touch (round3/ab/timing/33-LOADGATE.txt).As the review allowed, the gate was amended in a hashed addendum BEFORE any timing round:
round4/11-ADDENDUM.md, sha256fac26e72…, hashed 06:17:06Z. The first timing round started at 08:45:23Z.What replaced the gate:
load1and the top CPU processes.Noise is left to the design, as pre-registered: counterbalanced mirrored blocks, and each cell's own A/A floor.
What did not change: the arms and their trees, both shapes, the W2 command (byte for byte), the 192-round seeded order, the metric, the BLOCK rule and the analysis script (
ab3stats.py --timing, unchanged, sha256be9bf7f4…).The declared secondary (the review's suggestion): after W2, each round's container runs W2X. W2X is the same rig with every op timed, and a second pass with the mutex profile on. It uses a copy of the arm's tree plus one test file, byte-identical in every arm and not part of this PR.
The run. 192 rounds, 08:45–10:02Z, no unusable round. At every round's start no other container was running and no slot was held.
One whole-block pause: 09:03:53–09:15:24Z, between blocks 7 and 8. The orchestrator asked for the timing lock for fix(engine): keep serving on a paused listener for 1.5 s with TCP_DEFER_ACCEPT cleared, so a switch or PauseAccept no longer resets clients that had not yet sent a request (celeris#662, #675) #674, and the addendum allowed a pause only at a block boundary.
Load: start
load1was 2.3–5.4 (median 3.65). It was balanced across arms: against main, Mann-Whitney p ≥ 0.37 in every arm and shape.Foreign load, reported after the run. Another lane's native
go test -race -p 2runs overlapped this hold twice, because its host check did not gate on the timing lock (round4/FOREIGN-LOAD-NOTE-r4h2-20260927T0914Z.txt):The note asks for those rounds to be voided and re-taken. The pre-registration has no rule for foreign native load, so the pre-registered analysis keeps them. POST HOC, dropping those whole blocks changes no verdict (
38-FOREIGN-LOAD-SENSITIVITY.txt):Pre-registered verdicts: all four cells PASS (
round4/31-THROUGHPUT.txt). One round is the median of 3 reps, with 24 rounds per arm and cell. A cell BLOCKs if this PR is slower than main by more than that cell's A/A floor at one-sided p < 0.05.The #667-only arm's failed rounds are stalls, which the rank test counts as slowest.
MDE (the smallest slowdown of this PR the one-sided test detects with 80% power, from the base and A/A spread; reported afterwards, not a gate): epoll 8 MiB +3.5%; io_uring 8 MiB +7.5%; epoll 128 MiB +3.0%; io_uring 128 MiB +3.0%.
The borderline cell was replicated. io_uring at 128 MiB (four workers) read +2.29% at p = 0.059. The declared load sensitivity, which drops rounds above the 90th percentile of start load, read +3.37% at p = 0.019 in that cell. The primary is not a BLOCK. The sensitivity view would meet the rule, but it was declared with no decision. And one run discriminates nothing.
So I pre-registered a replication of that cell alone, before its seed:
round4/12-CONFIRM.md, sha2562a77764f…, hashed 10:06:05Z.round5/53-MDE-CONFIRM.txt; no verdict depends on it).ab4-secondary.py's MDE function gives +2.0% at 64 rounds per arm, the figure12-CONFIRM.mdquotes. With 4,000 draws per point, the pre-registered rule reaches 80% power between +1.75% and +2.0%. At round 4's +2.29% its power is 0.93, not the "about 80%" that12-CONFIRM.mdstates. That sentence had no saved computation behind it, and it was wrong. The hashed file is left as it was.round4/35-CONFIRM.txt).So there is no loss above the floor. The one open question is a small cost on io_uring with four workers. Both runs of that cell pooled, 88 rounds per arm, give +1.40% [-0.20, +2.54], one-sided p = 0.024, against an A/A floor of 0.56% (
round4/36-CONFIRM-POOLED.txt). That view was declared secondary, and it is biased upward, because round 4 is what flagged the cell. The unbiased test is the replication alone, and it passes. Without the blocks that overlapped foreign load, the pooled view is +0.60% [-0.41, +2.28] (post hoc). Either way, a cost of up to about 2.5% on io_uring with four workers is not excluded. At 8 MiB (one worker, the CI shape) this PR read -2.26%.Worker stall: the mechanism the review names is real, and small on average (declared secondary;
round4/34-SECONDARY.txt, with the post hoc detail in37-WORKERSTALL-DETAIL.txt). BenchmarkAB4EchoMutex counts only waits that parked, attributed to the lock holder's stack. With the mutex profile on, the engine worker inrequestPauseparks on apausedMuthat the handler holds across the resume callback:The #667-only arm has fewer ops because its stalled reps failed.
The cost comes from holding
pausedMuacross the callbacks, which is what #667 requires. Making those waits rarer would mean changing how the callbacks are ordered, and that is not this PR.Tail: not resolved at this n, and a small tail cost in epoll is possible. These are all of the A3 declared tail metrics (
round4/34-SECONDARY.txt); the post hoc detail is inround5/52-TAIL.txt, fromscripts/tail-detail.py. Order: main / A/A / #667 alone / this PR. Per-round contrasts are this PR vs main (one-sided Mann-Whitney, uncorrected).Each arm pools 600 ops from each of 24 rounds, 14,400 ops (the #667-only arm has 13,200 in the two cells where two of its rounds lost a rep).
Why p99.9 jumps. At 14,400 ops, p99.9 is the 15th slowest op, so it mostly records whether 15 ops passed 10 ms.
So the pooled p99.9 moves by about 10 ms on a difference of a few ops. The counts are the steadier reading of the same data.
epoll at 128 MiB: every tail metric leans against this PR, and one declared contrast reaches p < 0.05.
epoll at 8 MiB is the same epoll setup. Every round of both shapes logs
epoll: workers=4, and memlock limits only io_uring. Here the declared counts lean the same way, but less: 16 vs 13 ops at or above 10 ms (p = 0.36). The 2 ms count shows nothing: 19 vs 18.io_uring shows no lean at 2, 4 or 10 ms in either shape. For example, ops at or above 2 ms are 32 vs 33 at 8 MiB and 13 vs 10 at 128 MiB (p ≥ 0.34).
At 1 ms, about the 80th percentile of an op, the count describes the body of the distribution, not its tail. There the pooled binomial is not usable: in epoll at 8 MiB the A/A arm has 2,411 such ops against main's 2,898, a 17% gap between identical code. Per round, this PR vs main gives p ≥ 0.21 in every cell.
So the tail is not resolved at this n. A small tail cost in epoll is possible, most visible at 128 MiB; nothing here points to one on io_uring. If the cost is real, its likely source is the hold of
pausedMuacross the callbacks that the worker-stall numbers above measure.Stalls, again (descriptive; the stall class is post hoc). A W2 round stalls when a client's 60 s read deadline expires, which is the #672 wedge. Every count and p-value here is in
round5/51-STALLS.txt, written byscripts/stall-counts.pyfrom the raw round logs. It reproduces round 3's32-SENSITIVITY.jsoncounts exactly.Measurement: celeris#672 wedge rate on the rebased head (round 2)
Measured on d3472e7's
engineread.go.371c76a's differs from it only in the SetPauser ordering, and the harness callsSetPauseronce, before it starts its goroutines, so these numbers stand for the head. They are a unit-level harness; the real-socket measurement is the section above.The same harness and round shape as the previous revision, now committed. The escape hatch is removed, so one wedge ends one connection. A wedge is detected structurally: the engine is paused, the buffer is empty, and the reader makes no progress across 512
runtime.Gosched()yields. No wall clock is involved. Each round is 16 connections, one container and one process, and counts as ONE observation. Setup:golang:1.27,--cpus 4,GOMAXPROCS=4, memlock 8 MiB, no-race, Dockerlinux/arm64.Pre-registered before the seed was drawn and before any round ran:
engineread.goinside the head tree:baseis main9f4d89b.aais the same bytes as base in its own slots, which gives the A/A floor the previous wedge matrix did not have.fixis the middleware/websocket: chanReader applies pause/resume to the engine outside the lock that decides them, so a resume can be lost and the connection stops delivering (found by reading, not reproduced) #667 fix alone (3b6c500).armcis this head.Across 80 rounds this PR wedged 0 of 1,280 connections, over 32.1 million watermark edges. By the pre-registered verdicts:
Controls:
edges>0 && pauses>0or the round fails, and all 320 rounds were usable: GO_RC 0, one PASS line, 16 connection lines.Two things I have to disclose.
BenchmarkChanReaderContended, whose escape hatch runs every arm for the same op count, at n=10 per arm. There arm C is -6.64% vs base, but the benchmark's own A/A moved -4.78% at that n. cap16 is confounded by base degeneration (8/10 rounds), as fixed in advance. This changes no verdict: the per-edge metric already normalises for edge rate, and a 7% edge-rate difference cannot turn 560/640 wedged connections into 0/640.Whole suites, main vs this branch
go test -race -count=1 -v, full packages, one container per (arm, suite),golang:1.27,--cpus 4, seccomp unconfined, base and branch alternating which runs first. Strict tally: only--- PASS/FAIL/SKIP:lines, subtests included; a SKIP is counted as absent, never as a PASS.9f4d89b: rc, PASS / FAIL / SKIPd3472e7: rc, PASS / FAIL / SKIP./middleware/websocket, memlock 8 MiB (CI'sWS484_*env)TestMeasureWedgeRateabsent→SKIP./middleware/websocket, memlock 128 MiB (CI'sWS484_*env)TestMeasureWedgeRateabsent→SKIP./engine/epoll(its tests importmiddleware/websocket)./engine/iouring, memlock 8 MiB (one worker)TestListenRefusesToStartWhenNoWorkerReportsItsAddressPASS→SKIP./engine/iouring, memlock 128 MiB./engine/iouring, memlock 8 MiB, second run (branch first)PASS→FAIL: 0. PASS→SKIP/absent: 1.
TestHubBroadcastFormatsOnce. It skips on the branch too. The branch's extra SKIP is the wedge harness, which skips withoutWS667_WEDGE.TestListenRefusesToStartWhenNoWorkerReportsItsAddressin./engine/iouringat 8 MiB. It skips when io_uring is unavailable, and on the branch runio_uring_setupreturned ENOMEM (cannot allocate memory … current=8388608 bytes). The locked memory was momentarily exhausted in that process, and the log shows it. The cause is environmental, not this PR:./engine/iouringhas no diff between the two arms and does not depend onmiddleware/websocket(go list -deps -testfinds 0 matches), so both arms run the same test code. It passed on the branch at 128 MiB. A second full run of both arms at 8 MiB (branch first,iouring-m8-rerun) has it SKIP on the branch and SKIP on main (strict tally). Main skips it too, with the sameio_uring_setupENOMEM, so the skip is not the branch's. Across the four 8 MiB runs it passed 1 time and skipped 3 times, each skip on ENOMEM. At 8 MiB on this host it skipped more often than it ran, whichever arm ran it, and a skip there is a hole in coverage, not a verdict.Round 3, RULE 21 before the push: the whole
./middleware/websocketsuite on371c76a, the same shape as the first row above (-race -count=1 -v, memlock 8 MiB, CI'sWS484_*env, one container): rc 0, 230 PASS / 0 FAIL / 2 SKIP in 885 s (round3/07-suite/). Against round 2's runs of main and of d3472e7 (round3/07-suite-compare.txt): PASS→FAIL 0, PASS→not-PASS 0. The only new result lines are the five new tests and subtests, all PASS. The two SKIPs are the same as before:TestHubBroadcastFormatsOnce(skips on main too) and the wedge harness (skips withoutWS667_WEDGE).celeris#633's coverage reproducer, main vs this branch (round 2)
Pre-registered before any round ran (
60-cover633/10-PREREGISTRATION.md). The input is celeris#633's cheap reproducer: the CI stepgo test -race -covermode=atomic -run '^TestBackpressure' ./middleware/websocket/..., with CI'sWS484_*env at memlock 8 MiB. On a GitHub runner it failedTestBackpressurePauseDoesNotCancelInflightSendon both engines (4 close-timeouts each). One round is one container and onego testprocess. There are 10 rounds per arm, in blocks of four: a seeded permutation of the two arms, then its reverse. Every round ran on Dockerlinux/arm64with--cpus 4. Pre-registered deviations from the CI command:-v,-timeout=900s, and arm64.9f4d89bd3472e7TestBackpressurePauseDoesNotCancelInflightSendfailed (primary)/epollsubtest failed/io_uringsubtest failedcloseTimeoutper round, median [min–max] of 96 connsotherWriteErrper round, median [min–max] of 96 connsclientCloseFailper round, median [min–max] of 96 connsclosedOKper round, median [min–max] of 96 connscloseTimeoutper round, median [min–max] of 96 connsotherWriteErrper round, median [min–max] of 96 connsclientCloseFailper round, median [min–max] of 96 connsclosedOKper round, median [min–max] of 96 connsTestBackpressureInboundSequenceIntegritypassedPrimary: this branch 10/10 vs main 10/10 failed rounds, two-sided Fisher p = 1: no move this n can resolve.
The reproducer fails on main in 10 of 10 rounds on this host, so the failure predates this PR. This PR does not fix it. With main failing every round, the design would have called a move only if this branch had failed 5/10 rounds or fewer (
00-power.txt). A null result at this n does not show the rate is unchanged.TestBackpressureInboundSequenceIntegrityfailed once, on this branch (cover633-07-head.log), inio_uring/multishot_recv, withinbound_sequence_linux_test.go:132: server not ready within timeout. That is the test helper's 30 s wait for the server to report a listening address. The io_uring server in that sub-run logged its memlock worker cap and never logged that it was listening, so no connection was upgraded and no frame was sent. It is not an integrity verdict, and it did not run the code this PR changes (chanReaderexists only after an upgrade). Main 0/10 vs this branch 1/10, Fisher p=1. The helper discardsStartWithListenerAndContext's error, so the log cannot say whether the start failed or hung (now celeris#706; the round-3 A/B saw the same start failure in every arm at 8 MiB).The 5 engine subtests that PASSED (both arms together) had
clientCloseFailfrom 2 to 85 of 96 connections (median 23). That is the counter celeris#623 reports as printed but never asserted.Secondary, per engine, two-sided Mann-Whitney U over rounds, this branch vs main: epoll
closeTimeoutp=0.43; epollclientCloseFailp=0.73; epollotherWriteErrp=0.76; io_uringcloseTimeoutp=0.91; io_uringclientCloseFailp=0.33; io_uringotherWriteErrp=0.19. None is below 0.05.Deviations and conditions (read from the round logs):
void/cover633-14-base.attempt1.log, started 2026-09-26T17:49:21Z) stops after the first test's listener line. It has no result line and noROUND_RC, and the driver logged no end for it. The host's once-a-minute process sampler (_locks/proc-sampler.log) read 1038 user processes at 17:39:06Z and then logged nothing until 18:39:23Z. That is consistent withforkfailing, as it had twice earlier that afternoon (the lock tool'sfork: Resource temporarily unavailablelines indriver.txt). Docker's VM clock in that log reads 7 minutes behind the host. The attempt is void as apparatus, not an observation. It was re-run with the same tree, command and slot discipline, and only the re-run is analysed. Counting the void attempt as one more failed main round instead changes nothing (p=1; not pre-registered).go testpackage run. The delay came after its last result line, and its results are complete, so it counts as pre-registered.-v,-timeout=900sinstead of 120 s, andlinux/arm64instead of CI's amd64.Not in this PR
inbound_sequence_linux_test.goandpause_cancel_linux_test.go, which this PR does not touch, so they are left to their own PRs. What each needs, on this tree:pause_cancel_linux_test.gocountsclientCloseFail(:207,:225) and prints it (:255) but never asserts it. The assertion block at:257-305checkscloseTimeout,clientRST,protoErr,otherWriteErrandecanceledonly. The sibling inbound oracle has the same hole (inbound_sequence_linux_test.go:324/:333/:351counted,:407printed, never asserted). The fix is onet.Errorfper oracle, plus a control showing the new assertion fires on a run where the counter is non-zero and the test currently PASSES. The previous revision suggested a build that failed oncloseTimeoutwithclientCloseFail=0, which cannot show that (review point). Control, from runs already on record: in the epoll: a rare close-handshake stall with the receive queue already drained, 1 in 73 (split from #607) #633 reproducer on main9f4d89b(below),TestBackpressurePauseDoesNotCancelInflightSend/io_uringPASSED withclientCloseFail=74of 96 in round 14 (cover633-14-base.log,closeTimeout=0,otherWriteErr=0) and with 23 of 96 in round 5 (cover633-05-base.log). Three branch rounds passed with 2, 4 and 85. So the The WebSocket backpressure oracle counts clientCloseFail, prints it, and never asserts it: 66 of 96 connections failed and the test passed #623 fix's control is that reproducer on main, and rounds like 5 and 14 must turn FAIL.inbound_sequence_linux_test.go:210-222), abandoning chunks already queued in itschanReader, which the frame-count check (:466-470, gated only onclientFailed) then scores as lost. It needs the handler to keep reading after a write failure and count the drained remainder separately, and an assertion onechoWriteErrors(printed at:416, never asserted). The control is a build with a real inbound loss, showing the corrected count still flags it.linux/arm64on one laptop. amd64 is covered by compilation and by CI (x86-64 GitHub runners).Reproducing
middleware/websocket/engineread_wedge_linux_test.go. Its header gives the command for one observation and builds the base and middleware/websocket: chanReader applies pause/resume to the engine outside the lock that decides them, so a resume can be lost and the connection stops delivering (found by reading, not reproduced) #667-only arms withgit show 9f4d89b:…andgit show 3b6c500:…. Before this revision the header described the middleware/websocket: chanReader applies pause/resume to the engine outside the lock that decides them, so a resume can be lost and the connection stops delivering (found by reading, not reproduced) #667-only arm as "this tree's engineread.go with the re-check deleted", which is not the arm the A/B measured (review point).lane-20260926/round3/. ItsMANIFEST.txtmaps every output to the script that produced it:ab3-round.sh, which starts the container throughdock.sh(rounds 1–27),dock2.sh(28–29) and, under a held slot,dock-held.sh(30 on);run-ab3-matrix.shran rounds 1–29,run-ab3-matrix2.sh30–32,run-ab3-matrix3.sh33–72, andrun-ab3-matrix4.sh73–192 plus the three re-runs. Each restart is logged inab/driver.txt;mkschedule-ab3.pydraws the seed and the schedules, and refuses to run unless the pre-registration still matches its hash.power-ab3.pycomputes the power;run-ab3-timing.sh, thenrun-ab3-timing2.sh, only waited on the load gate. No timing round ran under either. Round 4's matrix ran underrun-ab4-timing.sh(below);ab3stats.pymakes the pre-registered round-failure verdicts and contrasts and the per-connection classes. With--timingit makes round 4's throughput verdicts.ab3-section.pyturns its output into the A/B tables;ab3-sensitivity.py(POST HOC) makes the start and stall kinds, round 3's stall p-values and the usability-rule sensitivity (ab/32-SENSITIVITY.txt);loadgate-summary.pymakes the load-gate figures (ab/timing/33-LOADGATE.txt);failing-first3.sh,mutants3.py,lockorder-control.sh,prove-gate3.sh,run-suite3.sh+suite-compare.py,fetch-ci3.sh, andassemble3.py(the body).lane-20260926/round4/(MANIFEST.txt):11-ADDENDUM.md+.sha256, and12-CONFIRM.md+.sha256(hashed before the seed);run-ab4-timing.sh→ab4-round.sh(withab4-warmup.sh);mkschedule-confirm.py→run-ab4-confirm.sh→ab4c-round.sh;ab3stats.py --timingfor both primaries,ab4-secondary.py(tail, worker stall, load, MDE),confirm-pooled.pyandtp4-tables.py;workerstall-detail.py(the worker-stall table's events and mean waits,37-WORKERSTALL-DETAIL.txt) andforeign-sensitivity.py(38-FOREIGN-LOAD-SENSITIVITY.txt);assemble4.py;apparatus/zz_ab4_secondary_linux_test.go.lane-20260926/round5/(MANIFEST.txt). They add no measurement and re-read round 3's and round 4's logs:stall-counts.py→51-STALLS.txt, every stall count and its Fisher test;tail-detail.py→52-TAIL.txt, the tail in full;mde-confirm.py→53-MDE-CONFIRM.txt, the replication's power;assemble5.py, the body.run-wedge4-all.sh→run-wedge4.sh,mkschedule-wedge4.py,power.py,report-wedge4.py→wedgestats4.py,failing-first.sh,mutants.py,prove-gate.sh+extract-step.py,run-suites.sh+tally.py,mkschedule-cover633.py→run-cover633.sh→stats-cover633.py(withcontrol-cover633.sh),fetch-ci.sh.Changes
middleware/websocket/engineread.go: both callbacks are applied underpausedMu(middleware/websocket: chanReader applies pause/resume to the engine outside the lock that decides them, so a resume can be lost and the connection stops delivering (found by reading, not reproduced) #667).requestPausere-checks the watermark after applying a pause and lifts a stale one (middleware/websocket: a pause decided on a stale depth snapshot wedges the connection permanently when the handler drains to empty first (distinct from #667) #672). Both critical sections unlock bydefer;Read's resume branch moves intoresumeIfDrained, so the deferred unlock ends with the decision and does not extend the hold overRead's copy.SetPauserwrites the callbacks underpausedMu, andrequestPausereadsr.pauseunder it (round 3). The lock-order note is re-derived for celeris#666's RWMutex (round 3). The comments name the engine closures, not their lines.middleware/websocket/engineread_test.go: the two middleware/websocket: chanReader applies pause/resume to the engine outside the lock that decides them, so a resume can be lost and the connection stops delivering (found by reading, not reproduced) #667 tests,TestChanReaderStalePauseConverges(middleware/websocket: a pause decided on a stale depth snapshot wedges the connection permanently when the handler drains to empty first (distinct from #667) #672),TestChanReaderCallbackPanicReleasesPausedMu, andTestChanReaderSetPauserOrderedWithAppend(round 3).middleware/websocket/engineread_lockorder_linux_test.go(round 3):TestChanReaderWakeFDWritersNeverWaitOnPausedMu.middleware/websocket/engineread_wedge_linux_test.go: the wedge harness, code unchanged from the one that produced the numbers; its header's middleware/websocket: chanReader applies pause/resume to the engine outside the lock that decides them, so a resume can be lost and the connection stops delivering (found by reading, not reproduced) #667-only recipe is now the measured arm (round 3).middleware/websocket/engineread_bench_linux_test.go: the middleware/websocket: chanReader applies pause/resume to the engine outside the lock that decides them, so a resume can be lost and the connection stops delivering (found by reading, not reproduced) #667 A/B benchmarks, includingBenchmarkWSEngineBackpressureEcho, the W2 rig (unchanged except comments)..github/workflows/ci.yml: the websocket step runs^(TestBackpressure|TestChanReader)with-v, and the interlock requires six tests and ten subtests (round 3: six and ten, up from four and seven).Test Plan
golang:1.27,linux/arm64, memlock 8 MiB and 128 MiB; end to end on real sockets with both engines (the A/B above)./middleware/websocketsuite on371c76abefore the push (RULE 21): 230 PASS / 0 FAIL / 2 SKIP371c76a. The pre-registered 192-round matrix PASSES in all four cells. The pre-registered 192-round replication of io_uring at 128 MiB PASSES.371c76a(run 36284528042, attempt 1): all 9 jobs green, plus CodeQL. The websocket step's own log readsceleris#667/#672 regression tests: want 6, passed 6; subtests want 10, passed 10. Per-test result lines exist only forgo test -vruns, so the strict tally covers those steps only: Unit's websocket and named-test steps 138 PASS / 0 FAIL / 0 SKIP, Conformance 119/0/0, Adaptive 103/0/0, io_uring init-failure 3/0/0. The Unit job's root race step and its four middleware sub-module steps, and the four Driver Conformance steps, run without-vand print no per-test lines, so a SKIP in them cannot be counted from the log. Round 2's "0 SKIP" had the same limit and did not say so (review point).Tested on: [ ] std [x] epoll [x] io_uring — [x] amd64 (compile + CI) [x] arm64
Release notes
breaking)bug)Summary by CodeRabbit