[FumingPower3925]

Flakes lane. TestShutdownPoolWaitGateRequestsInProgressAndIdleConns885/async-route failed on #1154 (run 40732286713, attempt 1, job 122971491083, the io_uring base tier job; the failing half is epoll): read tcp ...: i/o timeout at 1.501s, want a FIN (io.EOF) once the gate has passed. This is a product defect in the epoll engine's shutdown (a user can see it), not a test race. It is a regression of unreleased v1.6.0 code: the hold comes from shutdown's send drain (#760, phase 2b), which v1.5.11 did not have (v1.5.11's shutdown() closed every conn at once). The gate (#1116) hides it on a loop with an HTTP/2 conn. It is not a regression of #1148/#1153. Refs #885, Refs #1104, Refs #1154, Refs #1165, Refs #1166

Root cause

A loop with no HTTP/2 conn has nothing to wait for (h2PoolSettled is true at once), so it never raises the pool wait's gate and goes straight into Loop.shutdown(). Phase 2b of that is drainSends, which polls every conn that has a response queued until its peer takes it, up to the budget of the Shutdown (20 s in the test), and phase 3, which closes the conns, runs only after it. So an idle keep-alive conn (or one that sent nothing) on the same loop as ONE slow download got no FIN for as long as the drain lasted. A loop that does have an HTTP/2 conn closes its idle conns 250 ms in (poolWaitGate -> closeIdleH1); io_uring is, from reading its code, not affected (its drain runs inside the run loop and closeIdlePaced closes idle conns past the 250 ms floor, also on a worker with no HTTP/2 conns; not measured, io_uring is not available in my docker); net/http's Shutdown closes idle conns at once.

In the CI test there are 24 HTTP/2 conns on 4 SO_REUSEPORT loops, so a loop without one is rare (about (3/4)^24, 0.1%, for the download's loop), which is why it showed once.

The CI log also over-reports: the test read the conns one after the other, and a Go read on a conn whose deadline has passed fails before it looks at the socket (FIN or not), so the first late conn ("idle keep-alive 3") became five reports (idle 4, 5, silent 0, 1 were never really waited for). The test now reads all of them at the same time and reports each one's own wait.

The "+33 requests (want 32)" line is a log line of a passing wait: RequestCount counts the loop's read turns, not requests; it is not part of the failure and is untouched.

What a user could notice (release-note-worthy), and what this PR does not fix

On epoll, during Shutdown / context cancel, a keep-alive client whose conn was idle waited (up to the Shutdown budget, with no answer) for its FIN if another client on the same loop was slowly reading a large response and the loop had no HTTP/2 conn. After this PR such a conn gets its FIN at once, unless it is asyncPromoted: the flag is sticky and is set for every conn that ever served an Async route and, on a server with any Async route, for every plain conn whose request once arrived in pieces. Those conns are still held behind the drain for the whole budget (measured end to end by the review of this PR: 8 of 8 Async-route and split-head idle conns timed out at 3 s; tracked as #1165, the "follow-up F2" of #1116/#1138). Also not fixed here: a request in progress (slow upload, headers in pieces) on a loop with no HTTP/2 conn is not read and answered as the gate does on loops with one (#1166); and an Async-route conn on such a loop is not "cut at once" in general: with a slow download on its loop it first waits behind the drain, and is then cut by phase 3.

Differences from net/http's Shutdown, stated so nobody reads "like std": std closes StateIdle conns at once, but a conn that was accepted and has sent nothing (StateNew) is not closed until 5 s after accept, and a request it sends meanwhile is answered; the pass (like the gate) closes such a conn at once. This is deliberate (it mirrors the gate), and main's phase 3 closed it too, with an RST when bytes had arrived. OnDisconnect is still not uniform at shutdown: conns closed by this pass (and by the gate) fire it and count in closeCount / activeConns; conns closed in phase 3 still do neither.

Reproduction

• Unit, deterministic (commit 1, red on main): internal/engine/epoll TestShutdownClosesAnIdleConnWhileAnotherConnsSendDrainWaits1104: a rig loop with one conn whose peer reads nothing (2 MiB queued) and one idle conn; shutdown() with a 20 s budget. Main: no FIN 1.54 s in (the test's bound), the drain still waiting. Fix: FIN at once.
• At the real test's shape (24 HTTP/2 conns) the failure is ~0.1% per run and I did not reproduce it naturally: 140 arm-runs of the unmodified test (docker golang:1.27.3, -race, --cpus 1 and 2, one and two parallel processes) all passed. So, at the test/server level I lowered the number of HTTP/2 conns (experiment only, not committed; env knob) to move the loop-without-H2 probability, with the same test, -race, --cpus 2, arms (sync + async-route) that had at least one conn without its FIN at 1.5 s:

HTTP/2 conns	main	this PR
1	8 of 16 arms	0 of 16
2	12 of 24	0 of 16
3	3 of 40	0 of 16
6	0 of 24	n/a

(The model P(download's loop has no HTTP/2 conn) = (3/4)^n overshoots the measured rate for n >= 3: not every conn on a loop-without-H2 is held; the rate falls with n as it predicts, and it is the unit test that is the proof, not the table.)

Commits

1. tests: a loop with no HTTP/2 conn holds the FIN of its idle HTTP/1 conns behind another conn's send drain (red on main)
2. epoll: shutdown closes the idle HTTP/1 conns of a loop before anything in it waits ...: Loop.shutdown() runs the gate's own closeIdleH1(false, now) first (after the adoption queue and the drivers, before phase 1), until a pass closes fewer than its cap (closeIdleH1 returns the count now). The conns are then in the state the gate's code was written for (nothing torn down yet: dispatch goroutines live, detached streams not ended; I first placed it after phase 2 and moved it: phase 1 has already run CloseH1 on every conn by then). Same rules as the gate's: a conn with a response queued or unacked, a request in progress, a dispatch goroutine running a handler, or a detached stream not ended is left to the drain and to phase 3. Conns with unread bytes are closed as phase 3 would close them (an RST either way; the gate's first-pass read-and-answer is not used here, the run loop is already gone). Same teardown as the gate's closeConn, so such a conn is now counted closed and fires OnDisconnect at shutdown (phase 3 closes with a bare close(2) and does neither): the same as the conns a loop with HTTP/2 conns closes at its gate. TestShutdownIdlePassTearsAConnThatServedARequestDownOnce1104 pins, for a conn that has served a request, one OnDisconnect, one closeCount, activeConns 0 and a FIN (sync, 20 of 20 under -race); its promoted sub-case models production (dispatch goroutine parked, asyncPromoted set) and pins what this PR does NOT do: the pass leaves the conn live and uncounted, phase 3 closes it, with nothing counted (epoll shutdown: an idle conn that is asyncPromoted is held behind another conn's send drain for the whole Shutdown budget #1165 turns this case around). An earlier version of that sub-case ran the goroutine without asyncPromoted and so showed a close production never makes; the review found it.
3. epoll: shutdown's idle pass has a bound of its own; its unit test models a promoted conn as production does ... (review r1 of this PR): the pass loop is bounded by len(liveConns)/gateClosesPerPass + 1 passes (closeIdleH1 counts closeConn calls, and closeConn can leave a conn in place when a dispatch goroutine owes the close, so the count alone does not prove termination).
4. tests: the shutdown gate test reads every idle conn's FIN at the same time and reports each one's own wait
5. tests: the shutdown gate test holds the wait with 64 HTTP/2 conns, so that no loop is left without one: the same test also fails, for the same reason and at the same rate, on its other assertions (upload cut, Async-route conn closed) when a loop has no HTTP/2 conn; 24 big + 40 small (512 KiB, still above a stream's initial window) conns make a loop without one 4 (3/4)^64 ~ 0 on 4 loops (0.3% on 16 loops).

Verification

-race, --cpus 2: all *1104 tests of internal/engine/epoll -count=20 ok (166 s), and the internal/engine/epoll whole package ok (379 s) with the pass at the top of shutdown(). The test at 64 HTTP/2 conns, -count=15: 30 of 30 arms pass, every idle conn has its FIN 251 to 262 ms in. go vet linux; mage apiCheck ok; golangci-lint v2.15 local (CI pins v2.14): 3 errcheck findings on case <-done: lines that are on main, none in this diff. io_uring tests cannot run in this docker (io_uring not available), so the io_uring side is CI's; it is untouched. A broader local -run 'Shutdown|Drain' pass of the root package and test/server with the io_uring cases skipped is in the evidence directory of the lane.

Not verified

The CI occurrence itself (which conn was on a loop without an HTTP/2 conn) cannot be observed after the fact; the mechanism is shown by the unit test and by the lowered-H2 runs, not by a replay of that run.

[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 75.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/engine/epoll/loop.go	66.66%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!