Repository navigation
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
Conversation
…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)
…e gate (mutant; Refs #1165)
… 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)
…t its FIN, as the others do (Refs #1165)
…ery idle conn has had its FIN, so the gate's pass has run (Refs #1166)
|
Caution Review failedThe pull request is closed.
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. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
FumingPower3925
marked this pull request as ready for review
October 29, 2026 01:36
FumingPower3925
added a commit
that referenced
this pull request
Oct 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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).
closeIdleH1skipped everyasyncPromotedconn. 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), thendetachMuis try-locked and the parser tested under it (HTTP/1 still, not detached, noHasPendingData, at a request boundary, so also no h2c upgrade pending); the conn is marked close-after;closeConndoes the rest (the parked goroutine exits onasyncClosed).#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:
poolWaitSettled=h2PoolSettledand 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 bysendDrainWaitfromh2DrainStart, paced ath2PoolDrainPollMs, and runs the same gate meanwhile.refusesRecvreads on a promoted conn while a request is in progress on it (io_uring'sgateReadsOndoes).h1RequestInProgress, bounded bydrainEnd) for an in-progress HTTP/1 request, with the same gate meanwhile.Red / green
-race(TestShutdownIdlePassTearsAConnThatServedARequestDownOnce1104/promoted,TestGateClosesAPromotedConnOnlyWhenItIsIdle1165parked-idle and exited-idle)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 withConnection: closeand then a FIN-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,detachMulock ignored,HasPendingDataignored,AtRequestBoundaryignored (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,endedalways true, first-pass hold of an unread request removed, promoted conns skipped again: 10 of 10 killed.#1166 rules:
refusesRecvrefuses 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 ignoresh1InProgress, h2 conn counted, pacing removed: 11 of 11 killed in the unit tests; the 12th, the run loop callingh2PoolSettledinstead ofpoolWaitSettled, 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)
TestShutdownPoolWaitGateRequestsInProgressAndIdleConns885pinned "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, dockergolang:1.27.3,--cpus 2, one container at a time: the wholeinternal/engine/epollpackage ok (393 s on the stacked tree; the gate and shutdown tests again ok on the rebased tree);TestShutdownPoolWaitGate*5 of 5;go vetlinux;mage apiCheckok (no exported API change).Review fixes after the first CI run
a8a6df9: CI's-racejobs failedTestStalledSSEEndsWithShutdown855/epoll/h1/{tick,tight}:h1InProgressread a detached SSE stream's write state (csWritePending, written by the stream's writer goroutine underdetachMu) from the loop thread on the shutdown's first call. TheDetachedtest now comes before the queue test. Regression testTestH1InProgressDoesNotReadADetachedStreamsWriteState1166(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, insidepromotedRequestInProgresswhile 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. TestTestH1InProgressAsksTheKernelAboutAPromotedConn1166: 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 /refusesRecvasking after the gate) all killed. The mutant that drops the "stream has taken over" check (Detached) inpromotedRequestInProgress, 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
h1RequestInProgressdoes not see a request still in the kernel queue or an unreaped recv CQE when there are no sends pending.Connection: close, and the conn is then closed (review probe P3: stdclose=true, epollclose=false).H1State.CloseAfteris 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.detachMucounts as "in progress" for up to the gate's 250 ms (until it ends the producers); a wait that ends atdrainEndis recorded as "send drain gave up" in the stats.OnDisconnectis 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.Shutdownnow waits (up to its budget) for such a request, where it used to cut it.