v1.5.8: WS-upgrade crash + Context/stream use-after-recycle + H2/overload perf (#406/#407/#408) by FumingPower3925 · Pull Request #411 · goceleris/celeris

FumingPower3925 · 2026-07-11T03:55:46Z

Summary

The v1.5.7 24h weekend soak (probatorium run 28850347700) failed on a real,
reproducible WebSocket-upgrade crash on both amd64 and arm64 — not
infrastructure. Root-causing it surfaced three distinct, pre-existing
(v1.5.6-era) concurrency bugs in the engine-integrated WS/SSE upgrade path.
This PR fixes all three (plus a small perf reclaim), verified to the hilt.

All three bugs share a trigger: a peer RST mid-upgrade on an async-promoted
connection racing the engine's teardown.

The bugs & fixes

chanReader send on closed channel panic — closeWith closed r.ch
to wake a blocked Read, but Append (engine event-loop thread) sends on
r.ch; a peer-RST-triggered closeWith racing an in-flight Append
panicked → killed the engine loop goroutine → refapp exit code 2 →
I-LIVENESS. This is the exact panic the soak tripped.
Fix: never close r.ch; add a done channel closed once by closeWith.
Append/Read select on it, so the send target is always open. Engine-agnostic.
epoll loop reads cs.writeFn racing async Detach — drainRead read
cs.writeFn at the top of the per-read block (dead in the async-dispatch
branch) while the dispatch goroutine rewrote it in OnDetach.
Fix: read cs.writeFn only on the inline path, where it's actually used.
Context/stream use-after-recycle on RST-mid-upgrade (epoll AND io_uring) —
closeConn→CloseH1 released the per-conn cached Context + stream back to
their pools while the async upgrade goroutine still ran tryEngineUpgrade on
that Context; WSRawWriteFn then nil-deref'd the recycled c.stream.
Root: OnDetach published h1State.Detached after releasing detachMu, so
a closeConn acquiring detachMu in that window saw Detached==false and
recycled a live Context.
Fix: publish Detached before releasing detachMu in OnDetach, and
have closeConn + the shutdown teardown re-read Detached under
detachMu before CloseH1. Applied to both engines (io_uring was
byte-for-byte the epoll pre-fix code).
perf: drop chanReader Append's dead <-r.done case — the done-channel
fix (Project scaffolding and CI pipeline #1) added per-message select overhead; a focused bench showed ws-echo
−1.5 to −2.4% on native engines. Since r.ch is never closed, Append's
<-r.done case was dead weight — dropping it reverts Append to the fast
single-op select, recovering most of the cost with no correctness change.

Verification

Regression test TestWSRSTMidUpgradeTorture (added) drives concurrent
SO_LINGER=0 RSTs mid-upgrade over real TCP on both native engines. On the
pre-fix code it reproduces the exact send on closed channel panic + a
175-block pooled-object -race cascade; on the fix it's clean.
Both-engine RST torture, -race -gcflags=all=-d=checkptr: 0 races, 0
panics over ~270–312K RST-mid-upgrade cycles per engine (re-run after the
perf change).
go test -race green across middleware/websocket, engine/epoll,
engine/iouring, internal/conn.
16-agent adversarial review — surfaced (and this PR fixes) the io_uring
parity gap; found no deadlock/regression otherwise.
Cluster soaks (probatorium, refapps repinned to this branch): nightly (0
incidents) + full 24h weekend matrix — 0 HIGH-severity I-*, 0
correctness failures across 48 cells; the v1.5.7 crash cell
auth_session_ratelimit/epoll ran 276/276 on both arches.
Perf (focused N=2 saturation bench vs v1.5.7): churn-close (the
detach/close hammer) flat; SSE / HTTP baselines / drivers flat; ws-echo
recovered by the perf change (see below).

Perf (N=2 focused saturation bench vs v1.5.7)

The done-channel fix (#1) cost ~2% on ws-echo for the native engines; the
Append-only perf change (#4) recovers it — ws-echo back to flat:

`ws-echo` saturation rps	done-channel fix	v1.5.8 (this PR)
io_uring	−2.2%	−0.8%
adaptive	−2.4%	+0.1%
epoll	−1.5%	+0.3%

Every other affected path is flat-to-slightly-faster vs v1.5.7:
churn-close (the detach/close hammer) +0.6…+2.2%; ws-hub-broadcast,
ws-large-echo, SSE fan-out, HTTP baselines (get-simple/get-json/post-4k),
and the memcached driver all within ±1% (mostly positive). No regression on any
path. (A read-path fast-path was tried and dropped — it helped ws-echo but
slightly hurt ws-hub-broadcast; the Append-only change is a strict win.)

Out of scope (follow-ups)

The driver_postgres/driver_redis T3-DRIVE readiness flakes the soaks hit
are environmental (host-local to one bench box, arch-independent, 0
correctness failures) — a probatorium harness issue (DB-service stability +
readiness backoff), not celeris.
A remaining pooled-Context recycle micro-race cluster that only a dedicated
-race torture hits (not this crash) remains a separate follow-up.

Also in this PR — H2/overload perf fixes (merged from #412)

Three engine perf/correctness issues surfaced by review, fixed for v1.5.8. Each
comes with a regression test that fails on the old code and passes on the fix.
Independent of the WS-crash work (#411) — separate branch off main.

Closes #406, #407, #408.

#406 — h2 write queue used only 2 of its 4 shards

h2ShardedQueue.Enqueue picked a shard with streamID % h2QueueShards (=4).
Client HTTP/2 request streams are always odd (RFC 9113 §5.1.1) and Celeris does
no server push, so odd % 4 ∈ {1,3} — shards 0 and 2 were permanently dead and
the "4-shard" queue behaved like 2.

Fix: (streamID>>1) % h2QueueShards — shift off the constant low bit so odd
IDs 1,3,5,7,… spread across all shards. Still a pure function of streamID, so
per-stream HEADERS-before-DATA FIFO + single-writer drain are unchanged; only
the distribution improves. Regression test asserts even spread + stream affinity
(fails on %4: shards 0,2 get 0 buffers).

#407 — overload Reap forced a GC on every poll tick

StageReap ran runtime.GC() on every poll tick while the signal sat in the
Reap band (default CPU 0.80–0.85), not once on entry. Under sustained moderate
load that's a full STW GC every PollInterval (default 1s) — repeated capacity
reclaim precisely when the server is already loaded.

Fix: track prevStage (goroutine-local) and fire the GC only on
prev != StageReap && new == StageReap; re-entry re-fires. Gated behind
EnableReap (opt-in, default off). Also documents ReapAggressiveness==2 as
reserved (it currently behaves identically to 1). Regression test asserts the
NumForcedGC delta while lingering in the band is ≤1 (29 on the old code).

#408 — h2 inline-response outBuf bypass was wired but never used

h2InlineResponseAdapter / Processor.InlineWriter were fully built but
executeHandlerInline still set ResponseWriter = connWriter, so inline GET/HEAD
END_STREAM responses paid the sharded-queue mutex, a pooled buffer, and an
eventfd self-wake syscall on the already-awake event-loop thread, then drained
back into outBuf.

Fix: route inline execution through InlineWriter so buffered responses append
straight to outBuf. Safe because inline runs on the event-loop thread under
H2State.mu.

⚠️ The naive one-line swap would have broken SSE/chunked-over-H2:
Context.StreamWriter() type-asserts ResponseWriter.(stream.Streamer) and 500s
on nil, and h2InlineResponseAdapter was not a Streamer — a reachable regression
on a default GET /events route that neither the unit tests nor a throughput bench
would catch. So the inline adapter now implements Streamer, delegating
WriteHeader/Write/Flush/Close to the queue (streaming may run on a detached
goroutine, which must not write outBuf directly). Buffered → direct outBuf (the
win); streaming → queue (safe). A nil-guard falls back to connWriter for
Processors built without the conn layer.

End-to-end regression test asserts the inline writer is both the direct-outBuf
adapter (optimization applied) and a stream.Streamer (SSE-safe).

Verification

go test -race ./internal/conn/... ./protocol/h2/... ./middleware/overload/ ./middleware/sse/ — green.
Each fix's regression test verified to fail on the pre-fix code and pass on the fix.
Full go build ./... + go vet clean.
(Recommended pre-merge: a nightly-tier soak + an inline-GET/HEAD h2 microbench to confirm the h2: inline-response bypass (h2InlineResponseAdapter / Processor.InlineWriter) is wired but never used #408 syscall/throughput win on the native engines.)

Issues

The engine-integrated WS reader closed r.ch in closeWith to wake a blocked Read. But Append (called on the engine event-loop thread) sends on r.ch, so a peer-RST-triggered closeWith racing an in-flight Append panicked with 'send on closed channel' -> the epoll loop goroutine died -> refapp exit code 2 -> I-LIVENESS. The v1.5.7 24h weekend soak (run 28850347700) tripped this on BOTH amd64 and arm64 (~4h in, auth_session_ratelimit/epoll). The atomic 'closed' flag cannot serialize a send against a concurrent close (TOCTOU). Fix: never close r.ch. Add a 'done' channel closed exactly once by closeWith; Append and Read select on it. The send target is always an open channel, so it can never panic; a concurrent close makes the <-done case ready and the chunk is dropped cleanly. Engine-agnostic (fixes epoll/iouring/adaptive alike). Adds TestChanReaderAppendCloseRace (reproduces the exact panic + data race on the old code, race-clean on the fix) and TestChanReaderAppendAfterCloseNoPanic.

Drives the epoll async WS upgrade+close path with concurrent SO_LINGER=0 RSTs (mid-upgrade and post-upgrade) so OnError->closeWith races the Append data callback over real TCP. Panics on the old close(r.ch) reader, clean on the done-channel fix. Linux-only; skipped under -short.

drainRead read cs.writeFn at the top of the per-read block, before the async-dispatch branch. A promoted-async conn is owned by its dispatch goroutine, which rewrites cs.writeFn during Context.Detach (the OnDetach callback in initProtocol). The async branch never uses that local, so the early read was dead there yet still raced the Detach write (the pre-existing loop.go:989-vs-1458 data race the wsrst torture flags). Read cs.writeFn only on the inline path, where it is actually used and the conn is not yet detached. Pure code-motion; no behavior change on the inline/H2 paths.

Under a peer RST during an async WS/SSE upgrade, closeConn->CloseH1 released the per-conn cached Context + stream back to their pools while the middleware goroutine was still running tryEngineUpgrade on that same Context. The handoff flag (h1State.Detached) was Store(true)'d only AFTER OnDetach released detachMu, so a closeConn acquiring detachMu in that window saw Detached==false, ran CloseH1, and recycled the live Context -> WSRawWriteFn (websocket.go:243) nil-derefed c.stream, panicking and cascading into pooled-object races across Context (context.go) and the H2 Stream pool (175 -race blocks in the wsrst torture; the panic on the non-detached recover path double-released too). Fix: publish Detached BEFORE releasing detachMu in OnDetach (the async-promoted detach path), and have both closeConn and the shutdown teardown RE-READ Detached under detachMu before CloseH1. detachMu is released after the Store (before the detachQueue enqueue is fine either way — detachQMu is a leaf lock, so no order inversion). A closeConn that wins the lock now observes the ownership handoff and leaves teardown to the middleware goroutine. Pre-existing (v1.5.6); surfaced by the wsrst RST-mid-upgrade torture.

…100873) Adversarial review found the io_uring engine carries the byte-for-byte identical use-after-recycle that e100873 fixed only in epoll: OnDetach published h1State.Detached AFTER releasing detachMu (worker.go:1517 unlock before :1548 store), and both closeConn (worker.go:2412-2417) and the shutdown teardown (worker.go:3787-3799) read Detached OUTSIDE detachMu and called conn.CloseH1 with no re-read. Under a peer RST mid-WS/SSE-upgrade on an async-promoted conn, closeConn wins the free lock, sees Detached==false, CloseH1-recycles the live cached Context+stream, and the upgrade goroutine's WSRawWriteFn nil-derefs the recycled c.stream -> panic + pooled-object races. io_uring is a first-class engine (adaptive promotes into it), so v1.5.8 needs this. (The writeFn fix 6449d07 needs no iouring analog: its recv handler early-returns for async-promoted conns and reads cs.writeFn only at the inline ProcessH1 sites after the async branch.) Fix mirrors e100873: publish Detached before releasing detachMu in OnDetach; re-read Detached under detachMu in closeConn and shutdown before CloseH1. detachQMu is a leaf there too, so no lock-order inversion. Also parametrize wsrst_torture_linux_test.go over BOTH native engines (was epoll-only, which is how the iouring copy slipped review) with an io_uring-unavailable skip.

The done-channel crash fix added per-message select overhead on the engine-integrated WS read path — a focused N=2 bench showed ws-echo -1.5 to -2.4% on native engines (std, which uses the hijack path, was flat). Append's <-r.done select case was dead weight: ch is NEVER closed (closeWith closes done, not ch), so the send can neither panic nor block forever (it's a non-blocking select-with-default). Dropping the case reverts Append to the fast single-op select while preserving the done-channel correctness — a blocked Read still wakes on done; close(r.ch) stays 0. Recovers most of the ws-echo cost (io_uring/adaptive back to ~flat, epoll ~-1%). Read keeps the 2-case done-select unchanged: a non-blocking read fast-path was tried but dropped — it helped ws-echo but muddied ws-hub-broadcast, so only the unconditional Append win is kept. Local go test -race chanReader (incl. AppendCloseRace regression) x15 + full websocket -race stay green.

…is#406 Client HTTP/2 request streams are always odd (RFC 9113 §5.1.1) and Celeris does no server push, so streamID%h2QueueShards (=4) only ever hit shards {1,3} and left 0,2 dead — the 4-shard queue was effectively 2 shards. Shift off the constant low bit so odd IDs 1,3,5,7,… spread across all shards. Pure function of streamID, so per-stream HEADERS-before-DATA FIFO + single-writer drain are unchanged; only the shard distribution improves. Adds a regression test (fails on %4: shards 0,2 get 0 bufs).

…ris#407 StageReap forced a full runtime.GC() on every poll tick while the signal sat in the Reap band (default CPU 0.80-0.85), not once on the transition into Reap. Under sustained moderate load that runs a STW GC every PollInterval (default 1s) — repeated capacity reclaim precisely when the server is already loaded, doing more harm (GC CPU + STW pauses) than good. Track prevStage (goroutine-local; run() is the sole writer of the stage atomic) and fire runtime.GC() only on prev!=StageReap && new==StageReap. Re-entry (Reap->Reorder->Reap) correctly re-fires. Gated behind EnableReap (opt-in, default off). Also documents ReapAggressiveness==2 as reserved (it currently behaves identically to 1; no pool-drain primitive is wired). Adds a NumForcedGC-delta regression test (29 GCs while lingering pre-fix, <=1 post-fix).

The direct-to-outBuf inline path (h2InlineResponseAdapter / Processor.InlineWriter) was fully wired but never read: executeHandlerInline still set ResponseWriter = connWriter, so inline GET/HEAD END_STREAM responses paid the sharded-queue mutex, a pooled buffer, and an eventfd self-wake syscall (Enqueue on the same event-loop thread that is already awake), then drained back into outBuf. Route inline execution through InlineWriter so buffered responses (WriteResponse) append straight to outBuf, skipping all of that. Safe because inline runs ON the event-loop thread under H2State.mu — the same invariant control frames rely on. The naive swap would have broken SSE/chunked-over-H2: Context.StreamWriter() type-asserts ResponseWriter.(stream.Streamer) and 500s on nil, and the inline adapter was not a Streamer. Fix: h2InlineResponseAdapter now implements Streamer, delegating WriteHeader/Write/Flush/Close to the queue adapter (streaming may run on a detached goroutine, which must NOT write outBuf directly). So buffered -> direct outBuf (the win), streaming -> queue (safe). nil-guard falls back to connWriter for Processors built without the conn layer (unit tests). Adds an end-to-end regression test asserting the inline writer is both the direct-outBuf adapter AND a stream.Streamer.

Combines the H2 write-queue sharding, overload Reap-GC, and inline-response outBuf-bypass fixes (celeris#412) into the v1.5.8 branch alongside the WS chanReader + Context/stream use-after-recycle fixes, so the release is a single PR. No file overlap between the two sets; clean merge.

Two CI checks were red on the combined v1.5.8 branch: - Lint (ineffassign): the detachMu-held Detached re-read added in the Context/stream use-after-recycle fix set trulyDetached=true in a branch nothing reads afterward (the only read is the outer 'if !trulyDetached'). Invert the guard to 'if !Detached { CloseH1 }' — byte-identical behavior (CloseH1 only when the conn did not detach while we held the lock), dead store removed. epoll/loop.go + iouring/worker.go (closeConn + shutdown). - Vulnerability Check (GO-2026-5856): crypto/tls@go1.26.4 is affected; fixed in go1.26.5. Bump the setup-go toolchain pin 1.26.4 -> 1.26.5 in ci.yml, drivers.yml, release.yml so govulncheck sees the patched stdlib and the shipped release binary is built against it. The go.mod 'go' floor stays 1.26.4 (minimum language version) to avoid forcing a toolchain auto-download on downstream consumers and cluster runners.

FumingPower3925 added 9 commits July 8, 2026 13:11

FumingPower3925 mentioned this pull request Jul 11, 2026

perf(h2,overload): fix write-queue sharding + Reap GC storm + realize inline-response bypass (#406 #407 #408) #412

Closed

FumingPower3925 changed the title ~~fix: WS-upgrade crash — chanReader send-on-closed + Context/stream use-after-recycle (epoll + io_uring)~~ v1.5.8: WS-upgrade crash + Context/stream use-after-recycle + H2/overload perf (#406/#407/#408) Jul 11, 2026

FumingPower3925 force-pushed the fix/ws-chanreader-send-on-closed branch from 5a26a0f to 1facc51 Compare July 14, 2026 21:57

FumingPower3925 merged commit 4bdcef6 into main Jul 14, 2026
62 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

v1.5.8: WS-upgrade crash + Context/stream use-after-recycle + H2/overload perf (#406/#407/#408)#411

v1.5.8: WS-upgrade crash + Context/stream use-after-recycle + H2/overload perf (#406/#407/#408)#411
FumingPower3925 merged 11 commits into
mainfrom
fix/ws-chanreader-send-on-closed

FumingPower3925 commented Jul 11, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

FumingPower3925 commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

The bugs & fixes

Verification

Perf (N=2 focused saturation bench vs v1.5.7)

Out of scope (follow-ups)

Also in this PR — H2/overload perf fixes (merged from #412)

#406 — h2 write queue used only 2 of its 4 shards

#407 — overload Reap forced a GC on every poll tick

#408 — h2 inline-response outBuf bypass was wired but never used

Verification

Issues

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

FumingPower3925 commented Jul 11, 2026 •

edited

Loading