fix(iouring): hold a closed connection's send buffer until its SEND_ZC notification, past the release backstop (celeris#812) - #813
Draft
FumingPower3925 wants to merge 6 commits into
Draft
FumingPower3925 wants to merge 6 commits into
FumingPower3925 wants to merge 6 commits into
Conversation
FumingPower3925
added
bug
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueComment |
FumingPower3925
added a commit
that referenced
this pull request
Sep 28, 2026
…p only its array, show in gauges and stop at a cap (celeris#812) The round-2 review of #813 measured what the backstop hold costs while it lasts, and a hold lasts as long as the peer keeps its orphaned socket alive, which a peer that reads slowly can do for as long as it likes: - Every held entry stayed on pendingRelease, which the loop walks every pass while it is not empty, one closedOps lookup each: 94 us a pass at 10,000 held, and the request rate of live clients fell to 0.30. - A hold kept the whole connState (23.9 KB a held connection in the load probe, for a 7 KB response), although the kernel reads only the send buffer's array. - Nothing showed what was held now, and nothing stopped it growing. - A connection closed under the same (fd, generation) identity with no SEND_ZC of its own reached the backstop's release and dropped the shared identity, and the next pass pooled the SEND_ZC's connState with its array. New tests: TestZCHoldIsOffTheReleaseWalk, TestZCHoldKeepsOnlyTheSendBuffer, TestZCHoldCoversACollidingSibling, TestSendZCStopsWhileHeldBuffersReachTheCap, and BenchmarkZCHeldLoopPass / BenchmarkClosedIdentityRetireWithZCHolds. TestBackstopCountsItsZCHolds now also checks, on the wire fixture, that the held entry is off the walk, keeps the array alone, and shows in the gauges. TestPendingReleaseBackstopHoldsAZCSendPastItsDeadline and the shutdown test look for the hold where it now is. The metrics tests carry the two gauges. The names these tests use (Worker.zcHolds, zcHoldCount, zcHoldBytes, zcHold, zcHoldBytesMax, the zcHeldNow and zcHeldBytes gauges and the EngineMetrics fields CloseZCNotifHeldNow and CloseZCNotifHeldBytes) are declared here as stubs with no behaviour, so the tests compile and fail on this tree; the next commit gives them their behaviour.
FumingPower3925
added a commit
that referenced
this pull request
Sep 28, 2026
…e, in gauges, and stop arming SEND_ZC at 16 MiB held (celeris#812) The backstop's hold (the previous round) lasts until the SEND_ZC's notification, which comes when the peer takes the unsent tail or the kernel ends the orphaned socket. A peer that keeps reading, however slowly, keeps the socket alive, so the peer decides how long the hold lasts. The round-2 review of #813 measured what that cost: every held entry stayed on pendingRelease, which the loop walks every pass (94 us a pass at 10,000 held, live clients down to 0.30 of their request rate), each kept its whole connState (23.9 KB for a 7 KB response), nothing showed what was held, and nothing stopped it growing. - Off the walk. Past the backstop a held send buffer leaves pendingRelease and waits in Worker.zcHolds, keyed by its closed identity. Only a CQE of that identity reaches it: noteStaleTerminalOp settles the identity's holds (settleZCHolds), so a hold costs O(1) when it starts, at each of its identity's CQEs and when it ends, and nothing per loop pass. The stale-CQE path pays one length check, and one lookup while any hold exists. - The array alone. Once the SEND_ZC is all the identity owes, the connState is released as usual (releaseHeldConnState): to the pool without the array, or, detached, to the GC untouched. It leaves the identity first (its slot set to nil, so the conn count the collision rule reads is kept), so no later CQE writes into a connState the pool handed on. While another op is owed too (a recv the close's cancel has not ended yet), the whole entry is held, and handed back to the walk at that op's CQE. - Shown. CloseZCNotifHeldNow and CloseZCNotifHeldBytes are gauges of the arrays held right now and their capacity; a worker that shuts down takes its share out, and its arrays count in ShutdownZCBufRetained. - Bounded. A worker whose held arrays reach zcHoldBytesMax (16 MiB) arms no new SEND_ZC (prepSendSQE): its sends copy, and a copied send's unsent tail is the kernel's own socket memory, which its orphan and socket-memory limits bound. The held bytes can then grow only by the SEND_ZCs already in flight on live connections. - Decided on the identity. closedZCOwed no longer asks whether the connection's own send was a SEND_ZC: under an (fd, generation) collision a sibling with no SEND_ZC of its own reached the backstop's release, dropped the shared identity, and the next pass pooled the SEND_ZC's connState with its array. Now every conn under an identity that owes a SEND_ZC is held, and CloseZCNotifForced cannot move on this code; its docs now say it is a tripwire for a change that breaks the hold, not a measure of the kernel. The comments that said the kernel bounds the hold now say what does. noteStaleRecvExemplar (validation builds) skips a slot a hold emptied.
FumingPower3925
force-pushed
the
fix/celeris-812-zc-backstop
branch
from
September 28, 2026 09:51
ddf0786 to
da12b3f
Compare
…SEND_ZC notification still guards (celeris#812) A SEND_ZC to a peer that stopped reading keeps its notification owed for as long as the socket lives, and a close orphans the socket with the unsent tail still queued on the send buffer's pages. The pendingRelease backstop released such a connState 5 s after the close, to connStatePool with its sendBuf, or for a detached one to the GC, and when the peer read again the orphaned socket sent the tail from whatever the array held by then. The test builds the case with the #798 fixture (64 KiB SEND_ZC, a loopback peer with a 4 KiB receive buffer, the closing-drain sweep's close), makes the backstop due at once, keeps the peer stalled for 300 ms of loop passes, writes 0xAA over the array the moment the connState leaves pendingRelease (what its next owner does), and then lets the peer read to EOF: every byte must be the one sent, and the connState must be released once the notification arrives. Four cases: notification only, a recv and the notification, the send's first CQE unread at the close, and a detached connection. Fails on this tree: every case releases at the first pass after the backstop with the notification owed, and the peer reads the tail as 0xAA.
…C notification, past the release backstop (celeris#812) The pendingRelease backstop released a closed connState 5 s after the close whatever it still owed, treating an op owed that long as a kernel anomaly. A SEND_ZC's notification is not one: a peer that stops reading keeps the unsent tail of the send queued on the socket as references to cs.sendBuf's pages, and after the close the orphaned socket keeps offering it until the peer reads or the kernel gives up on the socket. The released connState went to connStatePool with its sendBuf (a detached one to the GC), its next owner wrote into the array, and the peer then received that tail from it: another connection's bytes. A closed identity's closedOps entry now counts its SEND_ZC apart (zcOwed, in the entry's padding): one per conn whose send in flight is a SEND_ZC at the close, taken off at that op's terminal CQE, its notification (or, for a single conn, a first CQE without F_MORE, which has none to follow). Past the backstop, drainPendingRelease holds an entry that still owes one until the notification retires it, and then releases it the usual way. The descriptor is not held with it: a notification names none and let it go already (#798); an op that does name it past the backstop is closed and counted as before (CloseFDForced). Worker shutdown closes the ring, which ends no SEND_ZC's use of its pages, and after which no notification can say when that use ends. Right before the ring closes, it reads what the ring holds and keeps every send buffer a SEND_ZC may still read, of a live or a held connection, in a process-lifetime list: the Worker is all that held them, and the GC could hand the arrays on once the engine is dropped. No wait is added: the run loop's 250 ms send drain already waits for the live connections' sends. New EngineMetrics (io_uring; summed by adaptive): CloseZCNotifHeld (holds, a rate), CloseZCNotifForced (an identity dropped with a SEND_ZC owed, i.e. a send buffer given up while the kernel may still send from it; must stay 0), and ShutdownZCBufRetained. Tests: the backstop's hold on hand-built state (it runs where SEND_ZC does not), the accounting on hand-built CQEs (each way a SEND_ZC ends, and a collision), the must-stay-0 counter's positive control, the counters on every wire case, and the shutdown retention.
…h a detector control The recv-theft job (both arches) runs the six celeris#812 tests by name at the runner's own 8 MiB memlock, -race, three runs, and requires every run of each of the twenty-one cases to PASS with no FAIL and no SKIP line (a SKIP would mean the runner gave the engine no working SEND_ZC). Its detector control applies .github/scripts/mutant-812-backstop-releases-zc.py, which disables the backstop hold, and requires every run of each of the four wire cases to report the release and foreign bytes on the wire, and to FAIL: a runner whose kernel copied the send's pages before the release would let the wire test pass with or without the fix.
…p only its array, show in gauges and stop at a cap (celeris#812) The round-2 review of #813 measured what the backstop hold costs while it lasts, and a hold lasts as long as the peer keeps its orphaned socket alive, which a peer that reads slowly can do for as long as it likes: - Every held entry stayed on pendingRelease, which the loop walks every pass while it is not empty, one closedOps lookup each: 94 us a pass at 10,000 held, and the request rate of live clients fell to 0.30. - A hold kept the whole connState (23.9 KB a held connection in the load probe, for a 7 KB response), although the kernel reads only the send buffer's array. - Nothing showed what was held now, and nothing stopped it growing. - A connection closed under the same (fd, generation) identity with no SEND_ZC of its own reached the backstop's release and dropped the shared identity, and the next pass pooled the SEND_ZC's connState with its array. New tests: TestZCHoldIsOffTheReleaseWalk, TestZCHoldKeepsOnlyTheSendBuffer, TestZCHoldCoversACollidingSibling, TestSendZCStopsWhileHeldBuffersReachTheCap, and BenchmarkZCHeldLoopPass / BenchmarkClosedIdentityRetireWithZCHolds. TestBackstopCountsItsZCHolds now also checks, on the wire fixture, that the held entry is off the walk, keeps the array alone, and shows in the gauges. TestPendingReleaseBackstopHoldsAZCSendPastItsDeadline and the shutdown test look for the hold where it now is. The metrics tests carry the two gauges. The names these tests use (Worker.zcHolds, zcHoldCount, zcHoldBytes, zcHold, zcHoldBytesMax, the zcHeldNow and zcHeldBytes gauges and the EngineMetrics fields CloseZCNotifHeldNow and CloseZCNotifHeldBytes) are declared here as stubs with no behaviour, so the tests compile and fail on this tree; the next commit gives them their behaviour.
…e, in gauges, and stop arming SEND_ZC at 16 MiB held (celeris#812) The backstop's hold (the previous round) lasts until the SEND_ZC's notification, which comes when the peer takes the unsent tail or the kernel ends the orphaned socket. A peer that keeps reading, however slowly, keeps the socket alive, so the peer decides how long the hold lasts. The round-2 review of #813 measured what that cost: every held entry stayed on pendingRelease, which the loop walks every pass (94 us a pass at 10,000 held, live clients down to 0.30 of their request rate), each kept its whole connState (23.9 KB for a 7 KB response), nothing showed what was held, and nothing stopped it growing. - Off the walk. Past the backstop a held send buffer leaves pendingRelease and waits in Worker.zcHolds, keyed by its closed identity. Only a CQE of that identity reaches it: noteStaleTerminalOp settles the identity's holds (settleZCHolds), so a hold costs O(1) when it starts, at each of its identity's CQEs and when it ends, and nothing per loop pass. The stale-CQE path pays one length check, and one lookup while any hold exists. - The array alone. Once the SEND_ZC is all the identity owes, the connState is released as usual (releaseHeldConnState): to the pool without the array, or, detached, to the GC untouched. It leaves the identity first (its slot set to nil, so the conn count the collision rule reads is kept), so no later CQE writes into a connState the pool handed on. While another op is owed too (a recv the close's cancel has not ended yet), the whole entry is held, and handed back to the walk at that op's CQE. - Shown. CloseZCNotifHeldNow and CloseZCNotifHeldBytes are gauges of the arrays held right now and their capacity; a worker that shuts down takes its share out, and its arrays count in ShutdownZCBufRetained. - Bounded. A worker whose held arrays reach zcHoldBytesMax (16 MiB) arms no new SEND_ZC (prepSendSQE): its sends copy, and a copied send's unsent tail is the kernel's own socket memory, which its orphan and socket-memory limits bound. The held bytes can then grow only by the SEND_ZCs already in flight on live connections. - Decided on the identity. closedZCOwed no longer asks whether the connection's own send was a SEND_ZC: under an (fd, generation) collision a sibling with no SEND_ZC of its own reached the backstop's release, dropped the shared identity, and the next pass pooled the SEND_ZC's connState with its array. Now every conn under an identity that owes a SEND_ZC is held, and CloseZCNotifForced cannot move on this code; its docs now say it is a tripwire for a change that breaks the hold, not a measure of the kernel. The comments that said the kernel bounds the hold now say what does. noteStaleRecvExemplar (validation builds) skips a slot a hold emptied.
…s, twenty-four cases) TestZCHoldIsOffTheReleaseWalk, TestZCHoldKeepsOnlyTheSendBuffer, TestZCHoldCoversACollidingSibling and TestSendZCStopsWhileHeldBuffersReachTheCap join the step's list: both arches, by name, at the runner's own 8 MiB memlock, -race, three runs, every case PASS and no FAIL or SKIP line. The detector control is unchanged: the hold it removes is still the one the wire test judges.
FumingPower3925
force-pushed
the
fix/celeris-812-zc-backstop
branch
from
September 28, 2026 09:53
da12b3f to
50efff7
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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
1 participant
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.
Fixes #812.
Stacked on #793. The base is
fix/celeris-685-close-recv-theft(#793's head,bf129c8). GitHub retargets this PR tomainwhen #793 merges, because the repo deletes merged branches. Until then, the CI workflow does not run on this PR:ci.ymltriggers only on pull requests tomain. So all the results below come from local runs, listed under Evidence.The defect
On io_uring, the
pendingReleasebackstop released a closed connection'sconnState5 s after the close, even while a SEND_ZC notification was still owed. A plain connection'sconnStatewent back toconnStatePoolwith itssendBuf. A detached one went to the GC.A SEND_ZC does not copy. When a peer stops reading, the unsent tail of the send stays queued on the socket as references to
sendBuf's pages. After the close, the orphaned socket keeps offering that tail until one of two things happens: the peer reads, or the kernel gives up on the socket. Only then does the kernel post the notification.Meanwhile the array had a new owner, and that owner wrote into it. When the peer read again, it received the tail from the array: another connection's bytes.
The fix
Accounting. A closed identity's
closedOpsEntrynow counts its SEND_ZC separately, inzcOwed. The field sits in the padding byte betweenhandoffandfdOps, so the entry is still 32 bytes.noteClosedInflightadds one for each conn whose send in flight is a SEND_ZC:sendIsZC && (sending || zcNotifPending).noteStaleTerminalOptakes it off at that op's terminal CQE, which is the notification.F_MOREhas no notification to follow, so it also ends the hold, but only when the identity holds one conn. Under an (fd, generation) collision, only a notification ends a hold. Holding late costs memory. Releasing early sends a peer another connection's bytes.The hold. Past the backstop,
drainPendingReleaseholds any entry that still owes a SEND_ZC (closedZCOwed→holdZCPastBackstop). This covers detached entries too. The entry stays until the notification retires the identity, and is then released the usual way.CloseFDForced).Shutdown. Closing the ring does not end a SEND_ZC's use of its pages, and once the ring is closed nothing can report when that use ends. The Worker was the last thing holding those connStates, so once the engine was dropped, the GC could hand the arrays on.
retainZCSendBufsAtShutdownruns just before the ring closes. Every send buffer a SEND_ZC may still read goes into a list that lives as long as the process, whether it belongs to a live connection (liveZCOwed) or to one queued for release (closedZCOwed).cs.sendingstays set until the notification, and a stalled peer's notification can take minutes.DEFER_TASKRUNring's task work, and it retires those CQEs. Nothing is submitted, becauseshutdownDrivershas already closed the driver descriptors.TestShutdownDoesNotWaitForAZCNotificationstill passes.endOwedOpsAtShutdownis unchanged.New
EngineMetricsfields. All three are io_uring only, and adaptive reports the sum over both sub-engines.CloseZCNotifHeld: entries the backstop held for a SEND_ZC. A rate.CloseZCNotifForced: must stay 0. Counted indropClosedOps, where the accounting lets go of an identity. A count there means an identity was dropped withzcOwed > 0, which is a send buffer given up while the kernel may still send from it. This is the must-stay-0 signal for this case, which fix(iouring): never release a descriptor number while an op can still resolve it: close paths, hijack, shutdown (celeris#685) #793 no longer counts underCloseFDForced.ShutdownZCBufRetained: send buffers kept for the life of the process at shutdown.CI (the
recv-theftjob, both arches):-race, 3 runs, and a tally. Skipping is forbidden, because a SKIP would mean the runner has no working SEND_ZC..github/scripts/mutant-812-backstop-releases-zc.py, which disables the hold. Every run of each of the four wire cases must then report the release and foreign bytes on the wire, and FAIL. This matters because a runner whose kernel copied the pages before the release would pass the wire test with or without the fix.Tests
TestBackstopHoldsASendBufferAZCNotificationStillReads(failing-first commit33bb077)releaseAtNanos; the production value is unchanged. The peer stays stalled for 300 ms of loop passes. The moment the array stops being held (the connState leavespendingRelease, or stays there without the array), the test writes0xAAover it, as its next owner would. Then the peer reads to EOF, and every byte must bebyte(i). Four cases:notif-only,recv-and-notif,send-done-after-close(the first CQE was unread at the close), anddetached-notif-only.TestBackstopCountsItsZCHoldsCloseZCNotifForced0,CloseFDForced0.TestPendingReleaseBackstopHoldsAZCSendPastItsDeadlineTestClosedOpsCountsTheZCSendApartzcOweddriven by hand-built CQEs: the notification, first CQE then notification, a first CQE withoutF_MORE, a recv's terminal CQE, a plain send, and a collision.TestDroppingAnIdentityWithAZCOwedIsCountedTestShutdownRetainsSendBuffersAZCMayStillReadEvidence
Every number below comes from the scripts and logs in the maintainer's evidence directory,
celeris-zc-backstop/fix/:scripts/chain2.shandchain3.sh: the runs.scripts/zcfix-run.sh: one container per run,golang:1.27, linuxkit 7.0.12 aarch64,--security-opt seccomp=unconfined, through the laptop slot lock. Shapes:m8is--cpus 4with memlock 8 MiB (CI's unit job);ciis--cpus 4with memlock unlimited.scripts/zcfix-cistep.sh: runs a step of the tree's ownci.yml, verbatim.scripts/zcfix-tally.py: tallies every--- PASS/FAIL/SKIPline and every result line.scripts/buildcheck.sh: the build, vet and lint checks.scripts/mktree.sh: builds each tree;README.txtmaps the trees to commits.All runs use
-race. There were 0 data races and 0 panics in any log.The fix against controls and mutants (
TestBackstopHoldsASendBufferAZCNotificationStillReadsunless stated):33bb077(#793 head + the test)---lines FAILddf0786(held, forced, fd_forced) = (1, 0, 0). Shutdown cases retained 1 / 0 / 1 as expected.bf129c8(0, 1, 0): the must-stay-0CloseZCNotifForcedfires.zcOwedtaken off at the SEND_ZC's first CQE instead of its notificationsend-done-after-closeFAILs in both tests;TestClosedOpsCountsTheZCSendApartFAILs(1, 0, 0): only the wire test sees this one.zcfix-cistep.sh, runner shape m8):want 18 PASS (63 cases), passed 18 (63 cases), FAIL lines 0, SKIP lines 0.result lines 3, detected 3, FAIL 3, and exits 0.want 6 PASS (15 cases), passed 6 (15 cases), with 0 FAIL and 0 SKIP.rv3/zz_rv3_wire_test.go, unmodified) uses the real 5 s backstop and a peer stalled for 5.5 s.33bb077, both backstop cases release at 5.003 s and 5.010 s with 1 op owed, and 61,200 bytes are corrupt. The three 300 ms cases have 0 corrupt.ddf0786, 2 runs, every case PASSes. The backstop cases release at 5.752–5.765 s with 0 owed, after the peer read. 0 corrupt.-tags=validation, ci × 3, the nineTestRecvTheft*trials) give 27/27 PASS.reused=false stolen=false answered=true.hijacker_read=true.answered=240 lost=0../engine/iouring/at memlock unlimited: 383 PASS, 0 FAIL, 2 SKIP (the twoSynackRetriesZerotests, which CI enforces in another step)../engine/iouring/at m8: 380 PASS, 0 FAIL, 5 SKIP. The 5 skips are exactly the ones on CI's allowed list for that step.f2-fix-iouring-m8-race) is void: 184 SKIPs, because everyio_uring_setupreturned ENOMEM. RLIMIT_MEMLOCK is charged per uid, every container here runs as root, and a concurrent lane's container had used up the 8 MiB.chain3.shreran it and accepts a run only with 5 SKIP lines or fewer../adaptive/, ci shape withCELERIS_REQUIRE_UPSWITCH=1and CI's skip ofTestRampH1Sync/TestRampH1Async: 113 PASS, 0 FAIL, 0 SKIP.probes/zz_zc812_orphan_test.go, evidence only; one m8 run onddf0786):tcp_orphan_retries=0,tcp_retries2=15.connection reset by peer.buildcheck.sh), all onddf0786:go build ./...,go vet ./engine/... ./adaptive/...,go vet -tags=validation, and test compiles ofengine/iouring(plain and validation),adaptiveandengine. All rc=0.Limits
ubuntu-latestandubuntu-24.04-armonce this PR targetsmain.CloseZCNotifForcedsees only what the accounting believes. M2 and M3 corrupt the wire without moving it. The wire test and its CI detector control are what judge the hold itself.staleConnCQE) now leaves that connState held for good instead of released at the backstop. That is one connState of memory, on the safe side.