Skip to content

epoll, io_uring: the shutdown gate closes idle conns that have a dispatch goroutine and answers an HTTP/1 request in progress on loops without HTTP/2 (v1.6.0, Refs #1165 #1166) - #1168

Merged
FumingPower3925 merged 12 commits into
mainfrom
gate-promoted-inprogress-1165-1166
Oct 29, 2026

Conversation

@FumingPower3925

@FumingPower3925 FumingPower3925 commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Addresses the two release-level gaps the review of #1163 found in the shutdown gate. Refs #1165, Refs #1166, Refs #885, Refs #884

Root causes

#1165 (epoll). closeIdleH1 skipped every asyncPromoted conn. 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. So such an idle keep-alive conn was held for the whole Shutdown budget behind another conn's send drain on a loop with no HTTP/2 conn (#1163 fixed that for the other conns), and for the whole pool wait on one with. The fix gives epoll io_uring's look inside the conn (promotedGateState): closeable only when the dispatch goroutine has no input waiting and is not inside a handler (asyncInMu), then detachMu is try-locked and the parser tested under it (HTTP/1 still, not detached, no HasPendingData, at a request boundary, so also no h2c upgrade pending); the conn is marked close-after; closeConn does the rest (the parked goroutine exits on asyncClosed).

#1166 (epoll, io_uring). The gate's "read on and answer the request in progress" ran only on a loop that had an HTTP/2 conn to wait for. On any other loop a request in progress at Shutdown (slow upload, headers in pieces, a body arriving) was never read and was cut with its conn. And on epoll the gate refused every read of a promoted conn, so an upload on a server with any Async route (which promotes the conn) was never read after the gate even on a loop that waited. Fixes:

  • epoll: poolWaitSettled = h2PoolSettled and no HTTP/1 request in progress (h1InProgress: parser state, undecided conn with bytes, promoted conn with input or a handler, and on the first call bytes in the kernel's receive queue of a conn that holds nothing). The wait is bounded by sendDrainWait from h2DrainStart, paced at h2PoolDrainPollMs, and runs the same gate meanwhile. refusesRecv reads on a promoted conn while a request is in progress on it (io_uring's gateReadsOn does).
  • io_uring: a worker with no sends pending used to shut down at once; it now waits (h1RequestInProgress, bounded by drainEnd) for an in-progress HTTP/1 request, with the same gate meanwhile.

Red / green

test before after
epoll unit, promoted parked conn closed by the pass, -race (TestShutdownIdlePassTearsAConnThatServedARequestDownOnce1104/promoted, TestGateClosesAPromotedConnOnlyWhenItIsIdle1165 parked-idle and exited-idle) red 3 of 3 green 20 of 20
public API e2e TestShutdownIdleAndInProgressH1ConnsWithoutHTTP2_1165: HTTP/1-only server, Workers 2, 4 sync + 4 Async-route + 4 split-head idle conns, optional 6 slow 16 MiB downloaders, Shutdown(20 s); every idle conn must get its FIN within 1.5 s and a slow 2 MiB upload in progress must get its response with Connection: close and then a FIN epoll with only the #1165 fix: upload not answered or cut, 3 of 3 runs, both variants (the FIN part is green) epoll and std, both variants: 30 of 30 each (rebased tree, -race, --cpus 2)

The e2e test first failed once in 5 with "no Connection: close" on the upload response: the response was written before the gate's pass had marked the conn (a request in flight when the shutdown began is a keep-alive answer). The test now sends the rest of the upload after every idle conn has had its FIN, which is after the gate's pass; that is the last commit.

Mutants (each rule has a test that fails without it)

#1165 rules (unit, -race): input waiting ignored, handler-running ignored, detachMu lock ignored, HasPendingData ignored, AtRequestBoundary ignored (killed by the h2c-upgrade-pending case, added after the first run left it alive), switched-to-h2 ignored, close-after not set, detached-stream test removed, ended always true, first-pass hold of an unread request removed, promoted conns skipped again: 10 of 10 killed.

#1166 rules: refusesRecv refuses promoted always, promoted not counted, plain pending data ignored, undecided conn ignored, kernel question not asked / asked on every call, promoted busy ignored, detached counted, wait ignores h1InProgress, h2 conn counted, pacing removed: 11 of 11 killed in the unit tests; the 12th, the run loop calling h2PoolSettled instead of poolWaitSettled, is killed by the e2e test (the red above). Logs are in the lane's evidence directory (partB-mutants*.log).

Changed expectation in an existing test (#885)

TestShutdownPoolWaitGateRequestsInProgressAndIdleConns885 pinned "an idle conn on an Async route (promoted) is not closed by the gate: it is left to the end of the wait". That was the known gap (#1165). It now expects that conn to get its FIN with the others (read together with them), and its doc comment says so.

Verification

-race, docker golang:1.27.3, --cpus 2, one container at a time: the whole internal/engine/epoll package ok (393 s on the stacked tree; the gate and shutdown tests again ok on the rebased tree); TestShutdownPoolWaitGate* 5 of 5; go vet linux; mage apiCheck ok (no exported API change).

Review fixes after the first CI run

  • a8a6df9: CI's -race jobs failed TestStalledSSEEndsWithShutdown855/epoll/h1/{tick,tight}: h1InProgress read a detached SSE stream's write state (csWritePending, written by the stream's writer goroutine under detachMu) from the loop thread on the shutdown's first call. The Detached test now comes before the queue test. Regression test TestH1InProgressDoesNotReadADetachedStreamsWriteState1166 (red under -race, green 20 of 20); the CI test at -count=20 -race: 2 races in 80 subtests before, 0 in 80 after. The same commit fixes the lint finding (QF1001, same line) and a test-setup flake of the e2e test (a conn still in the accept queue when the shutdown began was reset by the listener's close; the test now waits until the engine counts every conn: 1 of 30 before, 160 of 160 after).
  • a04e32e (review r2, MINOR-2): the first look also asks the kernel's receive queue of an idle PROMOTED conn, inside promotedRequestInProgress while it holds the conn's lock with the goroutine parked and nothing queued (a bare read of the plain conn's line would read state the dispatch goroutine owns and bring the race back). Before: a request that reached the kernel before the cancel on an idle promoted conn was closed unread by phase 0 while a plain conn's was waited for and answered. Test TestH1InProgressAsksTheKernelAboutAPromotedConn1166: red 3 of 3 (-race), green 20 of 20; mutants (question dropped / asked with a response queued / asked on every call / the caller passing false / refusesRecv asking after the gate) all killed. The mutant that drops the "stream has taken over" check (Detached) in promotedRequestInProgress, which survived the first round, is now killed by two table cases (a promoted detached conn with bytes in its parser is not a request in progress and is refused after the gate).

Not verified / known limits

  • io_uring is untested locally (no io_uring in my docker). In CI the new e2e cell passed for io_uring in the tier=base job (two workers) and at tier=high, but the tier=high slow-downloads cell ran with ONE worker (memlock cap) and so never reaches the code that needs two loops; adaptive ran on epoll in both. h1RequestInProgress does not see a request still in the kernel queue or an unreaped recv CQE when there are no sends pending.
  • An Async handler running when the shutdown begins is answered without Connection: close, and the conn is then closed (review probe P3: std close=true, epoll close=false). H1State.CloseAfter is read once at the start of a request, and the gate marks a conn busy at every pass only when it next meets it idle. That is a keep-alive answer on a conn that then gets a FIN; a client that already wrote its next request loses it. Not a regression (before this PR such a conn was cut), and it belongs to the "Connection: close after Shutdown" change (epoll, io_uring: every HTTP/1 response after Shutdown carries Connection: close and ends its conn (v1.6.0, Refs #885 #884) #1169), which will decide it from a shutdown flag the response writer checks.
  • Shutdown now waits, up to its budget (measured by the review: 2.0 s with a 2 s budget), for an HTTP/1 request in progress on a loop with no HTTP/2 conn, as it waits for HTTP/2 pool handlers; before, such a request was cut at once. io_uring waits the same way when it has no sends pending (untested locally). A client that never finishes its request holds the shutdown for that budget, as on std.
  • On io_uring a stream's guarded write that holds detachMu counts as "in progress" for up to the gate's 250 ms (until it ends the producers); a wait that ends at drainEnd is recorded as "send drain gave up" in the stats.
  • OnDisconnect is still not uniform at shutdown (phase 3 fires none); unchanged.

Release note

On epoll, an idle keep-alive conn that ever served an Async route (or whose request arrived in pieces on a server with an Async route) gets its FIN at shutdown instead of being held for the Shutdown budget; a request in progress (slow upload) is answered, with Connection: close, on loops that hold no HTTP/2 conn; on io_uring, an in-progress HTTP/1 request is answered before the worker shuts down. Shutdown now waits (up to its budget) for such a request, where it used to cut it.

…e behind another conn's send drain (red; Refs #1165)
…e, as io_uring's does, and closes it when it is idle (v1.6.0, Refs #1165)
…N and answer a request in progress, on every engine (red on epoll: the upload; Refs #1165, Refs #1166)
… request in progress on a loop with no HTTP/2 conn, and the gate reads on a promoted conn that has one (v1.6.0, Refs #1166)
…the wait; the h2 conn is not one (mutants; Refs #1166)
…in progress before it shuts down (v1.6.0, Refs #1166)
…ery idle conn has had its FIN, so the gate's pass has run (Refs #1166)
@FumingPower3925 FumingPower3925 added this to the v1.6.0 milestone Oct 28, 2026
@coderabbitai

coderabbitai Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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

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

…detached stream's write state (a data race with its writer); the e2e test waits for the engine to accept every conn (Refs #1165, Refs #1166)
@codecov

codecov Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.89189% with 9 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/engine/iouring/shutdown_gate_884.go 65.00% 7 Missing ⚠️
internal/engine/epoll/poolwait_gate_885.go 97.75% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@FumingPower3925
FumingPower3925 marked this pull request as ready for review October 29, 2026 01:36
@FumingPower3925
FumingPower3925 merged commit 92eb215 into main Oct 29, 2026
30 checks passed
@FumingPower3925
FumingPower3925 deleted the gate-promoted-inprogress-1165-1166 branch October 29, 2026 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant