Follow-ups from #774: epoll drops a first segment shorter than protocol detection needs, a test for the connState-reuse face, an inline witness, the cost sentence

[FumingPower3925]

The review of #774 (celeris#769) left these minor findings and nits. None of them blocks #774, whose failing-first crash and suites the review reproduced. Item 1 is a defect in code #774 does not touch, found while reading drainRead. Each item says what to do.

1. epoll drops a first TCP segment shorter than protocol detection needs (minor per the review; a pre-existing defect, not yet fixed)

In drainRead (origin/main 5936dd8 engine/epoll/loop.go), every read lands at the start of the connection's buffer (recvBuf := cs.buf, :1194), and while the protocol is not yet detected an ErrInsufficientData from detect.Detect just continues (:1295). The next read then overwrites the bytes that were too few to detect. io_uring keeps them in cs.detectAccum; epoll has no equivalent. So a request whose first segment is short is parsed without its head: GE then T /x HTTP/1.1... is parsed as method T and answered 405, and an h2c prior-knowledge preface split before its 24th byte is lost the same way.

The review's probe (Workers 2, TCP_NODELAY, write GE, sleep 50 ms, write the rest; kept in the maintainer's evidence tree as evidence/lanes-20260927/EPOLL-HIJACK/followups/review-probes/zz_review_detect_split_test.go) gave, per the review, on main: epoll status=405 10/10, std 0/10, io_uring 0/10. The probe's first run (Workers 1) failed config validation for every engine and is void. This lane has not re-run it.

To do: a failing-first test from that probe (both the HTTP/1 split and a split h2c preface), then keep the undetected bytes across reads on epoll (read after them in cs.buf, or an accumulator as io_uring does).

2. The Defect section stated an unreproduced face as fact; no test pins ErrHijacked against a plain nil check (minor)

#774's body said: when another worker's accept had already taken the released connState, the same line wrote that connection's parser state instead. That face was found by reading and never reproduced, and the nil-check mutant (cs.h1State != nil instead of the ErrHijacked check) survives 3/3, because it avoids the crash the tests reach. #774's body now says the second face is by reading. The fix (loop.go:1402: if tryInline && !errors.Is(processErr, conn.ErrHijacked)) does not touch cs at all, by construction.

To do: a test that forces the connState's reuse between the release and the reset (e.g. a test hook after releaseConnState in the inline branch that runs another conn's accept on the same P), so the nil-check mutant fails.

3. Nothing asserts that /hj ran inline (nit)

engine/epoll/hijack_inline_async_loop_linux_test.go checks only e.loops[0].async as a precondition and hijacked.Load() == n. If routing changed so that /hj went async, both tests would pass without reaching the path under test; today only the failing-first crash shows they reach it.

To do: assert that the loops' asyncPromoted count stays 0 in the async-loop-no-async-routes arm (and the equivalent for the user-facing test).

4. The cost sentence covered only a nil processErr (nit)

The body said that with a nil processErr, errors.Is returns at its first comparison. On an async route, the first inline attempt of each request returns ErrAsyncDispatch and pays a full errors.Is call (a comparison and an Unwrap probe of a sentinel from errors.New). Negligible, but not what the body said; the body now names that case. Not measured by the lane.

To do: nothing beyond the body, unless the async-route dispatch path is ever benchmarked, in which case include it.

5. CodeRabbit on #774: the tests carry no negative control of their own (minor)

CodeRabbit's one finding on #774 (#774 (comment)): both regression tests assert only post-fix success. It asks that TestInlineHijackOnAsyncLoop be shown to fail on /hj with the unfixed loop.go, and that TestHijackWithAsyncHandlersOnEpoll fail if /hj does not reach the inline path.

The first half is evidenced outside the tests: #774's failing-first run (test-only commit 051dace on main 698bed6, each test 5 times in its own go test -race process) died with a panic: at drainRead 5/5 for both tests, and TestInlineHijackOnAsyncLoop kills the fix-reverted mutant 3/3. The second half is item 3: neither test proves /hj ran inline, so a routing change that sent /hj async would leave both passing without reaching the path under test.

To do: with item 3, give both tests an inline witness (the engine test through the loops' asyncPromoted count staying 0 in the async-loop-no-async-routes arm; the user-facing test through an equivalent observable), and cite #774's failing-first numbers as the unfixed-code control.

[FumingPower3925]

Consolidated on 2026-10-02 (maintainer decision): polish items -> #885; defects -> #870. Nothing was dropped.

Already done or superseded here, per this thread: item(s) 4, 5.