test(iouring): judge the SEND_ZC first-completion window under -race, with its detachMu mutant (celeris#587) - #693
Conversation
…with the detachMu mutant as its detector control (celeris#587) On loopback the kernel copies, so a SEND_ZC's notification lands in the same completion batch as its first completion and the window in which the detached inline-egress guard must refuse the raw unix.Write fast path is a few hundred nanoseconds wide (celeris#601: IouringInlineGuardBlockedZC == 0 on an unmodified build). No test ever put a dispatch-goroutine write inside it, and nothing showed the race detector watches that path. validation.SetZCNotifDelay (-tags=validation only; the production stub is an empty function that inlines to nothing) holds the window open on the worker thread, lock-free, at the top of handleSend's notification branch. TestSendZCWindowGuardUnderRace drives a full-duplex 64 KiB echo against a clamped client receive window and requires: every echo byte-intact and in order, SEND_ZC armed on the detached conn, notifications == submits, and the guard declining with a notification outstanding at least once. The ci.yml zc-window job runs it three ways: as committed (PASS, race-clean), with CELERIS_IOURING_SEND_ZC=off (PASS, every ZC witness 0), and against the mutant that deletes the detachMu acquire in the CQE_F_MORE branch (.github/scripts/mutant-587-unlock-zc-first-cqe.py), which must FAIL with a DATA RACE report.
…llcheck SC2015), and a longer race excerpt
…on, so the detachMu mutant is caught every run (celeris#587) CI run 2 of the first version let the mutant survive: 3 ZC submits, 40 guard declines, no DATA RACE. The hold sat at the top of the NOTIF branch, so whenever the notification landed in a later worker iteration the worker's per-iteration flush had already taken and released cs.detachMu between the first completion and the dispatch goroutine's read -- which orders the two for the race detector with or without the lock under test (the #587 review's point 2). The hold now runs immediately after handleSend returns for a CQE_F_MORE completion, i.e. after the first completion's writes and its deferred unlock and before the worker releases anything else. It sits at the two handleSend call sites behind the new validation.Enabled constant, false in production, so both call sites compile away and handleSend itself is unchanged from main. The CI job now runs every arm three times as separate processes (the detector reports a given race once per process) and requires 3/3 PASS race-clean, 3/3 PASS with ZC off, 3/3 FAIL with a DATA RACE for the mutant.
…m, not an echo (celeris#587) CI run 3 (897b83a) showed two nondeterminisms in the echo shape: a run whose 16 MiB all went out on the inline fast path (RingBytes 0: the 300 us reader drained the socket), and a mutant run with no DATA RACE (3 ZC sends, 46 guard declines) because the hold also stops the worker delivering inbound frames, so an echo handler sat in ReadMessage while the window was open. The handler now streams 256 x 64 KiB frames at one per 500 us after a single 'go', and the client reads one per ms: the socket stays congested (ring sends carry the stream) and the handler calls guarded() through every hold, independent of the worker's recv processing. Backlog peaks near 8 MiB, an eighth of the detached send cap.
|
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: Repository: goceleris/celeris/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (7)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughAdds a validation-only hold between SEND_ZC completion and notification. A Linux WebSocket test checks frame integrity and SEND_ZC witness counters. CI runs the test with the committed implementation, with SEND_ZC disabled, and with a mutant that removes the detachMu acquire. ChangesSEND_ZC race-window validation
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Other Merge Risk: ⚪ Minimal · up to No merge-blocking issue is identified in this change. Complete the stated rebase, Docker suites, and CI run before taking it out of draft. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The new delay is restricted to validation builds, and ordinary builds retain the existing send synchronization. The proposed CI check is designed to verify that the race detector observes this boundary, but successful runs and required-check status have not been established. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation The PR covers the first-completion window and the Resolution Implement the remaining [
Comment |
…m, so the #587 window is entered on every host (celeris#587) A 1 ms reader against a 500 us writer does not congest a host whose 500 us sleep takes a millisecond or whose autotuned send buffer is large: the linuxkit arm64 container carried all 16 MiB inline in 10/10 runs (RingBytes 0), and 2 of 3 GitHub arm64 runs were under-exposed. The reader now paces 4 ms per frame (at most a quarter of the write rate, backlog under ~12 MiB), and a stream that did not reach the ring or did not show the guard declining is run again on a fresh connection, up to three attempts, each logged; the race detector watches all of them.
…der a slow reader; the hold makes it host-independent (celeris#587) The linuxkit arm64 container leg at ee19315 (10 runs per arm) measured the natural arm (hold 0): the guard declined 238-290 times per stream and the detachMu mutant FAILED with a DATA RACE 10/10, the write reported after the guard's read. The comments claimed the worker's per-iteration flush would always order the two accesses in the natural window; that is not what was measured, so they now say what was: the paced stream opens the window on its own on the hosts measured, and the hold is what makes the detector control independent of the host's timing. Comment-only.
…c validation package into internal/zcwindow (celeris#587) validation.Enabled, SetZCWindowHold and ZCWindowHold were exported no-ops in the published validation package, API for one measurement. They now live in internal/zcwindow (hold under -tags=validation, a false Enabled constant otherwise), so validation's public surface is the same as on main. The test also stops sleeping a fixed 300 ms + 4 x hold before reading the counters: it polls, with a 10 s bound, until every streamed byte is accounted to the inline or ring counter and every SEND_ZC submit has its notification.
…require the mutant's kill in every one to be a race report (celeris#587) The tally counted "WARNING: DATA RACE" once over an arm's concatenated log, so a mutant arm with one racing process and two failing for another reason would have counted as killed. Each process now writes its own log; a fixed/ZC-off process must PASS with no race report, and a mutant process must FAIL with its own DATA RACE report and "race detected during execution of test" line. The job comment describes the paced server push the test drives (not an echo) and no longer calls the job a standing gate: it is not in the main ruleset's required checks.
|
Review round 2 (head
Laptop container control at |
|
Review round 3 (head
Lane round-3 evidence: |
|
Review round 4 (fix round 3). No open finding on this PR, and the head is unchanged: celeris main moved. The freeze ended when #674 merged at 12:20Z, and #671 and #699 followed. Main is now |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Merging this PR will improve performance by 37.5%
Performance Changes
Tip Curious why performance improved? Comment Comparing 
Footnotes
|
Closes #587 (on merge). Draft: celeris main is frozen until #674 merges. After #674 this PR needs a rebase onto the new main, a green
zc-windowrun on both arches (the mutant script exits 2 if its anchor drifted, so a drift cannot pass silently), un-drafting and an approving review.What this does
#587 asks for the io_uring SEND_ZC send state to be judged by the race detector on the interleaving the v1.5.7 audit was about: the SEND_ZC first completion and notification (worker thread,
handleSend) against the detached inline-egress guard (dispatch goroutine, theguardedclosure). Two things kept that unmeasured:IouringInlineGuardBlockedZCat 0.-racewatches this path. A clean-racerun is evidence only if the same run reports a race once the lock is removed.Changes:
internal/zcwindow(SetHold/Hold/Enabled; round 2 moved them here from the publicvalidationpackage, whose exported surface is now identical to main). The hold runs right afterhandleSendreturns for aCQE_F_MOREcompletion: the first completion's writes are done, the deferred unlock has run, and nothing else has been released. It is compiled in only under-tags=validation; in productionzcwindow.Enabledis a false constant and both call sites compile away.handleSenditself is unchanged from main.TestSendZCWindowGuardUnderRace(linux && validation). The handler pushes 256 × 64 KiB frames at one per 500 µs, and the client reads one per 4 ms through an 8 KiB receive window, so the handler callsguarded()through every hold. It requires every frame intact and in order, SEND_ZC armed on the detached conn, notifications equal to submits, and the guard declining at least once while a notification was outstanding. An unexposed stream is re-run on a fresh connection, up to 3 attempts, each logged. Round 2: instead of sleeping a fixed 300 ms + 4 × hold before reading the counters, it polls (10 s bound) until every streamed byte is accounted to the inline or ring counter and every submit has its notification.Mutant,
.github/scripts/mutant-587-unlock-zc-first-cqe.py: deletes thedetachMuacquire in theCQE_F_MOREbranch, exits 2 on any source drift.CI job
zc-window(x86 and arm64). Each arm runs 3× as separate processes, each with its own log, each judged on its own (round 2; the job tallied one concatenated log per arm, so one racing process plus two failing for another reason would have counted as a kill):CELERIS_IOURING_SEND_ZC=offWARNING: DATA RACEandrace detected during execution of testSKIP is forbidden. The job is a check on every PR; it gates merges only if the main ruleset lists it (it does not today: Lint, Unit, Conformance, Driver Conformance, Build ×2, Vulnerability Check).
Evidence
Base dir:
evidence/measurements-585-587-588/lane-20260926/on the lane host.Round 2 head
9154522:CI run 36295432420, every job green (Lint, Unit, Conformance, Driver Conformance, Build ×2, Vulnerability Check, Adaptive, io_uring init-failure, and
zc-windowon both arches). Thezc-windowjob's own per-process lines (round2/587/ci-github/):ubuntu-latest, job 108553283717guard_blocked_zc364-412race detectedubuntu-24.04-arm, job 108553283824guard_blocked_zc356-358race detectedEvery stream was exposed on attempt 1 (256/256 frames, about 13.6 MB of 16.8 MB on the ring).
Laptop container (
round2/587/run-587-r2.sh, linux/arm64 linuxkit,--cpus 4, memlock 128 MiB, 10 runs per arm, one process and one log per run;TALLY.txtfromtally-587.sh):9154522(round2/587/20260927T034933Z-9154522)guard_blocked_zc342-360guard_blocked_zc346-410Judged by the CI job's new per-process rule (
tally-per-process-587.sh→PER-PROCESS.txt): every one of the 60 processes meets its arm's rule; the 20 mutant processes with the ZC arm reachable each FAIL with their ownWARNING: DATA RACEandrace detected during execution of test.Hot path (
round2/587/hotpath/hotpath-diff-587.sh 9154522, untaggedengine/iouringtest binary, main9f4d89bvs9154522):(*Worker).handleSend,runandprocessCQEhave the same instruction count and an identical mnemonic sequence on both arches (361/1749/237 on amd64, 420/1808/216 on arm64). On amd64 the operands are identical too. On arm64 the only differing instructions are address materialisations -- 1 ADRP + 1 ADD inhandleSend, 28 ADRP + 31 ADD inrun, none inprocessCQE(operand-classes.sh→operand-classes.txt): globals moved by the link layout, as at897b83a. The production engine executes the same instructions as main.Whole-package suites (
round2/suites/suites-celeris-r2.sh 9154522, one linux/arm64 container,-race -v, production and-tags=validation, over./engine/iouring/ ./internal/zcwindow/ ./validation/... ./middleware/websocket/; the base is round 1's9f4d89blogs, same shape):9f4d89b(round-1 logs)9154522-tags=validation0 regressions (
round2/suites/celeris-20260927T034934Z/TALLY.txt). The base-only FAIL isTestDriverHTTPZeroOverhead, the io_uring: UnregisterConn then Close leaks the driver socket — the fd-keyed ASYNC_CANCEL misses once the caller has closed the fd, so onClose never fires and the peer never sees EOF #691 area (fixed by fix(iouring): run every driver op through the engine's own duplicate of the socket, and count every cancel until its CQE, so closing after UnregisterConn is safe (celeris#691, celeris#707) #696). The SKIP isTestHubBroadcastFormatsOnceon both trees (alloc counts under-race).Cross-compile:
go vetfor linux/amd64 and linux/arm64, with and without-tags=validation, over the touched packages: clean.golangci-lint(both tag sets): 0 issues.actionlint: clean.Round 1 (head
20b1aa5, code-identical in production; kept): CI run 36248124203 green,zc-windowx86 3/3 PASS 0 races, ZC-off 3/3 PASS witnesses 0, mutant 3/3 FAIL with DATA RACE (6 reports); arm64 the same (4 reports). Laptop arms A–F atee1931510/10 each (587/20260926T134652Z-ee19315/TALLY.txt). The mutant's race report is the audit's feared pairing: the now-unlockedCQE_F_MOREwrite inhandleSend(worker.go:3213) against the guard's read ofcs.zcNotifPending(worker.go:2314) throughengineWriter.Writefrom(*Conn).WriteMessage.The design changes, each driven by a failing run (
587/ci-github/): hold at the NOTIF branch plus an echo let the mutant survive one CI run; the echo shape left RingBytes 0 (the hold also stalls inbound delivery); a 1 ms reader carried all 16 MiB inline on the laptop VM.