Follow-ups from #772: the standalone driver event loop reads a reused descriptor number after UnregisterConn (#710 there), the WorkerLoop contract sentence, a test for the check-and-read critical section, UnregisterConn doc

[FumingPower3925]

The review of #772 (celeris#710) left these minor findings and nits. None of them blocks #772, whose failing-first result and suites the review reproduced. Item 1 is a defect in code #772 does not touch; Fixes #710 would otherwise close the only record near it. Each item says what to do.

1. The standalone driver event loop has #710's defect, in a worse form (minor per the review; a defect, not yet fixed)

driver/internal/eventloop is the loop the drivers use when they are not given an engine (no celeris server running). Its worker reads a driver conn by descriptor number after UnregisterConn has returned, as the epoll engine did before #772:

• worker.handleReadable (origin/main driver/internal/eventloop/loop_linux.go:1058-1095) holds c.recvMu and reads fd in a loop until EAGAIN, calling c.onRecv after every read, and never checks c.closed.
• worker.UnregisterConn (:239-264) deletes the map entry, runs EPOLL_CTL_DEL, sets c.closed under c.mu and fires onClose. It takes neither recvMu nor anything else a worker inside handleReadable holds, so it does not wait for the read loop.

So after UnregisterConn(A) has returned, the caller closes A and another file X takes the number, a worker still inside A's read loop reads X. On the engine (#710) those bytes were dropped; here they are handed to A's onRecv, i.e. into A's protocol parser, and X never sees them. A blocking X with nothing to read parks the worker and every conn on it.

The review reproduced it with a probe in the shape of #772's tests (the worker parked in A's first onRecv after a full 16 KiB read, then UnregisterConn(A), close A, dup3 a new socketpair X onto the number, 10 bytes written to X's peer, the worker released). Its results, Docker linux/arm64, -race: A.onRecv after UnregisterConn returned: ["XXXXXXXXXX"]; X still holds "" (read err resource temporarily unavailable) FAIL 5/5 on #772's head 086c5b1, FAIL 3/3 on main 5936dd8, and FAIL on main + #772 + #773 + #774 + #776; the blocking variant fails with "the worker served no other conn for 3 s". The probe files are kept in the maintainer's evidence tree (evidence/lanes-20260927/EPOLL-HIJACK/followups/review-probes/zz_review710_standalone_test.go, and the second reviewer's version next to it); this lane has not re-run them.

To do: a failing-first test from that probe, then the same fix as #772: check c.closed and issue each read in one critical section with the flag UnregisterConn sets before it returns (here recvMu already serialises the reads with WriteAndPoll's caller-side reads, so the check can go under it, or UnregisterConn can wait on it). Check the WriteAndPoll* caller-side read paths for the same number reuse. A read-path lock change needs a measurement.

2. The engine.WorkerLoop contract says the opposite of what epoll guarantees once #772 merges (minor)

engine/provider.go says the epoll worker "still reads fd by number afterwards, so a number reused at once can lose its first bytes to it (celeris#710)". #772 left the sentence alone because #744 rewrote that paragraph and kept the sentence. #744 has since merged (f0886ca, 2026-09-28T03:05Z), so the sentence is on main, and #772 (based on 698bed6, before #744) does not touch the file: once #772 merges, main tells driver authors something epoll no longer does.

To do: when #772 merges (or in a merge of main into it), replace the sentence: the epoll engine fires onClose before UnregisterConn returns, and its worker reads fd no more once UnregisterConn has returned (celeris#710). While item 1 is open, add that the standalone driver event loop still reads by number after it.

3. No test pins that the closed check and the read share one critical section (minor)

#772's mutant m1-toctou (check dc.closed under dc.mu, release the lock, then read) survives 3/3: the tests park the worker so that the whole unregister happens before the worker's next check, which proves a check comes before each read, not that the check and the read are atomic. The fix is right by construction (driver.go:340-347 at 086c5b1: lock, check, read, unlock), but the suite cannot tell it from the broken check-then-read version.

To do: a nil-by-default test hook between the check and the read (for example a package-level func(fd int) the test sets), and a test that calls UnregisterConn, closes the fd and reuses the number from inside that hook. The hook is on the driver read path, so its cost (one load and a nil branch per read) needs a measurement, or a build-tagged hook.

4. UnregisterConn's doc: "the worker reads fd no more" is not "onRecv is not called again" (nit)

With #772, an UnregisterConn that races a read waits on dc.mu for the read syscall to finish, then fires onClose on the caller's goroutine while the worker calls onRecv with the bytes it just read (driver.go:340-361 at 086c5b1: unlock at :347, cb(...) at :360). Before #772, the worker's check after the read dropped those bytes, so an onRecv concurrent with or after onClose had a window of a few instructions; now it is as long as a read syscall. The contract does not forbid it and the redis driver serialises the two through its bridge, but the doc #772 wrote invites the stronger reading.

To do: one sentence in UnregisterConn's doc: a read that completed before UnregisterConn took dc.mu is still delivered, so onRecv can run once more, concurrently with or after onClose.

Done in #772's body (no action left)

• The body said "The lock/unlock count per read is unchanged". It is not: the head takes dc.mu before every read, including the final one that returns EAGAIN, EOF or an error, where main locked only after a read with n > 0 (a full 32 KiB read then EAGAIN: 1 lock on main, 2 on the head). The body now says so; the cost is what the benchmarks measure.
• The benchmark and its scripts lived only in the maintainer's evidence tree. The benchmark is now on two pushed branches, measure/710-driverread-base (main 698bed6 + the bench file) and measure/710-driverread-fix (fix(epoll): never read a driver conn's descriptor number after UnregisterConn has returned (celeris#710) #772's head 086c5b1 + the same file), which the queued cluster timing run uses.

[FumingPower3925]

Item 1 has now fired in the field: the one h2c_hang of the 24 h weekend soak

Soak run: https://github.com/goceleris/probatorium/actions/runs/36433207097 (probatorium 38779a4, celeris pinned at dbbaaee, which includes #793). The soak step passed. This event is ValidateGate's sixth violation, and it is also the only row behind ValidateDiff's strict MED error h2c_churn/h2c_hang.

What the soak recorded

• Where: arm64 (msr1), cell-19 driver_redis on the adaptive engine. 1 of the cell's 280,302 h2c probes got no answer and no decline: h2c_hang=1 (hang_eof=1), read 10,001 ms, err="EOF", local 127.0.0.1:28244, reported 2026-09-29T05:32:08.41Z.
• Totals: arm64 had 1 hang in 5,301,259 h2c probes across 32 cells. amd64 had 0 in 5,301,772 across 32 cells.
• The 4 diff.json rows are this one event. Three compare it with the other arm64 engines and one with amd64. In the cross-arch row the values are in (amd64, arm64) order, so the 0 1 does not mean amd64 hung.
• Not a panic and not io_uring, epoll: a panic on the async dispatch goroutine leaves cs.detachMu locked, and closeConn then parks the whole worker on it #791: the cell has requests_panic_expected 0 and I-PANIC was judged with 0 violations. The dossiers show 0 Mutex.Lock, closeConn or panic frames.

In-stall dossier (I-H2C-STALL at 05:32:01.41Z, 3 s into the read):

• The server socket (driver_redis fd 154) is ESTAB with Recv-Q 0, bytes_received:126 and no bytes_sent, lastrcv:3037. The validator's socket (fd 121) shows bytes_sent:126.
• It is the only server socket in the dump holding an unanswered request older than 181 ms.
• The goroutine dump has 177 goroutines, 12 of them standalone driver event loop workers.

HANG dossier (05:32:10Z): the server side of 127.0.0.1:12531<->28244 is in TIME-WAIT, and fd 154 is now a redis connection.

The close came from the header deadline. It is armed at accept (engine/iouring/worker.go:2505-2510 at dbbaaee) and cleared by any successful parse. The walker reads for 20 s and the redis ReadTimeout is 3 s. A 10.0 s EOF therefore means the server never parsed the headers. The request bytes left the socket, but no HTTP/1 parser saw them.

Which engine held the connection: in the 1 Hz series, standby_active goes 22->0 at 05:31:57Z and stays flat. So the hung connection was on io_uring after the switch (05:31:48Z), not on the epoll standby.

The mechanism: item 1 (code at dbbaaee)

• The refapp uses the standalone loop: driver_redis calls redis.NewClient(*addr) with no engine (probatorium 38779a4 validation/refapp/driver_redis/main.go:69).
• The worker never checks c.closed: driver/internal/eventloop/loop_linux.go:1058-1095 handleReadable reads fd by number to EAGAIN (unix.Read :1075, onRecv :1077).
• UnregisterConn does not wait: loop_linux.go:240-264 sets closed under c.mu and does not wait for a worker already in that loop.
• The close order frees the number: redisConn.Close calls UnregisterConn (driver/redis/conn.go:946), then file.Close (:953). The pool closes connections in Release above the idle cap (driver/internal/async/pool.go:242-243) and in Discard on a canceled or timed-out context (:257-258).
• The stolen bytes vanish silently: a parse error in redisConn.onRecv (conn.go:312-319) is stored and nothing is logged.
• No change on main: loop_linux.go is byte-identical at fe9264f and dbbaaee.

The preconditions were met in the field.

• Redis churn: between the two dossiers (8.7 s), 26 of 48 redis connections closed and 26 were dialled.
• Descriptor reuse: fd 154 was the hung HTTP connection at 05:32:01 and a redis connection at 05:32:10. Four more descriptors (35, 148, 161, 162) moved from redis to HTTP in the same span.
• Pool saturation: at the in-stall dump the pool was full (48/48 open, 29 goroutines blocked in Pool.Acquire).
• Steady source of cancels: a third of the h2c probes are reset after the write. That keeps Discard busy as well as Release.

Reproduction (linux/arm64 containers on the maintainer's laptop, celeris dbbaaee)

The repro is a celeris server with AsyncHandlers whose handlers use driver/redis.NewClient against real redis 7.2, driven by a separate h2c-churn and keep-alive client.

io_uring, -maxidle 1 (the setting only raises the rate of pool closes):

Arm	hang-eof	Probes
Pristine dbbaaee	9	2,186,933
Witness build	25, each joined by probe id to a "driver loop read this request after UnregisterConn" line	2,348,179
Fix (closed check plus read under c.mu)	0	2,300,705

• The chance of the fix arm's 0 at the pristine rate is about 7.7e-5.
• The witness also caught the dead connection eating 5 HELLO replies of new redis connections and 3 bulk replies.

std engine, -maxidle 1: 134 of 4,139,033 probes hung, all 134 joined to the witness. So the thief needs no celeris engine at all, which separates it from #715(a).

• The in-stall capture resolved 14 of 20 stolen sockets.
• Each was ESTAB with rx_queue 0 and tx_queue 0 about 7 ms after the theft, and each matches a probe that then got hang-eof at about 10 s. That is the field's picture.

Default pool: the race is rare after warm-up.

• Session 1 had 1 joined hang in its first 2m10s. That arm was cut, so its count comes from client.log only; the same log also shows 5 hang-timeouts around the cut.
• Session 2 had 0 hangs in 3,494,321 probes over 8 min.

Not reproduced on amd64: no amd64 host was available. The race is portable Go, so it is expected there too.

Limits of the attribution

The field data alone does not name the reader. The dead redis parser drops the bytes silently, and probatorium#450 means stale-recv counts cannot be placed in time. The attribution to item 1 rests on the repro.

Two cell counters bear on a residual io_uring stale recv (the #715 shape):

• Weakly for it: the hung cell's engine_stale_recv_data_closed is 2. It is 0 in its amd64 twin and 0 in the 6 other driver_redis cells. But 7 of the 27 other non-auth io_uring and adaptive cells have 1-13 with no hang, so the likelihood ratio is only about 4.
• More strongly against it: this soak's auth_session_ratelimit io_uring and adaptive cells had 1,515-5,092 stale_recv_data_closed each, yet 0 hangs in 815,658 probes.

The timing is suggestive, not proven. The event came 10.4 s after the epoll->io_uring switch and about 1 s after the 22 standby closes, with 0.51% of the cell's accepts done. No artifact measures redis churn after 05:32:10, so "pool warm-up" is not verified. If churn was steady, an event this early is a coincidence of about 0.5%.

Why only arm64

This is not an instruction-set difference: the race is in portable Go with no per-arch code. What did differ was the two hosts' exposure in this cell:

• Traffic was about equal. amd64 sent 37,493,900 requests in cell-19 against arm64's 34,492,777 (1.09x). Every other driver cell differs by 2.9-4.5x.
• Host contention was not. msa2-server ran its cell-19 at load1 median 5.0. msr1 ran at 28.9, with 30 runnable processes in the sample at 05:31:58Z, the minute of the write.
• That pressure widens a scheduling race. It gives a driver worker more chances to be descheduled between onRecv and its next read.

So the arm64 run plausibly had the higher rate for host-contention reasons. A 1-vs-0 count is not a cross-arch code divergence, but amd64 is not immune.

Fix direction

• The fix: what item 1 says. Check c.closed and issue each unix.Read in one critical section with the flag UnregisterConn sets before it returns (repro/fix.patch does it under c.mu). Alternatively, UnregisterConn can wait on recvMu (the fix(epoll): never read a driver conn's descriptor number after UnregisterConn has returned (celeris#710) #772 shape).
• Tests and audits:
  • Add a failing-first test.
  • Audit the WriteAndPoll* caller-side reads and the memcached and postgres drivers on the same loop.
  • Measure the cost of the read-path lock.
• Priority: the repro shows it also misdelivers redis replies (HELLO, bulk) between connections, so it is more than "minor".
• probatorium side (observability only): add a counter for bytes delivered to an already-closed driver conn's onRecv, and land probatorium#450 and deps(compose): bump redis from 8.8 to 8.10 in /test/conformance/redis in the all-compose-images group across 1 directory #457.

Evidence

Evidence root (maintainer machine): /Users/fuming/.claude/projects/-Users-fuming-Documents-github-celeris-probatorium/evidence/soak-36433207097/

• h2c/: FINDING.json, MANIFEST.txt, scripts/run_all.sh (regenerates derived/* and repro/REPRO-SUMMARY.txt offline), repro/ (program, patches, runs).
• h2c/SKEPTIC.json and h2c/skeptic/: the adversarial re-derivation, with its corrections applied above.
• artifacts/matrix-weekend-results-36433207097/.../20260928T171604-validate-msr1-auth_session_ratelimit/cell-19-driver_redis-adaptive/incidents/{20260929-053201-I-H2C-STALL,20260929-053210-I-H2C-HANG}/
• Job log: logs/matrix-soak-108994561896.log lines 6865 and 6883-6891.

[FumingPower3925]

Item 1: draft PR #843. It fixes the standalone loop's reads after UnregisterConn (in handleReadable and all three WriteAndPoll* paths), makes the teardowns act on the conn instead of the descriptor number, and fixes the same delete-by-number in the non-Linux fallback.

It includes:

• failing-first tests, a negative control and mutant controls;
• the soak repro as a second witness: 100 hang-eof on origin/main, 0 with the fix;
• the measured cost of the per-read lock.

Items 2-4 (#772's epoll code) are untouched. I found a new defect in the same loop on the way and filed it as #842: a stale batched epoll event is dispatched by number and can tear down the next conn on that number.

[FumingPower3925]

More follow-ups from #772 (merge train 11)

5. Re-check the end-to-end epoll driver-read cost in the next perf checkpoint's driver cells

#772 merges on the orchestrator's decision. Its merge-gating cluster timing (queue rows 48 and 49, the pre-registered analyze_cluster710.py) came back INCONCLUSIVE with no FAIL. Five judged shapes were INCONCLUSIVE because the A/A twin's own band was wider than the bar, up to ±11% on x86 PipelinedG16x4K. More blocks would not shrink that.

To do: in the next perf checkpoint, read the driver cells (redis, memcached and postgres on the epoll engine) against the checkpoint before #772. Include a writer-contended shape. The cluster's Contended512 rows were reported, not judged: x86 reader 1.3156, writer 1.0644; arm64 reader 0.9363, writer 0.8602 (the flag is writer < 0.70).

6. CodeRabbit nit on #772: "Measure the writer-contended read path" (trivial)

Review of 086c5b1, engine/epoll/driver.go:340-347. driverRead now holds dc.mu across unix.Read, and Loop.Write takes the same mutex, so a concurrent writer can wait for the read syscall. The tight-loop benchmark shows fewer Write calls. CodeRabbit rates this a workload-specific effect, not a proven production regression.

Its suggested change is not taken: release dc.mu before the read. That change is #772's surviving mutant m1-toctou (check closed, unlock, then read), which reopens the window #710 is about. The measurement it asks for is item 5.

How the open items relate to #772's merge

• Item 1 (the standalone driver loop) is draft PR fix(eventloop): never read a driver conn's descriptor number after UnregisterConn has returned (celeris#784) #843 (Part of #784), which fix(epoll): never read a driver conn's descriptor number after UnregisterConn has returned (celeris#710) #772 does not touch.
• Item 2 is now due. Once fix(epoll): never read a driver conn's descriptor number after UnregisterConn has returned (celeris#710) #772 is on main, the sentence at engine/provider.go:89-92 on main (e2508c7), "a worker already inside the conn's read loop still reads fd by number afterwards ... (celeris#710)", is false for epoll. fix(epoll): never read a driver conn's descriptor number after UnregisterConn has returned (celeris#710) #772 does not touch the file.
• Items 3 and 4 stand as written. Main's changes to engine/epoll/driver.go since fix(epoll): never read a driver conn's descriptor number after UnregisterConn has returned (celeris#710) #772's base 698bed6 touch only RegisterConn (fix(epoll): publish a driver conn before arming its descriptor, so its first edge is never dropped (celeris#770) #776). driverRead, UnregisterConn and closeDriver merge unchanged. The line numbers there are at 086c5b1; on main after the merge they are about 10 lines lower.

[FumingPower3925]

7. CodeRabbit nit on #772: "Cover the check-to-read boundary, not only the callback boundary" (trivial). This is item 3.

CodeRabbit's review of 0949f4f flags engine/epoll/driver_unregister_readloop_test.go:225-267. The reuse tests park the worker in onRecv, so a driverRead that unlocks between the dc.closed check and unix.Read still passes them. CodeRabbit asks for a deterministic seam between the check and the read: while the worker is paused there, unregister A, reuse its descriptor, write X's data, then resume and assert that X's data is still unread.

This is item 3 above, which #772's mutant m1-toctou already showed (it survives 3/3). The to-do is unchanged: add a nil-by-default or build-tagged hook between the check and the read, and measure its cost on the driver read path. One item covers both.

[FumingPower3925]

Consolidated on 2026-10-02 (maintainer decision): polish items -> #885, #887, #892; defects moved -> none. What remains here: item 1. Why it stays open: item 1 (the standalone driver loop reads a reused descriptor number after UnregisterConn) is a live defect, fixed by draft PR #843, which says 'Part of #784'. This issue stays open as its tracker until #843 merges. Nothing was dropped.

Already done or superseded here, per this thread: item(s) body-done, 6, 7.

[FumingPower3925]

Item 1 has landed: #843, squash 9bfbc23 (merge train 12)

The standalone driver event loop (driver/internal/eventloop) no longer issues a read(2), write(2) or epoll_ctl on a conn's descriptor number once UnregisterConn has returned:

• Every read goes through readOpen, which does the closed check and the read in one critical section under the conn's new read lock. This covers handleReadable and all three WriteAndPoll* paths.
• The EPOLLIN mask and re-arm go through setEvents, under c.mu after the same check.
• Every teardown (UnregisterConn, errorClose, shutdown) sets closed with both locks held before it removes the conn. It then removes the conn by identity, not by number (forget).
• UnregisterConn's doc in both loops now says that onRecv can run once more. That is item 4, for this loop.
• TestReadCheckAndReadAreOneCriticalSection784 is item 3, for this loop.

All seven required checks were green on the merged head (3e05270).

This comment does not close the issue: #843 said Part of #784. After the consolidation above, the open parts are tracked elsewhere:

• Epoll-engine items 2-4 (engine/epoll, fix(epoll): never read a driver conn's descriptor number after UnregisterConn has returned (celeris#710) #772's code):
  • v1.6.0 polish: drivers #887: the engine/provider.go contract sentence. Its "while fix(eventloop): never read a driver conn's descriptor number after UnregisterConn has returned (celeris#784) #843 is open" clause no longer applies: the standalone loop no longer reads by number after UnregisterConn.
  • v1.6.0 polish: epoll engine #885: the epoll check-to-read critical-section test and the UnregisterConn doc sentence.
• Measurements: v1.6.0 polish: measurements and evidence records #892, which includes forget's EPOLL_CTL_DEL under the worker's map lock.
• Defects found next to fix(eventloop): never read a driver conn's descriptor number after UnregisterConn has returned (celeris#784) #843, still open:
  • eventloop: the standalone driver loop dispatches an epoll event by descriptor number, so a closed conn's stale EPOLLRDHUP tears down the next conn on that number #842: a stale epoll event dispatched by number tears down the next conn on that number.
  • postgres: after a worker-side teardown, Close writes Terminate to and unregisters the conn that has taken the closed descriptor number #859: postgres Close after a worker-side teardown.
  • eventloop: a Write racing Loop.Close can write the closed eventfd's number in wake(), which another socket may hold by then (data race) #862: the wake() and Loop.Close eventfd race.
  • eventloop: the standalone driver loop drains a conn to EAGAIN before the next event, so a conn whose inflow never pauses starves the others on its worker #881: a conn whose inflow never pauses starves the other conns on its worker.

Nothing that is tracked only here is still open. The maintainer can close this issue.

[FumingPower3925]

Consolidated on 2026-10-03. Item 1 landed in #843 (9bfbc23); see the comment above. Items 2-4 and the measurement items are tracked in #885 (epoll), #887 (drivers) and #892 (measurements). The related defects are #842, #859, #862 and #881. Nothing is left that only this issue tracks.