[FumingPower3925]

Refs #885 (items 42, 49)

Draft: lane L3 EP-TIMEOUTS of v1.6.0. Two commits, verify-first: the first holds the tests (assertions on the real code, each checked failing on main or, for the review-round ones, on the previous head), the second the fix. This PR is 885.49 and 885.42 only: Opus review r1 of the first revision rejected the 885.51 ceiling and approved these two; 885.51 is the separate draft PR #1119. Review r2 of the shrunken head (2a6d68a) and r3 of the next one (ba5b5f2) asked for the fixes listed below (r2 M1, M2, m1 to m3, n2; r3 M1, m1, m2, n4).

Premises, checked on main before any fix (commit 1 against main's code, docker golang:1.27.3, -race)

item	premise on main	evidence
885.49, pool-wait gate	true	TestShutdownPoolWaitStopsServingH1AndDetached884, epoll and adaptive-on-epoll arms added: 16 of 16 keep-alive requests answered 1.5 s into the shutdown, 672 / 688 ticks in 900 ms (the issue measured 688 to 704).
885.49, what the gate leaves (r1 M2, r2 M1/M2/m2/m3)	true	TestShutdownPoolWaitGateClosesIdleH1AndEndsDetached885 and ...RequestsInProgressAndIdleConns885 (see below): idle keep-alive clients still connected 2.5 s into a 20 s wait, producers still writing; on the previous head (2a6d68a, before the fixes of this round) the detached streams' clients timed out with no FIN 2 s after their last byte, a connection with no byte sent stayed open, and a 64 KiB-into-2 MiB upload was reset ("broken pipe"/"connection reset") at the gate.
885.42, empty rounds	true, no correctness impact	TestQueuedZeroTimeoutRoundsDoNotStretchTheIdleWait885: 32 turns queued on an AF_UNIX pair, last read sees consecutiveEmpty=31 and the next idle wait 2 ms (want 0 and 1 ms). Two turns are enough to fail (SO_SNDBUF 212992, the stock wmem_max: 1 on main, 0 here), so it does not skip on a stock runner.

What changed

49 (poolwait_gate_885.go, one call in Loop.run): 250 ms after the pool wait began (h2DrainStart), at most once per 10 ms:

• refuseRecv is set: drainRead reads no more from a conn with !detected || protocol == HTTP1, the predicate of io_uring's refuseRecv, except a request in progress (r2 M2): a conn whose parse state holds a partial request (HasPendingData: headers in pieces, a body still arriving) or whose protocol decision has begun (detectN > 0) is read on and answered, as std's Shutdown answers the request of an active conn, and closed when it next comes up idle;

• detached producers are ended with TryLock (as shutdown's phase 1 ends them: asyncClosed, the parked read-end error, detachClosed, OnDetachClose behind the WSReady check; hijacked re-tested under the lock); a producer's next Write gets an error wrapping net.ErrClosed; a conn left with bytes queued, no EPOLLOUT armed and not on the dirty list is put on the dirty list on every pass (drainDetachQueue skips a detachClosed conn);

• closeIdleH1 (r1 M2, extended by r2 M1 and m2): an HTTP/1 conn that holds nothing (no engine queue, no kernel send queue by TIOCOUTQ, no request in progress, no dispatch goroutine) is closed with a FIN, as std's Shutdown closes idle conns: an idle keep-alive conn, a conn that was accepted and sent no byte (a browser preconnect), and a detached stream the gate has ended, once what was queued for it has gone out, on a sync route or on an Async one (r2 M1 for the sync route; r3 M1 for the Async one, whose conn a dispatch goroutine owns: its state is read under detachMu (TryLock), detachClosed is only set under that lock by the gate, after the handler had left it, the producer's writes no-op, and closeConn takes the lock as shutdown's own teardown does; the Async-route arms of the FIN test run -race). At most 64 conns per pass (r2 n2: a loop with thousands of idle conns keeps serving the HTTP/2 conns the wait is for).

• r3 m1, Connection: close: once the gate has passed, every HTTP/1 conn the loop parses itself is marked close-after (H1State.CloseAfter, a field and three lines in internal/conn/h1.go, outside the lane's files: nothing sets it on io_uring), so a request still read on (one in progress) is answered with Connection: close and its conn ends once the response has gone, as net/http's response written after Shutdown began does: a client does not write its next request to a conn that is about to close.

42 (run): a round with no events does not count toward consecutiveEmpty when a driver queue was owed (owedDriverReads != 0).

Tests added in this round (commit 1; the sync arm is r2 m3)

TestShutdownPoolWaitGateRequestsInProgressAndIdleConns885, arms sync (no Async route: the default synchronous loop) and async-route (an Async route exists: dispatch goroutines), the wait held by 24 HTTP/2 responses of 16 MiB that nobody gives window to (the reviewer's probe, as assertions): idle keep-alive conns get a FIN within 1.5 s; conns with no byte sent get a FIN (they are accepted about a second after the SYN: TCP_DEFER_ACCEPT); a 16 MiB download queued in the engine for a slow reader arrives whole; on the sync loop a 2 MiB POST with 64 KiB sent when the shutdown begins is answered got 2097152 after the gate; on the async-route loop a conn an Async route promoted is not closed (pinned: the documented exception). TestShutdownPoolWaitGateClosesIdleH1AndEndsDetached885 gains the FIN assertion: after the flushed bytes of each detached stream the client reads EOF or unexpected EOF within 2 s (a reset does not pass: r3 n4), in arms for streams on a sync route and on an Async route (r3 M1: it fails on the previous head with an i/o timeout for the Async-route arms). The sync arm of the in-progress test asserts resp.Close and the FIN after the answer (r3 m1).

Not done / known gaps (the loop's follow-ups.md has the drafts)

• On a server with any Async route (r3 m2): every conn whose read leaves a partial request (headers in pieces, a Content-Length body still arriving) is promoted to a dispatch goroutine whatever its route, so such a request is not read at the gate (a POST 64 KiB into 2 MiB, a GET with headers in pieces: hung to the end of the wait, as the reviewer measured); and an idle conn on an Async route is not closed. Follow-up (F2), shared with io_uring. (A chunked body is not promoted and is read on.)
• io_uring has none of the closes: by code reading (not run), its gate neither closes idle keep-alive conns, nor gives an ended detached stream a FIN (worker.go ~7901, and its detach queue skips a detachClosed conn ~6579), nor reads a request in progress, nor closes a conn that sent no byte. So after this PR the engines diverge on the idle close: epoll closes, io_uring waits for the end of the wait. Follow-up (F1); not touched here.
• The markDirty flush of detached bytes nothing else would flush has no test that fails without it (with the conns observed the remainder is always on EPOLLOUT); a WS conn not yet WSReady at the gate is untested.
• A keep-alive request that reaches the kernel just before the gate is never read, and the close then resets it (r2 n1: std has the same race; Go's transport retries idempotent requests).
• iouring/worker.go ~3913 still says epoll's pool wait does not stop reading ("celeris#885"): stale after this PR; reported for the next io_uring PR.

Release-note lines

• epoll: while a shutdown waits for HTTP/2 handlers, 250 ms into the wait the loop stops reading new HTTP/1 requests, closes idle HTTP/1 keep-alive connections and connections that sent nothing (their clients retry elsewhere instead of hanging until the end of the wait), and ends detached streams (Server-Sent Events, WebSocket), on any route: the producer's next Write returns an error wrapping net.ErrClosed, what was already queued is still flushed, and the stream's connection is then closed (a FIN). A response still going out is delivered whole. A request already in progress when the gate comes (headers in pieces, a body arriving) is read on and answered with Connection: close, except on a server that registers any Async route: there, such a request is not read (its connection is promoted to a dispatch goroutine whatever its route) and, like an idle connection on an Async route, it waits for the end of the wait. io_uring's gate does not close idle connections yet (it refuses reads and ends detached producers only): the two engines differ on this until a follow-up.
• epoll: a flood of queued driver reads no longer makes the next idle epoll_wait start from a stretched timeout.

loop.go outside the named functions (for rebasers)

Struct fields refuseRecv/producersStopNext; one if l.refuseRecv && refusesRecv(cs) at the head of drainRead (a load and branch per read event; the predicate runs only after the gate); the consecutiveEmpty branch in run; the call in the ctx.Err() block of run. checkTimeouts and sweepWriteClock are untouched (the sweep is byte-identical to main). The drainRead gate measured within noise (BenchmarkKeepAliveDispatch/nodriver 12338 base, 12606 head ns/op, A/A 11880, --cpus 2).

Verification

• The new tests and the epoll and adaptive-epoll arms of TestShutdownPoolWaitStopsServingH1AndDetached884: -race -count=20 (ok on the final head: the gate tests x20, 405 s; internal/conn and the full internal/engine/epoll package -race: ok); TestQueuedZero..., TestConnStateStaysInItsSizeClass: -count=20; the full ./internal/engine/epoll/ package -race; golangci-lint v2.14: 0 issues. The io_uring arms of the shutdown tests are not verified locally (docker has no io_uring): CI is the evidence.

[coderabbitai]

Caution

Review failed

The pull request is closed.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

❌ Patch coverage is 4.00000% with 72 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/engine/epoll/poolwait_gate_885.go	0.00%	69 Missing ⚠️
internal/engine/epoll/loop.go	50.00%	2 Missing ⚠️
internal/conn/h1.go	50.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!