diff --git a/.github/scripts/mutant-685-release-owed-fd.py b/.github/scripts/mutant-685-release-owed-fd.py new file mode 100755 index 00000000..1816cc9c --- /dev/null +++ b/.github/scripts/mutant-685-release-owed-fd.py @@ -0,0 +1,53 @@ +#!/usr/bin/env python3 +"""celeris#685 detector control: the MUTANT, applied in CI and never committed. + +Makes fdOwed (engine/iouring/fd_lifetime.go) report false for every +connection, which takes the fd-lifetime rule off every path it guards at +once: the close paths close the descriptor at once again (no kept number, +no read-side shutdown), hijackConn no longer submits its cancels before +handing the socket over, and worker shutdown no longer ends the owed ops +before closing descriptors. + +Against the mutated tree every run of the five trials that judge the rule +MUST detect the theft, as its result line reports it (the CI step reads +that line, not the --- line, so a failure for another reason does not +count): TestRecvTheft715ArmA and its hole twin TestRecvTheft715ArmAHole (a +recv still in the SQ ring at the close), TestRecvTheft685Linked and +TestRecvTheft685LinkedHole (a recv linked behind a SEND, not issued at the +close), each with hit=true reused=true stolen=true, and +TestRecvTheft685HijackMultishotCoop (a multishot recv owed at a Hijack on a +ring without DEFER_TASKRUN) with hijacker_read=false. If one does not, it +is not watching the path it claims to judge, and its green run on the tree +as committed proves nothing. + +The mutant also takes the rule off worker shutdown (endOwedOpsAtShutdown +ends only the ops fdOwed reports), but no trial judges that half: it is +covered by construction, not by a detector. + +The function is matched EXACTLY; any drift makes this script exit 2 instead +of silently mutating nothing. +""" +import sys + +PATH = sys.argv[1] if len(sys.argv) > 1 else "engine/iouring/fd_lifetime.go" + +BODY = ( + "func fdOwed(cs *connState) bool {\n" + "\treturn cs != nil && fdOps(cs) > 0 && !cs.fixedFile\n" + "}\n" +) + +src = open(PATH).read() +if src.count(BODY) != 1: + print(f"mutant-685: expected exactly one fdOwed body, found {src.count(BODY)}", file=sys.stderr) + sys.exit(2) +mutated = src.replace( + BODY, + "func fdOwed(cs *connState) bool {\n" + "\t// MUTANT celeris#685: the fd-lifetime rule is off everywhere\n" + "\t_ = cs\n" + "\treturn false\n" + "}\n", +) +open(PATH, "w").write(mutated) +print(f"mutant-685: fdOwed always false ({PATH})") diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a3e648f3..328454fb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1008,6 +1008,170 @@ jobs: tally mutant kill || ok=1 exit "$ok" + recv-theft: + name: io_uring close paths never release a descriptor an op can still resolve (celeris#685, ${{ matrix.os }}) + # Both arches: the theft was measured on both (celeris#715). + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, ubuntu-24.04-arm] + runs-on: ${{ matrix.os }} + timeout-minutes: 20 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version: "1.27.0" + # The celeris#715 / celeris#685 trials (-tags=validation only; the + # holds are internal/recvtheft). Each parks the worker that closed a + # connection A with a recv still able to resolve A's descriptor number + # (TestRecvTheft715ArmA: a recv SQE not yet submitted; + # TestRecvTheft685Linked: a recv linked behind a SEND, not yet + # issued), lets the sibling worker accept a fresh connection B, and + # asserts that B is answered and that no stale recv read B's bytes. + # On a tree that frees the number at the close, the sibling is given + # it and the old recv reads B's request: both fail in every trial. + # Their *Hole twins open a free number under A's once the closer is + # parked: the sibling's accept of the hole tests nothing and is + # skipped, so they must fail the same way (a verdict that counted it + # passed a tree without the fix). TestRecvTheft715Control (the ring + # submitted before the close) and TestRecvTheft715ArmC (celeris#715 + # hypothesis (c)) must pass either way. + # The three TestRecvTheft685Hijack* arms hold the worker right after a + # Hijack and send the hijacker's first bytes meanwhile: with multishot + # recv (owed across its request) on a ring without DEFER_TASKRUN, the + # recv reads them unless hijackConn submits its cancel first (Coop); + # the DEFER_TASKRUN and single-shot arms pass either way. + # + # The trials need TWO io_uring workers (12 MiB of RLIMIT_MEMLOCK each; + # the runner defaults to 8 MiB), so memlock is raised as in the + # `iouring` job, and CELERIS_REQUIRE_IOURING_WORKERS=1 turns a missing + # sibling into a failure. -v, -count=5, and a tally that requires + # every run of every test to PASS, with no FAIL and no SKIP line: a + # renamed test selects nothing and go test would still exit 0. + - name: celeris#685 recv-theft trials (two workers, skipping forbidden) + shell: bash + env: + CELERIS_REQUIRE_IOURING_WORKERS: "1" + CELERIS_RECV_THEFT_715: "1" + run: | + set -o pipefail + sudo prlimit --pid "$$" --memlock=unlimited:unlimited + echo "memlock (KiB): $(ulimit -l)" + names='TestRecvTheft715ArmA|TestRecvTheft715ArmAHole|TestRecvTheft715Control|TestRecvTheft715ArmC|TestRecvTheft685Linked|TestRecvTheft685LinkedHole|TestRecvTheft685HijackSingleShot|TestRecvTheft685HijackMultishotDefer|TestRecvTheft685HijackMultishotCoop' + runs=5 + want=$(( $(tr '|' '\n' <<<"$names" | wc -l) * runs )) + go test -tags=validation -race -count="$runs" -timeout=600s -v -run "^(${names})\$" \ + ./engine/iouring/ 2>&1 | tee /tmp/recvtheft685.log || true + grep -E 'RECVTHEFT(715|685) .*result' /tmp/recvtheft685.log || true + passed=$(grep -cE "^--- PASS: (${names}) \(" /tmp/recvtheft685.log || true) + failed=$(grep -cE '^[[:space:]]*--- FAIL' /tmp/recvtheft685.log || true) + skipped=$(grep -cE '^[[:space:]]*--- SKIP' /tmp/recvtheft685.log || true) + for n in ${names//|/ }; do + echo "$n: PASS $(grep -cE "^--- PASS: $n \(" /tmp/recvtheft685.log || true) FAIL $(grep -cE "^--- FAIL: $n \(" /tmp/recvtheft685.log || true)" + done + echo "celeris#685 recv-theft trials: want $want PASS, passed $passed, FAIL lines $failed, SKIP lines $skipped" + if [ "$passed" -ne "$want" ] || [ "$failed" -ne 0 ] || [ "$skipped" -ne 0 ]; then + echo "expected exactly $want trial runs to PASS with no FAIL or SKIP line -- did a close path" + echo "release a descriptor number a recv could still resolve, or was a test renamed or skipped?" + exit 1 + fi + # celeris#798: the rule counts only the ops that name the descriptor. A + # SEND_ZC's notification names none, yet a peer that stops reading holds + # it for as long as the socket is open. So it must not keep a closed + # connection's descriptor (the release backstop forced it after 5 s and + # counted CloseFDForced), nor stall worker shutdown's drain, while the + # connState, whose send buffer the notification guards, still waits for + # it. The unit job runs these two tests in its package step, on x86 only; + # here they run on both arches, by name, at the runner's own 8 MiB + # memlock (the pages SEND_ZC pins count against it), with a tally: every + # run of each of the five cases must PASS, with no FAIL and no SKIP line. + # A SKIP would mean the runner's kernel gave the engine no working + # SEND_ZC, so nothing was tested. + - name: celeris#798 a SEND_ZC notification holds no descriptor (both arches, skipping forbidden) + if: ${{ !cancelled() }} + shell: bash + env: + CELERIS_REQUIRE_IOURING_WORKERS: "1" + run: | + set -o pipefail + echo "memlock (KiB): $(ulimit -l)" + names='TestCloseReleasesDescriptorWithOnlyAZCNotificationOwed|TestShutdownDoesNotWaitForAZCNotification' + runs=3 + want=$(( $(tr '|' '\n' <<<"$names" | wc -l) * runs )) + wantsub=$(( 5 * runs )) + go test -race -count="$runs" -timeout=300s -v -run "^(${names})\$" \ + ./engine/iouring/ 2>&1 | tee /tmp/zc798.log || true + grep -E 'celeris798 ' /tmp/zc798.log || true + passed=$(grep -cE "^--- PASS: (${names}) \(" /tmp/zc798.log || true) + subpassed=$(grep -cE "^ --- PASS: (${names})/" /tmp/zc798.log || true) + failed=$(grep -cE '^[[:space:]]*--- FAIL' /tmp/zc798.log || true) + skipped=$(grep -cE '^[[:space:]]*--- SKIP' /tmp/zc798.log || true) + echo "celeris#798 SEND_ZC tests: want $want PASS ($wantsub cases), passed $passed ($subpassed cases), FAIL lines $failed, SKIP lines $skipped" + if [ "$passed" -ne "$want" ] || [ "$subpassed" -ne "$wantsub" ] || [ "$failed" -ne 0 ] || [ "$skipped" -ne 0 ]; then + echo "expected exactly $want runs ($wantsub cases) to PASS with no FAIL or SKIP line -- did a" + echo "SEND_ZC notification hold a descriptor or stall shutdown, or was a test renamed or skipped?" + exit 1 + fi + # The detector control. .github/scripts/mutant-685-release-owed-fd.py + # makes fdOwed report false, which takes the fd-lifetime rule off the + # close paths, Hijack and shutdown at once. Against that tree every run + # of the five trials that judge the rule must DETECT the theft, read + # from its result line, not from its --- line: a FAIL for any other + # reason (INCONCLUSIVE, a setup error, a race report) is not a + # detection. The close-path trials (ArmA, ArmAHole, Linked, LinkedHole) + # must report hit=true, reused=true and stolen=true: the sibling was + # given A's number and A's stale recv read B's request. The hijack + # trial (MultishotCoop) must report op_owed=+1 and hijacker_read=false. + # The trials that pass on any tree must still PASS. No trial judges the + # shutdown half of the rule (see the script). The mutation is undone + # before the step ends. + - name: celeris#685 detector control (the rule removed; every run of the five judging trials must detect the theft) + if: ${{ !cancelled() }} + shell: bash + env: + CELERIS_REQUIRE_IOURING_WORKERS: "1" + CELERIS_RECV_THEFT_715: "1" + run: | + set -o pipefail + sudo prlimit --pid "$$" --memlock=unlimited:unlimited + python3 .github/scripts/mutant-685-release-owed-fd.py + others='TestRecvTheft715Control|TestRecvTheft685HijackSingleShot|TestRecvTheft685HijackMultishotDefer' + runs=3 + log=/tmp/recvtheft685-mutant.log + go test -tags=validation -race -count="$runs" -timeout=600s -v \ + -run "^(TestRecvTheft715ArmA|TestRecvTheft715ArmAHole|TestRecvTheft685Linked|TestRecvTheft685LinkedHole|TestRecvTheft685HijackMultishotCoop|${others})\$" \ + ./engine/iouring/ 2>&1 | tee "$log" || true + git checkout -- engine/iouring/fd_lifetime.go + grep -E 'RECVTHEFT(715|685) .*result' "$log" || true + bad=0 + # judged + judged() { + local n=$1 line=$2 want=$3 lines det p + lines=$(grep -cE "$line" "$log" || true) + det=$(grep -E "$line" "$log" | grep -cE "$want" || true) + p=$(grep -cE "^--- PASS: $n \(" "$log" || true) + echo "mutant, judged $n: result lines $lines, detected $det, PASS $p (want $runs, $runs, 0)" + if [ "$lines" -ne "$runs" ] || [ "$det" -ne "$runs" ] || [ "$p" -ne 0 ]; then bad=1; fi + } + stolen=' hit=true .* reused=true .* stolen=true ' + judged TestRecvTheft715ArmA 'RECVTHEFT715 arm=A result ' "$stolen" + judged TestRecvTheft715ArmAHole 'RECVTHEFT715 arm=A-hole result ' "$stolen" + judged TestRecvTheft685Linked 'RECVTHEFT685 linked result ' "$stolen" + judged TestRecvTheft685LinkedHole 'RECVTHEFT685 linked-hole result ' "$stolen" + judged TestRecvTheft685HijackMultishotCoop 'RECVTHEFT685 hijack result tier=2 ' ' op_owed=\+1 .* hijacker_read=false ' + for n in ${others//|/ }; do + p=$(grep -cE "^--- PASS: $n \(" "$log" || true) + f=$(grep -cE "^--- FAIL: $n \(" "$log" || true) + echo "mutant, control $n: PASS $p FAIL $f (want PASS $runs)" + if [ "$p" -ne "$runs" ] || [ "$f" -ne 0 ]; then bad=1; fi + done + skipped=$(grep -cE '^[[:space:]]*--- SKIP' "$log" || true) + if [ "$skipped" -ne 0 ]; then echo "a SKIP is not a result ($skipped SKIP lines)"; bad=1; fi + exit "$bad" + conformance: name: Conformance runs-on: ubuntu-latest diff --git a/adaptive/engine.go b/adaptive/engine.go index d4acf0e0..b8d429fd 100644 --- a/adaptive/engine.go +++ b/adaptive/engine.go @@ -1158,6 +1158,11 @@ func (e *Engine) Metrics() engine.EngineMetrics { TransplantClaimDeferred: pm.TransplantClaimDeferred + sm.TransplantClaimDeferred, TransplantReapFailed: pm.TransplantReapFailed + sm.TransplantReapFailed, TransplantReapUnsupported: pm.TransplantReapUnsupported + sm.TransplantReapUnsupported, + // The same rule on the close paths (celeris#685): each close is an + // event on the one sub-engine that owned the connection, and a + // close of the standby's residue lands on the standby. + CloseFDDeferred: pm.CloseFDDeferred + sm.CloseFDDeferred, + CloseFDForced: pm.CloseFDForced + sm.CloseFDForced, // The post-switch sweep (celeris#657 PR-3). Both sub-engines sweep, // in opposite directions, and only the one draining runs passes at // all, so the pass count sums as a rate. The residual entries are diff --git a/adaptive/handoff_loss_metrics_test.go b/adaptive/handoff_loss_metrics_test.go index d5f2dc7a..1b256121 100644 --- a/adaptive/handoff_loss_metrics_test.go +++ b/adaptive/handoff_loss_metrics_test.go @@ -25,6 +25,7 @@ func TestMetricsSumsTheHandoffLossWitnesses(t *testing.T) { TransplantHeld: 5, TransplantReaps: 6, TransplantReapMisses: 7, TransplantHoldRescued: 8, TransplantDoubleClaim: 9, TransplantClaimDeferred: 12, TransplantReapFailed: 13, TransplantReapUnsupported: 14, + CloseFDDeferred: 15, CloseFDForced: 16, }) e.secondary.(*mockEngine).SetMetrics(engine.EngineMetrics{ StaleRecvDataClosed: 10, StaleRecvDataTransplanted: 20, @@ -32,6 +33,7 @@ func TestMetricsSumsTheHandoffLossWitnesses(t *testing.T) { TransplantHeld: 50, TransplantReaps: 60, TransplantReapMisses: 70, TransplantHoldRescued: 80, TransplantDoubleClaim: 90, TransplantClaimDeferred: 120, TransplantReapFailed: 130, TransplantReapUnsupported: 140, + CloseFDDeferred: 150, CloseFDForced: 160, }) m := e.Metrics() @@ -51,6 +53,8 @@ func TestMetricsSumsTheHandoffLossWitnesses(t *testing.T) { {"TransplantClaimDeferred", m.TransplantClaimDeferred, 132}, {"TransplantReapFailed", m.TransplantReapFailed, 143}, {"TransplantReapUnsupported", m.TransplantReapUnsupported, 154}, + {"CloseFDDeferred", m.CloseFDDeferred, 165}, + {"CloseFDForced", m.CloseFDForced, 176}, } { if c.got != c.want { t.Errorf("Metrics().%s = %d, want %d (sum of both sub-engines)", diff --git a/engine/engine.go b/engine/engine.go index f84646e9..166066c5 100644 --- a/engine/engine.go +++ b/engine/engine.go @@ -469,15 +469,15 @@ type EngineMetrics struct { //nolint:revive // user-approved name // - Closed: a connection this engine closed or hijacked (the close // paths and Hijack register the identity the same way). Usually // the client's bytes raced a server-side close, and that client - // sees its connection end. It is not always benign, so a non-zero - // Closed does not prove that no live client lost a request. After - // a Hijack the socket lives on under the hijacker's net.Conn, so a - // recv that completes with data before its cancel lands has taken - // the first bytes of a connection its client is still using. And a - // recv that had not reached the kernel when the descriptor was - // closed resolves the descriptor NUMBER when it does; if a new - // connection holds that number by then, the recv reads that - // client's request. + // sees its connection end. It was not always benign: after a + // Hijack, a recv that completed with data before its cancel landed + // took the first bytes of a connection its client was still using, + // and a recv that had not been issued when the descriptor was + // closed resolved the descriptor NUMBER when it was, reading the + // request of whatever new connection held that number by then + // (celeris#715). Since celeris#685 neither can happen (see + // CloseFDDeferred), so what Closed counts is a closed connection's + // recv reading its own client's late bytes. // - Transplanted: a connection this engine handed to the other // sub-engine. A recv armed before the hand-off outlived it and // read a request meant for the new owner — or, through a reused @@ -580,6 +580,27 @@ type EngineMetrics struct { //nolint:revive // user-approved name // probe finds the flags. io_uring-only; on the adaptive engine the sum // over both sub-engines. TransplantReapUnsupported uint64 + // CloseFDDeferred and CloseFDForced count how the io_uring close paths + // keep the same fd-lifetime rule (celeris#685): a connection's descriptor + // NUMBER is released only when no operation that names it can still be + // issued. Closing it earlier let a receive the kernel had not issued yet + // read the request of a new connection that another thread had been + // given the freed number for; that request was dropped as + // StaleRecvDataClosed, and its client was never answered. + // + // - CloseFDDeferred: closes whose descriptor stayed open until the + // last operation the kernel owed on it had completed, and was closed + // then (normally one loop iteration later). A rate: on an + // async-handler engine it is close to one per connection the server + // closes, and on a sync-mode engine it is the server-side closes of + // connections with a receive armed (timeouts). + // - CloseFDForced: such descriptors closed by the release backstop with + // an operation still owed. Must stay 0. + // + // io_uring-only and cumulative; zero on other engines. On the adaptive + // engine each is the sum over both sub-engines. + CloseFDDeferred uint64 + CloseFDForced uint64 // TransplantSweepPasses counts passes of the post-switch sweep, the // re-examination that moves a connection the drain would otherwise // reach only at that connection's own next event — which, for a diff --git a/engine/iouring/conn.go b/engine/iouring/conn.go index 732d3142..a450b9c8 100644 --- a/engine/iouring/conn.go +++ b/engine/iouring/conn.go @@ -8,6 +8,7 @@ import ( "sync/atomic" "github.com/goceleris/celeris/internal/conn" + "github.com/goceleris/celeris/internal/recvtheft" ) // maxSendQueueBytes is the per-connection back-pressure limit for @@ -397,6 +398,13 @@ type connState struct { // has repurposed. drainPendingRelease only releases a connState once // this counter reaches zero (with a wall-clock backstop for kernel // anomalies). Mirrors driverConn.inflightOps. + // + // recvArmSeq (validation builds only; zero-size in production, and not + // the last field, so it adds no padding) is the SQ ring sequence number + // of this conn's latest recv SQE: the close paths compare it with the + // kernel's SQ head to tell a recv the kernel has not consumed yet + // (celeris#715, Worker.recvUnsubmitted). Set by noteRecvPlaced. + recvArmSeq recvtheft.ArmSeq kernelInflight int32 // recvArmed is true while a recv SQE (single-shot or multishot) is // kernel-held for this conn. Set by prepareRecv / flushSendLink's diff --git a/engine/iouring/engine.go b/engine/iouring/engine.go index a65e2809..78b5e385 100644 --- a/engine/iouring/engine.go +++ b/engine/iouring/engine.go @@ -570,6 +570,8 @@ func (e *Engine) Metrics() engine.EngineMetrics { TransplantClaimDeferred: e.metrics.handoffLoss.claimDeferred.Load(), TransplantReapFailed: e.metrics.handoffLoss.reapFailed.Load(), TransplantReapUnsupported: e.metrics.handoffLoss.reapUnsupported.Load(), + CloseFDDeferred: e.metrics.handoffLoss.closeFDDeferred.Load(), + CloseFDForced: e.metrics.handoffLoss.closeFDForced.Load(), TransplantSweepPasses: e.metrics.sweep.passes.Load(), TransplantResidualDetached: e.metrics.sweep.residual[resDetached].Load(), diff --git a/engine/iouring/fd_lifetime.go b/engine/iouring/fd_lifetime.go index 9d0c7755..518f723e 100644 --- a/engine/iouring/fd_lifetime.go +++ b/engine/iouring/fd_lifetime.go @@ -3,6 +3,8 @@ package iouring import ( + "time" + "golang.org/x/sys/unix" "github.com/goceleris/celeris/engine" @@ -343,3 +345,286 @@ func (w *Worker) releaseHoldSlow(cs *connState, rescued bool) { func (w *Worker) rescueHold(cs *connState) { w.releaseHoldSlow(cs, true) } + +// The same rule for the close paths (celeris#685): a descriptor NUMBER is +// released only when no op that names it can still be submitted or issued. +// +// Fixed files are off (celeris#541), so a recv or send SQE names the +// descriptor by number, and the kernel resolves the number when it ISSUES the +// op, not when the SQE is written. Two kinds of op are not issued yet when a +// close path runs: +// +// - one still in the SQ ring: prepareRecv (a promoted connection's re-arm, +// a dirty-list retry) and flushSend only place SQEs, and they reach the +// kernel at the loop's next io_uring_enter; +// - one the kernel holds but has not issued: a recv linked behind a SEND +// (flushSendLink) is issued only when the SEND completes, and on a +// DEFER_TASKRUN ring as task work after that, which can still be queued +// when the SEND's CQE is read. +// +// finishClose and finishCloseDetached used to queue the ops' cancels and +// close(2) the descriptor at once. Another thread (a sibling worker's accept, +// the epoll sub-engine) can be given the freed number before this worker's +// next submit. The recv then reads that connection's request and completes +// under the closed connection's (fd, generation); staleConnCQE drops it as +// stale_recv_data_closed, and the new connection waits on an empty socket +// until its header deadline. Measured deterministically by +// TestRecvTheft715ArmA (celeris#715), and the linked form by +// TestRecvTheft685Linked. +// +// The rule is kept by NOT closing while the kernel owes an op on the +// descriptor (fdOwed). The close path does everything else as before (the +// cancels, the closedOps registration, the deferred release) and hands the +// descriptor to its pendingRelease entry, and drainPendingRelease closes it +// once no owed op names it: where it releases the connState, when every owed +// op has delivered its terminal CQE, or earlier when all that is left is +// SEND_ZC notifications (celeris#798). Those name no descriptor and guard +// only the send buffer, so the connState waits for them and the descriptor +// does not (fdOps). Until then the number stays allocated, so no accept or dup +// anywhere in the process can be given it, and an op issued late resolves +// this connection's own socket. To make sure every owed op does end, the +// close path also shuts the socket's read side down (shutdownHow; SHUT_RD +// alone on the fast path, which otherwise makes no shutdown call, see +// fastCloseShutdownHow): an owed recv returns at once when it is issued, even +// one no cancel can find (a linked recv not issued yet), and even on a kernel +// whose cancels fail (celeris#682). +// +// What it costs. No io_uring_enter is added: the close(2) moves from the +// close path to the release, one iteration later, and a path that already +// shut the write side down shuts both down instead. The one added syscall is +// the SHUT_RD of the H1 fast path, taken only when an op is owed there: a +// server-side close of a connection with its recv armed (a timeout); a +// sync-mode Connection: close response has no recv armed when it closes, and +// neither has a client's FIN. What the peer sees barely moves. An issued recv +// holds its own reference to the file, so its socket was never released +// before that recv's cancel landed anyway, and the FIN (or the RST of unread +// data) went out then; it goes out at the same point now. Only a close with +// a recv still in the SQ ring, whose socket nothing held, used to release the +// socket at the close and now does so one enter later. The worker does not +// park while such a close is outstanding (closeFDOwed). +// +// A hijack keeps the socket open under the hijacker, so it cannot wait: it +// submits its cancels before handing the socket over (see hijackConn). Worker +// shutdown ends the ops owed on every descriptor before it closes any +// (endOwedOpsAtShutdown). + +// fdOwed reports whether the kernel still owes cs an op that names its +// descriptor: a recv or send whose SQE was written and that has not +// completed (fdOps). A fixed-file connection names a slot, not a number, and +// keeps its own close (CLOSE_DIRECT). A nil cs (closeMissingConnState) owes +// nothing we can know of. +func fdOwed(cs *connState) bool { + return cs != nil && fdOps(cs) > 0 && !cs.fixedFile +} + +// fdOps counts the ops the kernel still owes live cs that name its +// descriptor. kernelInflight counts every recv and send from the moment its +// SQE is placed (prepareRecv, flushSend, flushSendLink, both halves of a +// linked pair), submitted or not, issued or not, and staleConnCQE retires it +// at the terminal CQE; the header timer and the cancels name no descriptor +// and are not counted. +// +// One op in that count may name nothing (celeris#798): a SEND_ZC whose send +// has completed (its first CQE, IORING_CQE_F_MORE, set zcNotifPending) keeps +// its count until the notification CQE, and the notification is not an op on +// the descriptor. It says the kernel has let go of the send buffer's pages, +// which a stalled peer's unread data can hold for as long as the socket +// lives. Counting it kept the descriptor, and so the socket, open until the +// 5 s release backstop forced it (CloseFDForced). So it is left out here, +// and only here: kernelInflight still counts it, and the connState, whose +// sendBuf those pages are, is still released only at the notification +// (drainPendingRelease). A connection has one send in flight at most +// (flushSend and flushSendLink wait out cs.sending and cs.zcNotifPending), +// so zcNotifPending stands for exactly one op. +// +// Live connections only: a closed one's count moves in its closedOps entry +// (closedOpsEntry.fdOps, read by closedFDNamed). +func fdOps(cs *connState) int32 { + if cs.zcNotifPending { + return cs.kernelInflight - 1 + } + return cs.kernelInflight +} + +// closedFDNamed reports whether an op the kernel still owes closed cs names +// its descriptor, as drainPendingRelease asks of an entry that kept the +// descriptor (holdsFD) while kernelInflight is not yet 0. Only a SEND_ZC +// notification names none (see fdOps), and only a connection whose last send +// was armed as SEND_ZC (sendIsZC, which nothing changes after the close) can +// owe one; for any other the answer is yes without a lookup. For that one, +// its closedOps entry counts what is left (fdOps). No entry means the +// accounting cannot say, and the descriptor stays kept. +func (w *Worker) closedFDNamed(cs *connState) bool { + if !cs.sendIsZC { + return true + } + e := w.closedOps[encodeConnOpKey(cs.fd, cs.generation)] + return e == nil || e.fdOps > 0 +} + +// releaseKeptFD closes the descriptor a close path left to e (holdsFD): the +// release closes it when cs goes, or earlier, once only a SEND_ZC +// notification is still owed (celeris#798). Worker thread only. +func (w *Worker) releaseKeptFD(e *pendingReleaseEntry) { + _ = unix.Close(int(e.fd)) + e.holdsFD = false + w.closeFDOwed-- +} + +// keptFD is the descriptor a close path hands to its pendingRelease entry: +// fd when an op is owed, else -1 (the close path closes it at once). +func keptFD(fd int, owed bool) int { + if owed { + return fd + } + return -1 +} + +// closeUnlessOwed is a close path's last step: close(2) now when nothing is +// owed; otherwise leave the descriptor to its pendingRelease entry, which +// drainPendingRelease closes at the last owed op's terminal CQE. +func closeUnlessOwed(fd int, owed bool) { + if !owed { + _ = unix.Close(fd) + } +} + +// shutdownHow is the shutdown(2) of a close path that half-closes: SHUT_WR +// (the FIN goes out now) as before, and the read side too while an op is +// owed, so the owed recv ends as soon as the kernel issues it. +func shutdownHow(owed bool) int { + if owed { + return unix.SHUT_RDWR + } + return unix.SHUT_WR +} + +// fastCloseShutdownHow is the shutdown(2) the H1 fast path adds when an op is +// owed: SHUT_RD, which sends nothing, so the close(2) at the release still +// decides between FIN and RST as the fast path always did. With a SEND still +// owed (the closing-drain sweep reaps a conn whose SEND never completed) the +// write side goes too: a blocked SEND then fails at once, where a cancel +// alone cannot end it on a kernel whose cancels fail (celeris#682). +func fastCloseShutdownHow(cs *connState) int { + if cs.sending || cs.zcNotifPending { + return unix.SHUT_RDWR + } + return unix.SHUT_RD +} + +// shutdownFDDrainNanos bounds how long worker shutdown waits for the ops still +// owed on connection descriptors (endOwedOpsAtShutdown). The cancels and the +// SHUT_RDWR end every one within the first enter or two on a healthy kernel; +// the bound is for a kernel that never answers, and matches the send drain +// before it (shutdownSendDrainNanos). +const shutdownFDDrainNanos int64 = int64(250 * time.Millisecond) + +// endOwedOpsAtShutdown is worker shutdown's half of the rule: it ends every op +// the kernel still owes on a live connection's descriptor, and on a +// descriptor a close path left to its pendingRelease entry (holdsFD), +// and closes the latter. shutdown closes the live ones itself, after it. +// +// Without it, shutdown closed every descriptor and then the ring. An op whose +// SQE was still in the SQ ring was harmless there (nothing submits after the +// closes), and an issued op holds its own file. The exception is an op the +// kernel had consumed and not issued: a recv linked behind a SEND, or queued +// as task work when the SEND's CQE was read. Where the ring's teardown still +// issues such work, it resolves a number shutdown had already freed. So each +// live connection with an op owed (fdOwed) has its ops cancelled and its +// socket shut down both ways (a consumed recv then ends as soon as it is +// issued, a blocked SEND at once), and the ring is run until every such op +// has delivered its terminal CQE: recv and send completions are retired by +// staleConnCQE exactly as the loop retires them, and an accept's new +// descriptor, which nobody will serve, is closed. Everything the SQ ring still +// holds is submitted by the first enter, which is why this runs before +// shutdownDrivers closes the driver descriptors. Bounded by +// shutdownFDDrainNanos; past it (or on a ring error) the descriptors are +// closed anyway, as shutdown always did. Worker thread only; skipped under +// SQPOLL (no tier enables it) and without a ring. +// +// It waits for the ops that name a descriptor, not for SEND_ZC +// notifications (celeris#798, see fdOps): a stalled peer can hold one for as +// long as the socket is open, far past this bound. A live connection's +// SEND_ZC whose first CQE the drain reads names the descriptor no more +// either; handleSend, which records that in zcNotifPending, does not run +// here, so the drain keeps its own note (zcDone) and changes nothing on the +// connection, which the dispatch goroutine may still read. +func (w *Worker) endOwedOpsAtShutdown() { + var owed []*connState + for _, fd := range w.liveConns { + cs := w.conns[fd] + if cs == nil || !fdOwed(cs) { + continue + } + owed = append(owed, cs) + } + var zcDone map[*connState]bool + // named is fdOwed for the drain: fdOps, less a SEND_ZC it saw complete. + named := func(cs *connState) bool { + n := fdOps(cs) + if zcDone[cs] { + n-- + } + return n > 0 + } + pending := func() bool { + for _, cs := range owed { + if named(cs) { + return true + } + } + for i := range w.pendingRelease { + if e := &w.pendingRelease[i]; e.holdsFD && e.cs.kernelInflight > 0 && w.closedFDNamed(e.cs) { + return true + } + } + return false + } + if w.ring != nil && !w.sqpoll && pending() { + for _, cs := range owed { + w.cancelConnOps(cs.fd, cs) + _ = unix.Shutdown(cs.fd, unix.SHUT_RDWR) + } + deadline := time.Now().UnixNano() + shutdownFDDrainNanos + for pending() && time.Now().UnixNano() < deadline { + if err := w.ring.SubmitAndWaitTimeout(10 * time.Millisecond); err != nil { + break + } + head, tail := w.ring.BeginCQ() + for ; head != tail; head++ { + c := w.ring.cqeAt(head) + ud := c.UserData + switch ud & udMask { + case udRecv, udSend: + fd := int(ud & fdMask) + // A live connection's SEND_ZC completing (F_MORE marks + // only that on a send; a connection has one send in + // flight at most): noted, or its notification would be + // waited for as an op on the descriptor. A closed + // identity's is counted by staleConnCQE. + if ud&udMask == udSend && cqeHasMore(c.Flags) && fd < len(w.conns) { + if cs := w.conns[fd]; cs != nil && cs.generation == decodeGen(ud) && !cs.zcNotifPending { + if zcDone == nil { + zcDone = make(map[*connState]bool) + } + zcDone[cs] = true + } + } + w.staleConnCQE(c, fd, ud) + case udAccept: + if c.Res >= 0 && !w.fixedFiles { + _ = unix.Close(int(c.Res)) + } + } + } + w.ring.EndCQ(head) + } + } + for i := range w.pendingRelease { + if e := &w.pendingRelease[i]; e.holdsFD { + _ = unix.Close(int(e.fd)) + e.holdsFD = false + } + } + w.closeFDOwed = 0 +} diff --git a/engine/iouring/fd_lifetime_close_test.go b/engine/iouring/fd_lifetime_close_test.go new file mode 100644 index 00000000..9032ddbd --- /dev/null +++ b/engine/iouring/fd_lifetime_close_test.go @@ -0,0 +1,662 @@ +//go:build linux + +package iouring + +import ( + "context" + "errors" + "io" + "log/slog" + "net" + "runtime" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + "unsafe" + + "golang.org/x/sys/unix" + + "github.com/goceleris/celeris/engine" + "github.com/goceleris/celeris/engine/internal/errclass" + "github.com/goceleris/celeris/internal/conn" + "github.com/goceleris/celeris/resource" +) + +// The fd-lifetime rule on the close paths (celeris#685): a descriptor number +// is released only when no op that names it can still be issued. These run in +// every build and need one ring (or one worker), so they run in the CI `unit` +// job's shape; the trials that force the theft itself are the -tags=validation +// TestRecvTheft715ArmA, TestRecvTheft685Linked and TestRecvTheft685Hijack*. + +// TestPendingReleaseEntryStaysTwentyFourBytes pins that the celeris#685 hold +// (holdsFD, fd) fits in the padding after detached: the queue is appended to +// on every close. +func TestPendingReleaseEntryStaysTwentyFourBytes(t *testing.T) { + if unsafe.Sizeof(uintptr(0)) != 8 { + t.Skip("64-bit layout only") + } + if n := unsafe.Sizeof(pendingReleaseEntry{}); n != 24 { + t.Fatalf("pendingReleaseEntry is %d bytes, want 24", n) + } +} + +// newOwedCloseWorker returns a synthetic worker with a real ring and one H1 +// connection on a socketpair, and the peer end (never read by the worker). +// detached gives the connection a detachMu, which routes closeConn to +// finishCloseDetached, as for an async-dispatch connection. +func newOwedCloseWorker(t *testing.T, detached bool) (*Worker, *connState, int) { + t.Helper() + ring := newTestRing(t) + pair, err := unix.Socketpair(unix.AF_UNIX, unix.SOCK_STREAM|unix.SOCK_NONBLOCK, 0) + if err != nil { + t.Skipf("socketpair: %v", err) + } + local, peer := pair[0], pair[1] + localTarget := fdTarget(local) + t.Cleanup(func() { + _ = unix.Close(peer) + // Only if the test left it open: the number may be someone else's + // once the release closed it. + if fdTarget(local) == localTarget { + _ = unix.Close(local) + } + }) + w := &Worker{ + ring: ring, + conns: make([]*connState, local+1), + liveConns: make([]int, 0, 4), + errs: &errclass.Counters{}, + activeConns: &atomic.Int64{}, + closeCount: &atomic.Uint64{}, + recvArm: &recvArmStats{}, + handoffLoss: &handoffLossStats{}, + } + w.cachedNow = time.Now().UnixNano() + cs := &connState{ + fd: local, + liveIdx: -1, + generation: 11, + buf: make([]byte, 4096), + h1State: conn.NewH1State(), + detected: true, + } + cs.protocol.Store(int32(engine.HTTP1)) + if detached { + cs.detachMu = new(sync.Mutex) + } + w.conns[local] = cs + w.addLiveConn(cs) + w.connCount = 1 + w.activeConns.Add(1) + return w, cs, peer +} + +// runRingOnce submits what the ring holds, waits up to d for completions, and +// feeds every recv/send completion to staleConnCQE, as the loop does. +// Returns the recv completions' results. +func runRingOnce(t *testing.T, w *Worker, d time.Duration) []int32 { + t.Helper() + if err := w.ring.SubmitAndWaitTimeout(d); err != nil { + t.Fatalf("submit: %v", err) + } + var res []int32 + head, tail := w.ring.BeginCQ() + for ; head != tail; head++ { + c := w.ring.cqeAt(head) + ud := c.UserData + switch ud & udMask { + case udRecv, udSend: + if ud&udMask == udRecv { + res = append(res, c.Res) + } + w.staleConnCQE(c, int(ud&fdMask), ud) + } + } + w.ring.EndCQ(head) + return res +} + +// peerSawEnd reports whether the peer reads EOF (or a reset) within d. +func peerSawEnd(peer int, d time.Duration) bool { + buf := make([]byte, 64) + for end := time.Now().Add(d); time.Now().Before(end); { + n, err := unix.Read(peer, buf) + if n == 0 && err == nil { + return true + } + if err != nil && !errors.Is(err, unix.EAGAIN) && !errors.Is(err, unix.EINTR) { + return true + } + time.Sleep(time.Millisecond) + } + return false +} + +// TestFinishCloseKeepsDescriptorWhileRecvOwed is the rule's mechanism on the +// H1 fast path: a close with a recv SQE placed and not submitted (the #715 +// precondition) must leave the descriptor open, shut its read side, and hand +// it to its pendingRelease entry; the recv, issued by the next enter, ends at +// once against this socket; the release then closes the descriptor. +func TestFinishCloseKeepsDescriptorWhileRecvOwed(t *testing.T) { + for _, detached := range []bool{false, true} { + t.Run(map[bool]string{false: "fast", true: "detached"}[detached], func(t *testing.T) { + w, cs, peer := newOwedCloseWorker(t, detached) + fd := cs.fd + target := fdTarget(fd) + if !w.prepareRecv(cs, cs.buf) || cs.kernelInflight != 1 { + t.Fatalf("recv not placed: kernelInflight=%d", cs.kernelInflight) + } + w.closeConn(fd) + if w.conns[fd] != nil { + t.Fatal("the conn is still registered after closeConn") + } + if got := fdTarget(fd); got != target { + t.Fatalf("fd %d was released with its recv still unsubmitted (now %q, was %q)", fd, got, target) + } + if len(w.pendingRelease) != 1 || !w.pendingRelease[0].holdsFD || int(w.pendingRelease[0].fd) != fd || w.closeFDOwed != 1 { + t.Fatalf("the kept descriptor is not on its pendingRelease entry: %+v closeFDOwed=%d", w.pendingRelease, w.closeFDOwed) + } + // The async path shuts the write side down too, as it always did: + // its client sees the FIN now, not at the release. + if detached && !peerSawEnd(peer, time.Second) { + t.Error("finishCloseDetached kept the descriptor but its peer saw no FIN") + } + w.drainPendingRelease() + if fdTarget(fd) != target { + t.Fatal("drainPendingRelease closed the descriptor before the owed recv ended") + } + var res []int32 + for end := time.Now().Add(2 * time.Second); cs.kernelInflight > 0 && time.Now().Before(end); { + res = append(res, runRingOnce(t, w, 50*time.Millisecond)...) + } + if cs.kernelInflight != 0 { + t.Fatalf("the owed recv never ended: kernelInflight=%d", cs.kernelInflight) + } + w.cachedNow = time.Now().UnixNano() + w.drainPendingRelease() + if fdTarget(fd) == target { + t.Fatalf("fd %d still open after its owed recv ended (recv results %v)", fd, res) + } + if w.closeFDOwed != 0 || len(w.pendingRelease) != 0 { + t.Fatalf("closeFDOwed=%d pendingRelease=%d after the release, want 0 and 0", w.closeFDOwed, len(w.pendingRelease)) + } + if !peerSawEnd(peer, time.Second) { + t.Error("the peer saw no end of the connection after the release") + } + t.Logf("celeris685 mechanism detached=%v recv_results=%v", detached, res) + }) + } +} + +// TestFinishCloseClosesAtOnceWhenNothingOwed is the other half: with no op +// owed (the sync Connection: close response, a client's FIN) the close is +// what it was, the descriptor closed at once and nothing held. +func TestFinishCloseClosesAtOnceWhenNothingOwed(t *testing.T) { + w, cs, peer := newOwedCloseWorker(t, false) + fd := cs.fd + target := fdTarget(fd) + w.closeConn(fd) + if fdTarget(fd) == target { + t.Fatalf("fd %d kept open with nothing owed", fd) + } + if w.closeFDOwed != 0 || (len(w.pendingRelease) == 1 && w.pendingRelease[0].holdsFD) { + t.Fatalf("a close with nothing owed registered a kept descriptor: closeFDOwed=%d %+v", w.closeFDOwed, w.pendingRelease) + } + if !peerSawEnd(peer, time.Second) { + t.Error("the peer saw no end of the connection") + } +} + +// TestShutdownEndsOwedOpsBeforeClosing drives worker shutdown with idle +// keep-alive connections, each with its next recv armed (linked behind its +// last response's SEND), so every one has an op owed at shutdown. Shutdown +// must end those ops and then close every descriptor: none may be left open, +// the engine must stop promptly, and the release backstop must not be what +// closed any of them. +func TestShutdownEndsOwedOpsBeforeClosing(t *testing.T) { + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + addr := ln.Addr().String() + _ = ln.Close() + e, err := New(resource.Config{ + Addr: addr, + Protocol: engine.HTTP1, + Resources: resource.Resources{Workers: 2}, + Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + }, transplantTestHandler{}) + if err != nil { + skipOrFail656(t, "iouring engine unavailable: %v", err) + } + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { done <- e.Listen(ctx) }() + defer cancel() + for deadline := time.Now().Add(8 * time.Second); e.Addr() == nil; { + select { + case err := <-done: + skipOrFail656(t, "iouring engine failed to start: %v", err) + default: + } + if time.Now().After(deadline) { + t.Fatal("engine did not start") + } + time.Sleep(10 * time.Millisecond) + } + const n = 8 + var clients []net.Conn + var servers []int + var targets []string + for i := range n { + c, err := net.DialTimeout("tcp", addr, time.Second) + if err != nil { + t.Fatalf("dial %d: %v", i, err) + } + clients = append(clients, c) + _ = c.SetDeadline(time.Now().Add(3 * time.Second)) + if _, err := c.Write([]byte("GET / HTTP/1.1\r\nHost: x\r\n\r\n")); err != nil { + t.Fatalf("write %d: %v", i, err) + } + buf := make([]byte, 512) + var got []byte + for !strings.Contains(string(got), "\r\n\r\nok") { + k, err := c.Read(buf) + got = append(got, buf[:k]...) + if err != nil { + t.Fatalf("read %d: %v (%q)", i, err, got) + } + } + srv := -1 + for end := time.Now().Add(time.Second); srv < 0 && time.Now().Before(end); { + srv = serverFDFor(c.LocalAddr().String()) + } + if srv < 0 { + t.Fatalf("conn %d: no server-side descriptor found", i) + } + servers = append(servers, srv) + targets = append(targets, fdTarget(srv)) + } + defer func() { + for _, c := range clients { + _ = c.Close() + } + }() + start := time.Now() + cancel() + select { + case <-done: + case <-time.After(3 * time.Second): + t.Fatal("the engine did not stop within 3s") + } + took := time.Since(start) + left := 0 + for i, fd := range servers { + if fdTarget(fd) == targets[i] { + left++ + } + } + m := e.Metrics() + t.Logf("celeris685 shutdown conns=%d took=%v left_open=%d close_fd_forced=%d", n, took, left, m.CloseFDForced) + if left != 0 { + t.Fatalf("%d of %d server-side descriptors still open after the engine stopped", left, n) + } + if m.CloseFDForced != 0 { + t.Fatalf("CloseFDForced = %d, want 0", m.CloseFDForced) + } +} + +// The SEND_ZC half of the rule (celeris#798). A SEND_ZC completes in two +// CQEs: the send's result (IORING_CQE_F_MORE), then a notification once the +// kernel has let go of the send buffer's pages. The notification names no +// descriptor, so no op can resolve the number through it; it guards the send +// buffer, which the connState holds. But a peer that stops reading keeps the +// unsent part of the send queued, and the notification with it, for as long +// as the socket is open, and keeping the descriptor keeps the socket open. A +// close that counted the notification as an op owed on the descriptor kept +// it until the 5 s release backstop forced it (CloseFDForced, which must +// stay 0), and worker shutdown waited out its whole drain bound for it. + +// zcReleaseBound is how soon after the close the descriptor must be closed +// when all that holds the connection is a SEND_ZC notification and ops that +// end at the close's cancel and shutdown. The rule closes it at the close +// itself when no owed op names it, and otherwise at the drainPendingRelease +// of the first loop pass that reads the last naming op's terminal CQE (a +// cancelled recv) or the send's first CQE: one pass, and a pass here waits +// at most 10 ms for completions. 200 ms leaves 20 passes of headroom for +// -race on a 4-CPU container, and is 25 times shorter than the 5 s backstop +// (pendingReleaseHoldNanos) the descriptor was held to. +const zcReleaseBound = 200 * time.Millisecond + +// zcShutdownBound is the same for worker shutdown's drain: with only a +// notification owed it does not drain at all, and with the send's first CQE +// already in the ring it stops at the first pass, which returns at once. +// Held for the notification, the drain ran its whole 250 ms bound +// (shutdownFDDrainNanos); 100 ms sits well between the two. +const zcShutdownBound = 100 * time.Millisecond + +// zcPayload is the send: 64 KiB, far over sendZCMinBytes (so it goes out as +// SEND_ZC) and over what a peer with a 4 KiB receive buffer can take, so what +// the send queued past the peer's window stays in the server's send queue and +// holds the notification. Small enough that the pages SEND_ZC charges to +// RLIMIT_MEMLOCK fit the CI unit job's 8 MiB next to the test ring. +const zcPayload = 64 << 10 + +// newZCCloseWorker is newOwedCloseWorker over loopback TCP, with SEND_ZC on +// as the engine turns it on: the startup probe must find it functional, and +// CELERIS_IOURING_SEND_ZC=on then enables it (resolveSendZCPolicy); a kernel +// where the engine never sends zero-copy cannot reach the case. The peer has +// a 4 KiB receive buffer, set before the connect so that the window it +// advertises is small from the first segment, and reads nothing until +// drainZCPeer. +func newZCCloseWorker(t *testing.T) (*Worker, *connState, int) { + t.Helper() + res, reason := probeSendZCCached() + if on, _ := resolveSendZCPolicy(res == SendZCTrueZeroCopy || res == SendZCCopyFallback, "on"); !on { + t.Skipf("the engine does not turn SEND_ZC on here (probe: %v %s)", res, reason) + } + ring := newTestRing(t) + lfd, err := unix.Socket(unix.AF_INET, unix.SOCK_STREAM|unix.SOCK_CLOEXEC, 0) + if err != nil { + t.Fatalf("listen socket: %v", err) + } + defer func() { _ = unix.Close(lfd) }() + if err := unix.Bind(lfd, &unix.SockaddrInet4{Addr: [4]byte{127, 0, 0, 1}}); err != nil { + t.Fatalf("bind: %v", err) + } + if err := unix.Listen(lfd, 1); err != nil { + t.Fatalf("listen: %v", err) + } + sa, err := unix.Getsockname(lfd) + if err != nil { + t.Fatalf("getsockname: %v", err) + } + peer, err := unix.Socket(unix.AF_INET, unix.SOCK_STREAM|unix.SOCK_CLOEXEC, 0) + if err != nil { + t.Fatalf("peer socket: %v", err) + } + if err := unix.SetsockoptInt(peer, unix.SOL_SOCKET, unix.SO_RCVBUF, 4096); err != nil { + _ = unix.Close(peer) + t.Fatalf("SO_RCVBUF: %v", err) + } + if err := unix.Connect(peer, sa); err != nil { + _ = unix.Close(peer) + t.Fatalf("connect: %v", err) + } + local, _, err := unix.Accept4(lfd, unix.SOCK_NONBLOCK|unix.SOCK_CLOEXEC) + if err != nil { + _ = unix.Close(peer) + t.Fatalf("accept: %v", err) + } + localTarget := fdTarget(local) + t.Cleanup(func() { + _ = unix.Close(peer) + if fdTarget(local) == localTarget { + _ = unix.Close(local) + } + }) + w := &Worker{ + ring: ring, + conns: make([]*connState, local+1), + liveConns: make([]int, 0, 4), + errs: &errclass.Counters{}, + activeConns: &atomic.Int64{}, + closeCount: &atomic.Uint64{}, + recvArm: &recvArmStats{}, + handoffLoss: &handoffLossStats{}, + sendZC: true, + } + w.cachedNow = time.Now().UnixNano() + cs := &connState{ + fd: local, + liveIdx: -1, + generation: 13, + buf: make([]byte, 4096), + h1State: conn.NewH1State(), + detected: true, + } + cs.protocol.Store(int32(engine.HTTP1)) + w.conns[local] = cs + w.addLiveConn(cs) + w.connCount = 1 + w.activeConns.Add(1) + return w, cs, peer +} + +// startZCSend places one SEND_ZC of zcPayload bytes on cs, submits it, and +// returns what it sent once its first CQE (the send's result, F_MORE) is in +// the completion ring. With reap it then dispatches that CQE as the loop does +// (staleConnCQE, then handleSend, which records zcNotifPending); without, it +// leaves it there unread, as a close that runs before the loop reads it finds +// it. Either way the notification must still be owed 100 ms later, with the +// peer reading nothing, or the case did not form. +func startZCSend(t *testing.T, w *Worker, cs *connState, reap bool) int32 { + t.Helper() + payload := make([]byte, zcPayload) + for i := range payload { + payload[i] = byte(i) + } + cs.writeBuf = payload + if w.flushSend(cs) || !cs.sendIsZC || cs.kernelInflight != 1 { + t.Fatalf("no SEND_ZC placed: sendIsZC=%v kernelInflight=%d", cs.sendIsZC, cs.kernelInflight) + } + if _, err := w.ring.Submit(); err != nil { + t.Fatalf("submit: %v", err) + } + var c *completionEntry + for end := time.Now().Add(2 * time.Second); c == nil; { + if head, tail := w.ring.BeginCQ(); head != tail { + c = w.ring.cqeAt(head) + break + } + if time.Now().After(end) { + t.Fatal("no completion for the SEND_ZC within 2s") + } + time.Sleep(time.Millisecond) + } + if c.UserData&udMask != udSend || !cqeHasMore(c.Flags) || c.Res <= 0 { + t.Fatalf("first completion ud=%#x flags=%#x res=%d, want the SEND_ZC's result (F_MORE, res > 0)", c.UserData, c.Flags, c.Res) + } + sent := c.Res + if reap { + head, _ := w.ring.BeginCQ() + if !w.staleConnCQE(c, cs.fd, c.UserData) { + w.handleSend(c, cs.fd, time.Now().UnixNano()) + } + w.ring.EndCQ(head + 1) + if !cs.zcNotifPending || !cs.sending || cs.kernelInflight != 1 { + t.Fatalf("after the first CQE: zcNotifPending=%v sending=%v kernelInflight=%d, want true true 1", cs.zcNotifPending, cs.sending, cs.kernelInflight) + } + } + time.Sleep(100 * time.Millisecond) + want := uint32(1) + if reap { + want = 0 + } + if head, tail := w.ring.BeginCQ(); tail-head != want { + t.Fatalf("%d completions in the ring 100 ms after the send, want %d: the notification arrived with the peer "+ + "reading nothing (sent %d of %d), so the case did not form", tail-head, want, sent, zcPayload) + } + return sent +} + +// sweepClose closes fd as a connection with a send still owed is closed: +// closeConn defers it (cs.closing), and the closing-drain sweep in +// checkTimeouts reaps it once the peer has read nothing for +// closingDrainTimeoutNanos. The sweep's two calls, without the 5 s wait. +func sweepClose(t *testing.T, w *Worker, cs *connState) { + t.Helper() + fd := cs.fd + w.closeConn(fd) + if w.conns[fd] != cs || !cs.closing { + t.Fatalf("closeConn did not defer the close of a conn with a send owed: registered=%v closing=%v", w.conns[fd] == cs, cs.closing) + } + w.removeDirty(cs) + w.finishCloseAny(fd, cs) + if w.conns[fd] != nil { + t.Fatal("the conn is still registered after the sweep's close") + } +} + +// drainZCPeer lets the peer read everything to EOF while the loop runs, and +// reports what it read and whether the notification arrived (cs's last op +// retired while cs was still queued for release) before cs was released. +func drainZCPeer(t *testing.T, w *Worker, cs *connState, peer int) (got int, eof, notified bool) { + t.Helper() + if err := unix.SetNonblock(peer, true); err != nil { + t.Fatalf("peer nonblock: %v", err) + } + buf := make([]byte, 64<<10) + for end := time.Now().Add(3 * time.Second); time.Now().Before(end); { + for !eof { + n, err := unix.Read(peer, buf) + if n > 0 { + got += n + continue + } + if n == 0 && err == nil { + eof = true + } + break + } + runRingOnce(t, w, 5*time.Millisecond) + if len(w.pendingRelease) == 1 && w.pendingRelease[0].cs == cs && cs.kernelInflight == 0 { + notified = true + } + w.cachedNow = time.Now().UnixNano() + w.drainPendingRelease() + if eof && len(w.pendingRelease) == 0 { + break + } + } + return got, eof, notified +} + +// TestCloseReleasesDescriptorWithOnlyAZCNotificationOwed is celeris#798 on the +// close paths. A connection sent a SEND_ZC to a peer that stopped reading, so +// the notification stays owed, and was closed by the closing-drain sweep: +// +// - notif-only: the send's first CQE was read; nothing else is owed. No +// op names the descriptor, so it must be closed at the close. +// - recv-and-notif: a recv was armed after the send too (the keep-alive +// path arms one behind every unlinked send). The descriptor is kept for +// the recv, which the close's cancel and shutdown end at once, and must +// then be closed although the notification is still owed. +// - send-done-after-close: the send's first CQE was in the ring, unread, +// when the close ran, so the close kept the descriptor for the send. The +// CQE says the send is done, and the descriptor must then be closed. +// +// In each, the descriptor must be closed within zcReleaseBound of the close +// and CloseFDForced must stay 0. The send buffer must not go with it: the +// connState, whose sendBuf the kernel may still read, must stay queued until +// the notification arrives (here, once the peer has read everything), and +// be released then, by the notification and not by the backstop. +func TestCloseReleasesDescriptorWithOnlyAZCNotificationOwed(t *testing.T) { + for _, tc := range []struct { + name string + reap, recv bool + }{ + {"notif-only", true, false}, + {"recv-and-notif", true, true}, + {"send-done-after-close", false, false}, + } { + t.Run(tc.name, func(t *testing.T) { + runtime.LockOSThread() + defer runtime.UnlockOSThread() + w, cs, peer := newZCCloseWorker(t) + fd := cs.fd + target := fdTarget(fd) + sent := startZCSend(t, w, cs, tc.reap) + if tc.recv { + if !w.prepareRecv(cs, cs.buf) || cs.kernelInflight != 2 { + t.Fatalf("recv not placed: kernelInflight=%d", cs.kernelInflight) + } + if res := runRingOnce(t, w, 10*time.Millisecond); len(res) != 0 { + t.Fatalf("the recv completed before the close (%v): the peer sent nothing", res) + } + } + closedAt := time.Now() + sweepClose(t, w, cs) + var releasedAfter time.Duration + for { + if fdTarget(fd) != target { + releasedAfter = time.Since(closedAt) + break + } + if time.Since(closedAt) > 6*time.Second { + t.Fatalf("fd %d still open 6s after the close", fd) + } + runRingOnce(t, w, 10*time.Millisecond) + w.cachedNow = time.Now().UnixNano() + w.drainPendingRelease() + } + forced := w.handoffLoss.closeFDForced.Load() + t.Logf("celeris798 close case=%s sent=%d released_after=%v close_fd_forced=%d", tc.name, sent, releasedAfter.Round(time.Microsecond), forced) + if releasedAfter > zcReleaseBound || forced != 0 { + t.Fatalf("fd %d was closed %v after the close (bound %v) with CloseFDForced=%d: a pending SEND_ZC "+ + "notification, which names no descriptor, held it", fd, releasedAfter, zcReleaseBound, forced) + } + if w.closeFDOwed != 0 { + t.Fatalf("closeFDOwed=%d after the descriptor was closed, want 0", w.closeFDOwed) + } + // The send buffer: cs stays queued, holding sendBuf, while the + // notification is owed, however often the release runs. + for end := time.Now().Add(100 * time.Millisecond); time.Now().Before(end); { + runRingOnce(t, w, 10*time.Millisecond) + w.cachedNow = time.Now().UnixNano() + w.drainPendingRelease() + } + if len(w.pendingRelease) != 1 || w.pendingRelease[0].cs != cs || w.pendingRelease[0].holdsFD || cs.kernelInflight == 0 || cs.fd != fd { + t.Fatalf("with the notification still owed the connState must stay queued for release without "+ + "its descriptor: pendingRelease=%+v kernelInflight=%d", w.pendingRelease, cs.kernelInflight) + } + got, eof, notified := drainZCPeer(t, w, cs, peer) + t.Logf("celeris798 close case=%s peer_got=%d eof=%v notified=%v released=%v", tc.name, got, eof, notified, len(w.pendingRelease) == 0) + if !eof || got != int(sent) { + t.Fatalf("the peer read %d bytes (EOF %v), want the %d the send completed with and then EOF", got, eof, sent) + } + if !notified || len(w.pendingRelease) != 0 { + t.Fatalf("the connState was not released at the notification: notified=%v pendingRelease=%d", notified, len(w.pendingRelease)) + } + if forced := w.handoffLoss.closeFDForced.Load(); forced != 0 { + t.Fatalf("CloseFDForced = %d, want 0", forced) + } + }) + } +} + +// TestShutdownDoesNotWaitForAZCNotification is celeris#798 at worker +// shutdown: endOwedOpsAtShutdown waits for the ops that name a live +// connection's descriptor, and a SEND_ZC notification names none. With the +// notification owed after the send's first CQE was read (notif-pending), or +// with that CQE still in the ring when the drain starts +// (send-done-during-drain), the drain must end within zcShutdownBound rather +// than run out its 250 ms bound. +func TestShutdownDoesNotWaitForAZCNotification(t *testing.T) { + for _, tc := range []struct { + name string + reap bool + }{ + {"notif-pending", true}, + {"send-done-during-drain", false}, + } { + t.Run(tc.name, func(t *testing.T) { + runtime.LockOSThread() + defer runtime.UnlockOSThread() + w, cs, _ := newZCCloseWorker(t) + sent := startZCSend(t, w, cs, tc.reap) + start := time.Now() + w.endOwedOpsAtShutdown() + took := time.Since(start) + t.Logf("celeris798 shutdown case=%s sent=%d took=%v kernelInflight_after=%d zcNotifPending_after=%v", tc.name, sent, took.Round(time.Microsecond), cs.kernelInflight, cs.zcNotifPending) + if took > zcShutdownBound { + t.Fatalf("endOwedOpsAtShutdown took %v (bound %v) with only a SEND_ZC notification owed", took, zcShutdownBound) + } + if w.closeFDOwed != 0 { + t.Fatalf("closeFDOwed=%d after shutdown's drain, want 0", w.closeFDOwed) + } + }) + } +} diff --git a/engine/iouring/fd_probe_linux_test.go b/engine/iouring/fd_probe_linux_test.go new file mode 100644 index 00000000..00f29128 --- /dev/null +++ b/engine/iouring/fd_probe_linux_test.go @@ -0,0 +1,53 @@ +//go:build linux + +package iouring + +import ( + "os" + "strconv" + + "golang.org/x/sys/unix" +) + +// Descriptor probes shared by the celeris#685 / celeris#715 tests (the +// fd-lifetime tests in every build, the recv-theft trials under +// -tags=validation). They read /proc/self/fd, so they see this process's +// descriptors only. + +// fdTarget returns what /proc/self/fd/ names ("socket:[inode]" for a +// socket), or "" when fd is not open. +func fdTarget(fd int) string { + s, err := os.Readlink("/proc/self/fd/" + strconv.Itoa(fd)) + if err != nil { + return "" + } + return s +} + +// serverFDFor returns this process's descriptor whose peer is local (the +// server side of a loopback connection the test dialed), or -1. +func serverFDFor(local string) int { + ents, err := os.ReadDir("/proc/self/fd") + if err != nil { + return -1 + } + for _, ent := range ents { + fd, err := strconv.Atoi(ent.Name()) + if err != nil { + continue + } + if namesPeer(fd, local) { + return fd + } + } + return -1 +} + +// namesPeer reports whether fd is a socket whose peer is local: the server +// side of the loopback connection the test dialed from local. It tells a +// descriptor number still naming that connection's socket from the same +// number freed and given to another socket. +func namesPeer(fd int, local string) bool { + sa, err := unix.Getpeername(fd) + return err == nil && sockaddrString(sa) == local +} diff --git a/engine/iouring/handoff_loss.go b/engine/iouring/handoff_loss.go index 60879284..a36bcd38 100644 --- a/engine/iouring/handoff_loss.go +++ b/engine/iouring/handoff_loss.go @@ -2,7 +2,11 @@ package iouring -import "sync/atomic" +import ( + "sync/atomic" + + "github.com/goceleris/celeris/internal/recvtheft" +) // handoffLossStats are the celeris#657 witnesses: the request loss a // reverse (io_uring→epoll) hand-off can cause, counted where it happens. @@ -33,8 +37,13 @@ import "sync/atomic" // the socket lives on under the hijacker's net.Conn, so a recv that // completes with data before its cancel lands has read bytes from a // connection its client is still using. After a close, a recv that -// had not reached the kernel yet resolves the fd NUMBER when it does, -// and a new connection may hold that number by then. +// had not been issued yet resolved the fd NUMBER when it was, and a +// new connection could hold that number by then (celeris#715). Both +// are closed off by the fd-lifetime rule on those paths +// (celeris#685): hijackConn submits its cancels before handing the +// socket over, and a close keeps the number while an op is owed. +// What Closed still counts is a closed conn's recv reading its OWN +// client's late bytes, which that client sees end with the close. // // A stale CQE whose (fd, generation) equals the fd's current // occupant's (a generation collision, which needs the process-wide @@ -92,6 +101,17 @@ import "sync/atomic" // paths only, never on the per-request path while no drain is set, and like // the celeris#586 witnesses they are per-event invariants that a // per-iteration batch could lose at loop exit. +// +// The same rule on the close paths (celeris#685; see the close-path section +// of fd_lifetime.go) keeps two more: +// +// - closeFDDeferred: closes whose descriptor was left open because the +// kernel still owed an op on it, and closed at that op's terminal CQE. A +// rate, and on an async-handler engine with Connection: close traffic +// close to one per request, so it is the one counter here that is +// batched per loop iteration (Worker.closeFDDeferredBatch). +// - closeFDForced: such descriptors the pendingRelease backstop closed +// with an op still owed. Must stay 0. type handoffLossStats struct { staleRecvDataClosed atomic.Uint64 staleRecvDataTransplanted atomic.Uint64 @@ -105,6 +125,14 @@ type handoffLossStats struct { claimDeferred atomic.Uint64 reapFailed atomic.Uint64 reapUnsupported atomic.Uint64 + closeFDDeferred atomic.Uint64 + closeFDForced atomic.Uint64 +} + +func (s *handoffLossStats) noteCloseFDForced() { + if s != nil { + s.closeFDForced.Add(1) + } } // The fd-lifetime counters are nil-safe: a hand-built test Worker has none. @@ -178,6 +206,22 @@ func (w *Worker) noteStaleRecvData(ud uint64) { } } +// noteStaleRecvExemplar hands an armed recvtheft trial what a stale recv +// with data read: its identity, its result and the first bytes of the closed +// conn's cs.buf, where every single-shot recv except the direct-body one +// lands (celeris#715). Validation builds only (the caller is guarded by +// recvtheft.Enabled). Must run before noteStaleTerminalOp retires the +// closedOps entry. Worker thread only. +func (w *Worker) noteStaleRecvExemplar(c *completionEntry, fd int, ud uint64) { + var head []byte + if e := w.closedOps[connOpKey(ud)]; e != nil && len(e.conns) > 0 && w.bufRing == nil { + buf := e.conns[0].buf + n := min(int(c.Res), len(buf), 64) + head = buf[:n] + } + recvtheft.NoteStaleRecvData(w.id, fd, decodeGen(ud), c.Res, head) +} + // noteHandoffInFlight counts a hand-off that detached cs while the kernel // still held, or was about to be handed, an op on its descriptor. Called at // both hand-off sites after the hand-off is committed and before the close diff --git a/engine/iouring/handoff_loss_metrics_test.go b/engine/iouring/handoff_loss_metrics_test.go index 773b0354..f62a4e96 100644 --- a/engine/iouring/handoff_loss_metrics_test.go +++ b/engine/iouring/handoff_loss_metrics_test.go @@ -26,6 +26,8 @@ func TestMetricsCarriesTheHandoffLossWitnesses(t *testing.T) { e.metrics.handoffLoss.claimDeferred.Store(31) e.metrics.handoffLoss.reapFailed.Store(37) e.metrics.handoffLoss.reapUnsupported.Store(41) + e.metrics.handoffLoss.closeFDDeferred.Store(43) + e.metrics.handoffLoss.closeFDForced.Store(47) m := e.Metrics() for _, c := range []struct { @@ -44,6 +46,8 @@ func TestMetricsCarriesTheHandoffLossWitnesses(t *testing.T) { {"TransplantClaimDeferred", m.TransplantClaimDeferred, 31}, {"TransplantReapFailed", m.TransplantReapFailed, 37}, {"TransplantReapUnsupported", m.TransplantReapUnsupported, 41}, + {"CloseFDDeferred", m.CloseFDDeferred, 43}, + {"CloseFDForced", m.CloseFDForced, 47}, } { if c.got != c.want { t.Errorf("Metrics().%s = %d, want %d — the witness exists but cannot "+ diff --git a/engine/iouring/recv_theft_685_hijack_linux_test.go b/engine/iouring/recv_theft_685_hijack_linux_test.go new file mode 100644 index 00000000..482250e7 --- /dev/null +++ b/engine/iouring/recv_theft_685_hijack_linux_test.go @@ -0,0 +1,221 @@ +//go:build linux && validation + +package iouring + +import ( + "context" + "io" + "log/slog" + "net" + "strings" + "testing" + "time" + + "github.com/goceleris/celeris/engine" + "github.com/goceleris/celeris/internal/recvtheft" + "github.com/goceleris/celeris/protocol/h2/stream" + "github.com/goceleris/celeris/resource" +) + +// celeris#685, the hijack path. hijackConn hands the socket to the handler as +// a net.Conn and queues the cancel of any op the worker still has on it; the +// cancel reaches the kernel at the worker's next io_uring_enter. An op the +// kernel still owes the connection there can read the hijacker's first bytes +// first. Which op, and on which ring: +// +// - single-shot recv (the default): the recv that brought the request has +// completed before its handler runs, so nothing is owed at the hijack +// (TestRecvTheft685HijackSingleShot: the witness stays 0); +// - multishot recv (CELERIS_IOURING_MULTISHOT_RECV=1): the recv stays +// armed across its request, so it is owed. Its completion runs as task +// work. On a DEFER_TASKRUN ring that work runs only inside the enter, +// after the submit of the queued cancel, so the cancel wins +// (TestRecvTheft685HijackMultishotDefer). On a ring without it (kernels +// before 6.1, or wherever the probe finds DEFER_TASKRUN unusable) it runs +// at the worker thread's next return from any syscall, before that enter +// (TestRecvTheft685HijackMultishotCoop, a COOP_TASKRUN ring as the high +// tier builds without DEFER_TASKRUN). +// +// Each test hijacks one connection with the worker held right after the +// hijack (recvtheft.SetHijackHold), has the client send a payload during the +// hold, and asserts that the hijacker reads it and that no stale recv read +// it. Needs CELERIS_RECV_THEFT_715=1 and an io_uring worker (one is enough). + +const ( + hijack685Hold = 300 * time.Millisecond + hijack685Payload = "PING-685-hijacker-first-bytes\n" +) + +type hijack685Handler struct{ conns chan net.Conn } + +func (h hijack685Handler) HandleStream(_ context.Context, s *stream.Stream) error { + if s.ResponseWriter == nil { + return nil + } + if s.Path != "/hijack" { + return s.ResponseWriter.WriteResponse(s, 200, + [][2]string{{"content-type", "text/plain"}, {"content-length", "2"}}, []byte("ok")) + } + hj, ok := s.ResponseWriter.(stream.Hijacker) + if !ok { + h.conns <- nil + return nil + } + c, err := hj.Hijack(s) + if err != nil { + h.conns <- nil + return nil + } + h.conns <- c + return nil +} + +// hijack685Tier is how a test arm sets the engine's tier up before Listen. +type hijack685Tier int + +const ( + hijackTierDefault hijack685Tier = iota // as probed; single-shot recv + hijackTierMultiDefer // multishot recv, DEFER_TASKRUN ring + hijackTierMultiCoop // multishot recv, COOP_TASKRUN ring +) + +func runHijack685(t *testing.T, tier hijack685Tier) { + t.Helper() + requireRecvTheft715(t) + if tier != hijackTierDefault { + t.Setenv("CELERIS_IOURING_MULTISHOT_RECV", "1") + } + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("pick port: %v", err) + } + addr := ln.Addr().String() + _ = ln.Close() + h := hijack685Handler{conns: make(chan net.Conn, 1)} + e, err := New(resource.Config{ + Addr: addr, + Protocol: engine.HTTP1, + Resources: resource.Resources{Workers: 2}, + Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + }, h) + if err != nil { + skipOrFail656(t, "iouring engine unavailable: %v", err) + } + if tier != hijackTierDefault { + ht, ok := e.tier.(*highTier) + if !ok || !ht.multishotRecv { + skipOrFail656(t, "celeris#685 hijack arm needs the high tier with multishot recv; have %T", e.tier) + } + if tier == hijackTierMultiDefer && !ht.deferTaskrun { + skipOrFail656(t, "celeris#685 hijack arm needs DEFER_TASKRUN") + } + cp := *ht + cp.deferTaskrun = tier == hijackTierMultiDefer + cp.fixedFiles = false + e.tier = &cp + } + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { done <- e.Listen(ctx) }() + defer func() { + cancel() + select { + case <-done: + case <-time.After(5 * time.Second): + t.Error("engine did not stop within 5s") + } + }() + for deadline := time.Now().Add(8 * time.Second); e.Addr() == nil; { + select { + case err := <-done: + skipOrFail656(t, "iouring engine failed to start: %v", err) + default: + } + if time.Now().After(deadline) { + t.Fatal("engine did not start listening within 8s") + } + time.Sleep(10 * time.Millisecond) + } + var multishot, deferTR bool + for _, w := range e.workers { + multishot = w.bufRing != nil + deferTR = w.tier.SetupFlags()&setupDeferTaskrun != 0 + } + if (tier != hijackTierDefault) != multishot { + skipOrFail656(t, "celeris#685 hijack arm: multishot recv is %v on the worker, want %v", multishot, tier != hijackTierDefault) + } + + recvtheft.SetHijackHold(hijack685Hold) + defer recvtheft.SetHijackHold(0) + holds0 := recvtheft.HijackHolds() + owed0 := recvtheft.HijackWithOpOwed() + stale0 := e.metrics.handoffLoss.staleRecvDataClosed.Load() + + c, err := net.DialTimeout("tcp", addr, time.Second) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer func() { _ = c.Close() }() + if _, err := c.Write([]byte("GET /hijack HTTP/1.1\r\nHost: recv-theft-685\r\n\r\n")); err != nil { + t.Fatalf("write request: %v", err) + } + // The payload goes out while the worker is held after the hijack, before + // its next enter. + for deadline := time.Now().Add(2 * time.Second); recvtheft.HijackHolds() == holds0; { + if time.Now().After(deadline) { + t.Fatal("the hijack hold never ran: the request did not reach Hijack") + } + time.Sleep(time.Millisecond) + } + time.Sleep(20 * time.Millisecond) + if _, err := c.Write([]byte(hijack685Payload)); err != nil { + t.Fatalf("write payload: %v", err) + } + var hc net.Conn + select { + case hc = <-h.conns: + case <-time.After(3 * time.Second): + t.Fatal("the handler never returned from Hijack") + } + if hc == nil { + t.Fatal("Hijack failed") + } + defer func() { _ = hc.Close() }() + _ = hc.SetReadDeadline(time.Now().Add(2 * time.Second)) + buf := make([]byte, 256) + var got []byte + for !strings.Contains(string(got), hijack685Payload) { + n, err := hc.Read(buf) + got = append(got, buf[:n]...) + if err != nil { + break + } + } + time.Sleep(50 * time.Millisecond) + owed := recvtheft.HijackWithOpOwed() - owed0 + stale := e.metrics.handoffLoss.staleRecvDataClosed.Load() - stale0 + read := strings.Contains(string(got), hijack685Payload) + t.Logf("RECVTHEFT685 hijack result tier=%d multishot=%v defer_taskrun=%v op_owed=+%d stale_recv_data_closed=+%d hijacker_read=%v got=%q", + tier, multishot, deferTR, owed, stale, read, got) + if tier == hijackTierDefault && owed != 0 { + t.Errorf("a single-shot hijack found an op owed (+%d): the recv that brought the request should have completed", owed) + } + if tier != hijackTierDefault && owed != 1 { + t.Errorf("the multishot hijack found no op owed (+%d): the arm did not reach its precondition", owed) + } + if !read || stale != 0 { + t.Fatalf("the hijacker lost its first bytes: read=%v (got %q), stale recv data on the closed identity +%d", read, got, stale) + } +} + +// TestRecvTheft685HijackSingleShot: nothing is owed at a single-shot hijack. +func TestRecvTheft685HijackSingleShot(t *testing.T) { runHijack685(t, hijackTierDefault) } + +// TestRecvTheft685HijackMultishotDefer: owed, and the queued cancel still +// wins, because the recv's completion waits for the enter. +func TestRecvTheft685HijackMultishotDefer(t *testing.T) { runHijack685(t, hijackTierMultiDefer) } + +// TestRecvTheft685HijackMultishotCoop: owed, and without DEFER_TASKRUN the +// recv's completion runs at the worker's next syscall return, before the +// enter that would submit the cancel queued by hijackConn. +func TestRecvTheft685HijackMultishotCoop(t *testing.T) { runHijack685(t, hijackTierMultiCoop) } diff --git a/engine/iouring/recv_theft_685_linked_linux_test.go b/engine/iouring/recv_theft_685_linked_linux_test.go new file mode 100644 index 00000000..dd6a82de --- /dev/null +++ b/engine/iouring/recv_theft_685_linked_linux_test.go @@ -0,0 +1,420 @@ +//go:build linux && validation + +package iouring + +import ( + "context" + "errors" + "io" + "log/slog" + "net" + "strconv" + "strings" + "testing" + "time" + + "golang.org/x/sys/unix" + + "github.com/goceleris/celeris/engine" + "github.com/goceleris/celeris/internal/recvtheft" + "github.com/goceleris/celeris/protocol/h2/stream" + "github.com/goceleris/celeris/resource" +) + +// celeris#685, the linked form of the celeris#715 theft, made deterministic. +// +// A sync-mode HTTP/1 response is flushed with its connection's next recv +// chained behind the SEND (flushSendLink, IOSQE_IO_LINK). The kernel consumes +// both SQEs at the submit, but it issues the recv, and resolves its descriptor +// NUMBER, only when the SEND completes: on a DEFER_TASKRUN ring as task work, +// which the enter that posts the SEND's CQE can leave queued. A close path +// that runs at that CQE (completeSend of a connection closeConn deferred while +// the SEND was in flight) queues a cancel that cannot find the recv (it is in +// no cancel table yet) and closed the descriptor at once. If a sibling worker +// is given the number before the closer's next enter, that enter issues the +// recv against the new connection's socket. +// +// The trial: +// 1. A sends half a request (TCP_DEFER_ACCEPT holds the accept until it +// does). The test finds A's server-side descriptor N and fills its send +// queue from outside the engine (A never reads), so the next SEND cannot +// complete. +// 2. A sends the rest. The worker W1 answers with a linked SEND+recv; the +// SEND waits for room. +// 3. WriteTimeout fires (once A's header timer has woken W1, see +// linkedHeaderTimeout): checkTimeouts' closeConn finds the SEND in +// flight and defers the close. +// 4. A drains its socket. The SEND completes, and completeSend runs the +// deferred close with the linked recv owed; W1 parks when the close path +// returns (recvtheft.Options.LinkedRecv). +// 5. The test dials B's candidates until the sibling W2's accept of one +// tests the theft (theftHit): one given N, because the close released it +// (W2 then parks before submitting B's recv), or any one while N is still +// A's socket. An accept given a lower free number while N is free is +// skipped. TestRecvTheft685LinkedHole opens such a hole on purpose. +// 6. W1 is released (its next enter runs the queued task work, which issues +// the linked recv), then W2. +// +// Asserts what the fix restores: B answered, no stale recv carrying B's +// bytes, and a number kept open at the close released after. Fails on a tree +// that closes the number at once whenever the kernel left the recv unissued +// at the SEND's CQE (see TestRecvTheft685Linked's log: reused, stolen). +// Needs two io_uring workers and CELERIS_RECV_THEFT_715=1, like the #715 arms. + +type linkedTheftHandler struct{} + +func (linkedTheftHandler) HandleStream(_ context.Context, s *stream.Stream) error { + if s.ResponseWriter == nil { + return nil + } + return s.ResponseWriter.WriteResponse(s, 200, + [][2]string{{"content-type", "text/plain"}, {"content-length", "2"}}, []byte("ok")) +} + +const ( + linkedWriteTimeout = 200 * time.Millisecond + // linkedHeaderTimeout is what wakes W1 once its SEND is blocked. A + // worker whose only op is that SEND waits in the ring with no timeout + // (run's mode 3a: a SEND is expected to complete), so its timeout sweep + // does not run until some completion arrives. A's header timer, armed + // by its half request, is that completion: it fires this long after A's + // first bytes, finds the headers complete and does nothing, and from + // the next iteration on the worker waits with a timeout and sweeps. + linkedHeaderTimeout = 1 * time.Second + // linkedCloseWait covers the header timer, WriteTimeout, and the timeout + // sweep's cadence (every 32 iterations, each at most 25 ms with + // ReadHeaderTimeout set). + linkedCloseWait = 2500 * time.Millisecond + linkedReqHead = "GET /a HTTP/1.1\r\n" + linkedReqTail = "Host: recv-theft-685\r\n\r\n" +) + +func startLinkedTheftEngine(t *testing.T) (*Engine, int) { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("pick port: %v", err) + } + addr := ln.Addr().String() + port := ln.Addr().(*net.TCPAddr).Port + _ = ln.Close() + e, err := New(resource.Config{ + Addr: addr, + Protocol: engine.HTTP1, + Resources: resource.Resources{Workers: 2}, + WriteTimeout: linkedWriteTimeout, + ReadHeaderTimeout: linkedHeaderTimeout, + Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + }, linkedTheftHandler{}) + if err != nil { + skipOrFail656(t, "iouring engine unavailable: %v", err) + } + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { done <- e.Listen(ctx) }() + t.Cleanup(func() { + cancel() + select { + case <-done: + case <-time.After(5 * time.Second): + t.Error("engine did not stop within 5s") + } + }) + for deadline := time.Now().Add(8 * time.Second); e.Addr() == nil; { + select { + case err := <-done: + skipOrFail656(t, "iouring engine failed to start: %v", err) + default: + } + if time.Now().After(deadline) { + t.Fatal("engine did not start listening within 8s") + } + time.Sleep(10 * time.Millisecond) + } + if n := e.NumWorkers(); n < 2 { + skipOrFail656(t, "celeris#685 needs a sibling io_uring worker: workers=%d (RLIMIT_MEMLOCK funds one per 12 MiB)", n) + } + return e, port +} + +// fillSendQueue writes to fd (a server-side socket whose peer never reads) +// until two passes 50 ms apart add nothing: the peer's window is closed and +// the send queue is full. Returns the bytes written. +func fillSendQueue(fd int) int { + junk := make([]byte, 64<<10) + total, idle := 0, 0 + for idle < 2 { + wrote := 0 + for { + n, err := unix.SendmsgN(fd, junk, nil, nil, unix.MSG_DONTWAIT|unix.MSG_NOSIGNAL) + if n > 0 { + wrote += n + } + if err != nil { + break + } + } + total += wrote + if wrote == 0 { + idle++ + } else { + idle = 0 + } + time.Sleep(50 * time.Millisecond) + } + return total +} + +type linkedTheftResult struct { + attempts int + hit bool + fd, closer, sibl int + bFD int + reused bool + heldOpen bool + heldByA bool + released bool + hole int + missHole int + accepts0 uint64 + dialed int + filled int + witness uint64 + staleClosed uint64 + stale []recvtheft.StaleRecv + stolen bool + answered bool + answerErr error + missNoClose int + missQueued int + missCloserAccepts int +} + +func linkedTheftAttempt(t *testing.T, e *Engine, sa *unix.SockaddrInet4, arm string, hole bool, r *linkedTheftResult) bool { + waitEngineQuiet(t, e, r.accepts0, r.dialed) + fillFDHoles(t) + var cands []int + defer func() { + for _, fd := range cands { + _ = unix.Close(fd) + } + }() + for range theftCandidates { + fd, err := unix.Socket(unix.AF_INET, unix.SOCK_STREAM|unix.SOCK_CLOEXEC, 0) + if err != nil { + t.Fatalf("socket: %v", err) + } + cands = append(cands, fd) + } + // A: a small receive buffer, so the fill below stays small. + a, err := unix.Socket(unix.AF_INET, unix.SOCK_STREAM|unix.SOCK_CLOEXEC, 0) + if err != nil { + t.Fatalf("socket A: %v", err) + } + defer func() { _ = unix.Close(a) }() + _ = unix.SetsockoptInt(a, unix.SOL_SOCKET, unix.SO_RCVBUF, 4096) + + witness0 := recvtheft.CloseWithLinkedRecv() + stale0 := e.metrics.handoffLoss.staleRecvDataClosed.Load() + tr := recvtheft.Arm(recvtheft.Options{LinkedRecv: true, HoldMax: 10 * time.Second}) + defer tr.Disarm() + + if err := unix.Connect(a, sa); err != nil { + t.Fatalf("connect A: %v", err) + } + r.dialed++ + if _, err := unix.Write(a, []byte(linkedReqHead)); err != nil { + t.Fatalf("write A head: %v", err) + } + aLocalSA, err := unix.Getsockname(a) + if err != nil { + t.Fatalf("getsockname A: %v", err) + } + aLocal := sockaddrString(aLocalSA) + srv := -1 + for deadline := time.Now().Add(2 * time.Second); srv < 0 && time.Now().Before(deadline); { + if srv = serverFDFor(aLocal); srv < 0 { + time.Sleep(2 * time.Millisecond) + } + } + if srv < 0 { + t.Fatalf("A (%s) was not accepted within 2s", aLocal) + } + // The search's own /proc/self/fd reads can leave a hole under N (a read + // that took the lowest number while the accept was still to come); fill + // it, so the number A's close frees is again the lowest free one. + fillFDHoles(t) + r.filled = fillSendQueue(srv) + if _, err := unix.Write(a, []byte(linkedReqTail)); err != nil { + t.Fatalf("write A tail: %v", err) + } + // The response's SEND now waits for room, with the next recv linked + // behind it; WriteTimeout's closeConn defers the close meanwhile. + closes0 := e.metrics.closeCount.Load() + time.Sleep(linkedCloseWait) + outqBefore, _ := unix.IoctlGetInt(srv, unix.SIOCOUTQ) + // Drain A until the close hold fires: the SEND completes, and the + // deferred close runs at its CQE. + drained := make(chan struct{}) + go func() { + defer close(drained) + buf := make([]byte, 64<<10) + tv := unix.NsecToTimeval(int64(20 * time.Millisecond)) + _ = unix.SetsockoptTimeval(a, unix.SOL_SOCKET, unix.SO_RCVTIMEO, &tv) + for end := time.Now().Add(3 * time.Second); time.Now().Before(end); { + n, err := unix.Read(a, buf) + if n == 0 && err == nil { + return + } + if err != nil && !errors.Is(err, unix.EAGAIN) && !errors.Is(err, unix.EINTR) { + return + } + } + }() + ce, ok := tr.WaitClose(4 * time.Second) + if !ok { + r.missNoClose++ + <-drained + outqAfter, _ := unix.IoctlGetInt(srv, unix.SIOCOUTQ) + t.Logf("RECVTHEFT685 %s attempt=%d miss=no-close-hold filled=%d outq_before_drain=%d outq_after=%d srv_after=%q closes=+%d witness=+%d", + arm, r.attempts, r.filled, outqBefore, outqAfter, fdTarget(srv), e.metrics.closeCount.Load()-closes0, recvtheft.CloseWithLinkedRecv()-witness0) + return false + } + aTarget := fdTarget(ce.FD) + r.heldOpen = strings.HasPrefix(aTarget, "socket:") + r.heldByA = r.heldOpen && namesPeer(ce.FD, aLocal) + if hole { + // A hole under N, as in TestRecvTheft715ArmAHole: the last candidate, + // never dialed, was allocated before A's server-side descriptor. + r.hole = cands[len(cands)-1] + cands = cands[:len(cands)-1] + if r.hole > ce.FD { + t.Fatalf("the hole candidate's number %d is not below A's %d", r.hole, ce.FD) + } + _ = unix.Close(r.hole) + } + + var hit *recvtheft.AcceptEvent + var hitFD int + for i, fd := range cands { + if err := unix.Connect(fd, sa); err != nil { + t.Fatalf("connect candidate %d: %v", i, err) + } + r.dialed++ + if _, err := unix.Write(fd, []byte(theftRequestB)); err != nil { + t.Fatalf("write candidate %d: %v", i, err) + } + ev, ok := tr.NextAccept(theftAcceptWait) + if !ok { + r.missQueued++ + continue + } + if ev.Worker == ce.Worker { + r.missCloserAccepts++ + continue + } + if !theftHit(ev.FD, ce.FD, r.heldByA) { + r.missHole++ + t.Logf("RECVTHEFT685 %s attempt=%d candidate=%d accepted by the sibling %d as fd %d, a hole below A's released %d: skipped", arm, r.attempts, i, ev.Worker, ev.FD, ce.FD) + continue + } + ev0 := ev + if ev.FD == ce.FD { + held, ok := tr.WaitAcceptHeld(time.Second) + if !ok { + t.Fatalf("accept of fd %d by worker %d reported but its hold did not fire", ev.FD, ev.Worker) + } + ev0 = held + r.reused = true + } + hit, hitFD = &ev0, fd + break + } + if hit == nil { + t.Logf("RECVTHEFT685 %s attempt=%d miss=no-sibling-accept fd=%d closer=%d held_by_a=%v queued=%d closer_accepts=%d hole_accepts=%d", + arm, r.attempts, ce.FD, ce.Worker, r.heldByA, r.missQueued, r.missCloserAccepts, r.missHole) + tr.Disarm() + <-drained + return false + } + r.hit, r.fd, r.closer, r.sibl, r.bFD = true, ce.FD, ce.Worker, hit.Worker, hit.FD + + tr.ReleaseClose() + for deadline := time.Now().Add(theftSubmitWait); time.Now().Before(deadline) && len(tr.Stale()) == 0; { + time.Sleep(time.Millisecond) + } + tr.ReleaseAccept() + answer, aerr := readHead(hitFD, theftAnswerWait) + r.answered, r.answerErr = strings.HasPrefix(answer, "HTTP/1.1 200"), aerr + for deadline := time.Now().Add(theftReleaseWait); ; { + if cur := fdTarget(ce.FD); !r.heldOpen || cur != aTarget { + r.released = true + break + } + if time.Now().After(deadline) { + break + } + time.Sleep(time.Millisecond) + } + <-drained + r.witness = recvtheft.CloseWithLinkedRecv() - witness0 + r.staleClosed = e.metrics.handoffLoss.staleRecvDataClosed.Load() - stale0 + r.stale = tr.Stale() + for _, s := range r.stale { + if s.FD == r.fd && int(s.Res) == len(theftRequestB) && len(s.Head) > 0 && + string(s.Head) == theftRequestB[:len(s.Head)] { + r.stolen = true + } + } + return true +} + +// TestRecvTheft685Linked is the linked form's trial (see above). One trial +// per run; tally the --- lines of -count=N. +func TestRecvTheft685Linked(t *testing.T) { runLinkedTheft(t, "linked", false) } + +// TestRecvTheft685LinkedHole is the linked trial with a hole opened under A's +// number once the closer is parked, as TestRecvTheft715ArmAHole does for arm +// A: on a tree that releases the number at the close it must fail exactly as +// the linked trial does. +func TestRecvTheft685LinkedHole(t *testing.T) { runLinkedTheft(t, "linked-hole", true) } + +func runLinkedTheft(t *testing.T, arm string, hole bool) { + requireRecvTheft715(t) + e, port := startLinkedTheftEngine(t) + sa := &unix.SockaddrInet4{Port: port, Addr: [4]byte{127, 0, 0, 1}} + r := linkedTheftResult{hole: -1, accepts0: e.metrics.acceptCount.Load()} + for r.attempts < theftMaxAttempts { + r.attempts++ + if linkedTheftAttempt(t, e, sa, arm, hole, &r) { + break + } + } + var heads []string + for _, s := range r.stale { + heads = append(heads, strconv.Quote(string(s.Head))) + } + errText := "" + if r.answerErr != nil { + errText = r.answerErr.Error() + } + t.Logf("RECVTHEFT685 %s result attempts=%d hit=%v fd=%d closer=%d sibling=%d b_fd=%d reused=%v held_open=%v held_by_a=%v released=%v hole=%d filled=%d witness=+%d stale_recv_data_closed=+%d stolen=%v answered=%v answer_err=%q stale=[%s] misses{no_close=%d queued=%d closer_accepts=%d hole_accepts=%d}", + arm, r.attempts, r.hit, r.fd, r.closer, r.sibl, r.bFD, r.reused, r.heldOpen, r.heldByA, r.released, r.hole, r.filled, r.witness, r.staleClosed, + r.stolen, r.answered, errText, strings.Join(heads, " "), r.missNoClose, r.missQueued, r.missCloserAccepts, r.missHole) + if !r.hit { + skipOrFail656(t, "INCONCLUSIVE: no sibling accept that tests the theft while the closer was parked, in %d attempts", r.attempts) + } + if !r.reused && !r.heldByA { + t.Fatalf("a hit that tests nothing: B was given %d, A's number %d was neither reused nor kept as A's socket (see theftHit)", r.bFD, r.fd) + } + if r.witness == 0 { + t.Fatalf("the close hold fired but close_with_linked_recv did not move") + } + if r.stolen || !r.answered { + t.Fatalf("B (fd %d on worker %d; A's number %d reused=%v, kept open at the close=%v) lost its request: stolen by A's linked recv=%v (stale_recv_data_closed +%d), answered=%v (%v)", + r.bFD, r.sibl, r.fd, r.reused, r.heldOpen, r.stolen, r.staleClosed, r.answered, r.answerErr) + } + if !r.released { + t.Fatalf("A's number %d, kept open at the close, still names A's socket %v after the closer's release: the kept descriptor leaked", r.fd, theftReleaseWait) + } +} diff --git a/engine/iouring/recv_theft_715_linux_test.go b/engine/iouring/recv_theft_715_linux_test.go new file mode 100644 index 00000000..e016879d --- /dev/null +++ b/engine/iouring/recv_theft_715_linux_test.go @@ -0,0 +1,629 @@ +//go:build linux && validation + +package iouring + +import ( + "bytes" + "context" + "errors" + "fmt" + "io" + "log/slog" + "net" + "os" + "strconv" + "strings" + "sync" + "testing" + "time" + + "golang.org/x/sys/unix" + + "github.com/goceleris/celeris/engine" + "github.com/goceleris/celeris/internal/recvtheft" + "github.com/goceleris/celeris/protocol/h2/stream" + "github.com/goceleris/celeris/resource" +) + +// celeris#715 hypothesis (a), the celeris#685 class, made deterministic. +// +// The io_uring worker prepares a recv SQE that reaches the kernel only at its +// next submit, and the SQE names the descriptor NUMBER (fixed files are off). +// finishClose / finishCloseDetached queue the recv's cancel behind it and close +// the descriptor at once. The claim under test: if a SIBLING worker's accept is +// given the freed number before this worker submits, the old recv reads the new +// connection's request (stale_recv_data_closed), and the new connection, whose +// own recv then finds an empty socket, is never answered. +// +// The theft needs one ordering: the sibling's accept after the close, and the +// closing worker's submit before the sibling's submit of its own recv. Two +// holds (internal/recvtheft, -tags=validation only) force it: +// - W1, the worker that owns connection A, is parked right after closing A's +// descriptor N with A's recv still unsubmitted (recvtheft.HoldAfterClose); +// - W2, the other worker, is parked right after it accepted B as N and +// prepared B's recv, before it submits (recvtheft.AfterAccept); +// - W1 is released (it submits the old recv), then W2. +// +// A reaches "recv prepared, then closed in the same loop iteration" through the +// engine's own async path: A's first request is on an async route, so the +// inline parse bails, promoteConnToAsync starts the dispatch goroutine and +// re-arms A's recv; the request asks for Connection: close and the handler +// writes nothing, so the goroutine sets asyncClosed with nothing to flush and +// queues A; drainDetachQueue closes A (finishCloseDetached). The one test-only +// step is recvtheft.Options.PromoteGate: the worker waits after the re-arm +// until the goroutine has queued, so the close lands in the same iteration +// instead of racing it. +// +// Arms (one trial per test run; tally the --- lines of -count=N): +// - A (TestRecvTheft715ArmA): the tree as it is. Asserts the property the +// fix must restore: B is answered and no stale recv read B's bytes. +// FAILS on main by design when hypothesis (a) holds. Not in CI; the +// celeris#685 fix enables it. +// - control (TestRecvTheft715Control): the same trial with +// recvtheft.Options.SubmitBeforeClose, the close paths submitting before +// they close (the fix direction). Asserts the same property. +// - A with a hole (TestRecvTheft715ArmAHole): arm A with one of B's +// candidate sockets, whose number is below A's, closed once the closer is +// parked, so a free number lies under the one A's close frees. The +// sibling's first accept is given the hole; it tests nothing (see +// theftHit) and is skipped, and the next one is given A's number if the +// close released it. Pins that the verdict cannot pass a tree without +// the fix because a hole took B (celeris#793 review round 2). +// - C (TestRecvTheft715ArmC): hypothesis (c), the promoted connection's +// hand-off to its dispatch goroutine, with the window between the +// worker's asyncInMu unlock and its Signal / goroutine start widened +// (recvtheft.SetWakeHold). Asserts every request is answered. +// +// All three need two io_uring workers (so RLIMIT_MEMLOCK of at least 24 MiB) +// and run only with CELERIS_RECV_THEFT_715=1 in a -tags=validation build. + +const envRecvTheft715 = "CELERIS_RECV_THEFT_715" + +func requireRecvTheft715(t *testing.T) { + t.Helper() + if os.Getenv(envRecvTheft715) != "1" { + t.Skipf("celeris#715 recv-theft measurement: set %s=1 to run (arm A fails on main by design)", envRecvTheft715) + } +} + +// recvTheftHandler answers every path with "ok" except /close, which writes +// nothing. /close and /async are async routes. +type recvTheftHandler struct{} + +func (recvTheftHandler) HandleStream(_ context.Context, s *stream.Stream) error { + if s.ResponseWriter == nil || s.Path == "/close" { + return nil + } + return s.ResponseWriter.WriteResponse(s, 200, + [][2]string{{"content-type", "text/plain"}, {"content-length", "2"}}, []byte("ok")) +} +func (recvTheftHandler) RouteAsync(_, path string) bool { return path == "/close" || path == "/async" } +func (recvTheftHandler) HasAsyncRoutes() bool { return true } + +// startRecvTheftEngine runs an async-handler io_uring engine with two workers +// on a free loopback port and returns it with its port. +func startRecvTheftEngine(t *testing.T) (*Engine, int) { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("pick port: %v", err) + } + addr := ln.Addr().String() + port := ln.Addr().(*net.TCPAddr).Port + _ = ln.Close() + e, err := New(resource.Config{ + Addr: addr, + Protocol: engine.HTTP1, + Resources: resource.Resources{Workers: 2}, + AsyncHandlers: true, + Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + }, recvTheftHandler{}) + if err != nil { + skipOrFail656(t, "iouring engine unavailable: %v", err) + } + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { done <- e.Listen(ctx) }() + t.Cleanup(func() { + cancel() + select { + case <-done: + case <-time.After(5 * time.Second): + t.Error("engine did not stop within 5s") + } + }) + for deadline := time.Now().Add(8 * time.Second); e.Addr() == nil; { + select { + case err := <-done: + skipOrFail656(t, "iouring engine failed to start: %v", err) + default: + } + if time.Now().After(deadline) { + t.Fatal("engine did not start listening within 8s") + } + time.Sleep(10 * time.Millisecond) + } + n := e.NumWorkers() + t.Logf("RECVTHEFT715 engine workers=%d", n) + if n < 2 { + skipOrFail656(t, "celeris#715 needs a sibling io_uring worker: workers=%d (RLIMIT_MEMLOCK funds one per 12 MiB)", n) + } + return e, port +} + +// fillFDHoles opens /dev/null until the kernel hands out a number above every +// descriptor this process has open, so the next descriptors are allocated at +// the top and a number freed later is the lowest free one. The fillers are +// closed when the test ends. +func fillFDHoles(t *testing.T) { + t.Helper() + ents, err := os.ReadDir("/proc/self/fd") + if err != nil { + t.Fatalf("read /proc/self/fd: %v", err) + } + top := -1 + for _, ent := range ents { + if fd, err := strconv.Atoi(ent.Name()); err == nil && fd > top { + top = fd + } + } + var fillers []int + for { + fd, err := unix.Open("/dev/null", unix.O_RDONLY|unix.O_CLOEXEC, 0) + if err != nil { + t.Fatalf("open /dev/null: %v", err) + } + if fd > top { + _ = unix.Close(fd) + break + } + fillers = append(fillers, fd) + } + t.Cleanup(func() { + for _, fd := range fillers { + _ = unix.Close(fd) + } + }) +} + +// waitEngineQuiet waits, before an attempt fills the descriptor holes, until +// the engine has accepted every connection the trial has dialed (dialed, since +// acceptCount read accepts0) and holds none of them. A candidate that a missed +// attempt left in the closer's accept queue is accepted only once the closer +// is released; its close after the fill opened a hole under the next +// attempt's number, and the holes cascaded into further misses (the round-2 +// smoke on the negative control: 4 such accepts skipped in one attempt). +// Holes cannot make a trial pass (theftHit), so a late accept past the +// deadline is logged, not fatal; a connection still open is fatal as before. +func waitEngineQuiet(t *testing.T, e *Engine, accepts0 uint64, dialed int) { + t.Helper() + for deadline := time.Now().Add(3 * time.Second); ; { + acc := e.metrics.acceptCount.Load() - accepts0 + act := e.metrics.activeConns.Load() + if acc >= uint64(dialed) && act == 0 { + return + } + if time.Now().After(deadline) { + if act != 0 { + t.Fatalf("engine still holds %d connections from the previous attempt", act) + } + t.Logf("RECVTHEFT the engine accepted %d of the %d connections dialed so far; going on", acc, dialed) + return + } + time.Sleep(5 * time.Millisecond) + } +} + +// readHead reads from a blocking socket until the response head is complete, +// the peer closes, or d passes. +func readHead(fd int, d time.Duration) (string, error) { + tv := unix.NsecToTimeval(int64(50 * time.Millisecond)) + _ = unix.SetsockoptTimeval(fd, unix.SOL_SOCKET, unix.SO_RCVTIMEO, &tv) + var got []byte + buf := make([]byte, 4096) + for deadline := time.Now().Add(d); time.Now().Before(deadline); { + n, err := unix.Read(fd, buf) + if n > 0 { + got = append(got, buf[:n]...) + if bytes.Contains(got, []byte("\r\n\r\n")) { + return string(got), nil + } + continue + } + if errors.Is(err, unix.EAGAIN) || errors.Is(err, unix.EINTR) { + continue + } + if err != nil { + return string(got), err + } + return string(got), io.EOF + } + return string(got), fmt.Errorf("no response head within %v", d) +} + +// theftResult is one trial of arm A, A with a hole, or the control. +// +// A hit is a sibling accept of one of B's candidates, while the closer is +// parked after its close, that tests the theft (theftHit). On a tree that +// frees the number at the close, that is the accept given the number: reused +// is true, and the sibling is parked too, with B's recv prepared. On a tree +// that keeps the number allocated until the owed recv has ended +// (celeris#685), heldByA is true, the accept is given another number (reused +// is false), and B is served with no hold at all. A sibling accept given +// another number while A's number was free filled a lower hole: it is +// skipped (missHole) and the next candidate is dialed. heldOpen, heldByA and +// released read the closer's descriptor: whether the number still named a +// socket while the closer was parked, whether that socket was A's (its peer +// is A's local address), and whether it was released after (closed, or +// reused by something else) within theftReleaseWait of the closer's release, +// so a kept descriptor is not a leak. +type theftResult struct { + attempts int + hit bool + fd int + closer, sibl int + bFD int + reused bool + heldOpen bool + heldByA bool + released bool + hole int + missHole int + accepts0 uint64 + dialed int + gate bool + witness uint64 + staleClosed uint64 + stale []recvtheft.StaleRecv + stolen bool + answered bool + answer string + answerErr error + missNoClose int + missQueued int + missOtherFD int + candidatesHit int +} + +const ( + theftMaxAttempts = 10 + theftCandidates = 6 + theftAcceptWait = 300 * time.Millisecond + theftSubmitWait = 300 * time.Millisecond + theftAnswerWait = 2 * time.Second + theftReleaseWait = 2 * time.Second + theftClosePath = "/close" + theftRequestAHead = "GET " + theftClosePath + " HTTP/1.1\r\nHost: recv-theft-715\r\nConnection: close\r\n\r\n" + theftRequestB = "GET /b HTTP/1.1\r\nHost: recv-theft-715\r\n\r\n" +) + +// theftArm is one arm of the trial: its name in the result line, whether the +// close paths submit before they close (the control), and whether a hole is +// opened under A's number once the closer is parked (A with a hole). +type theftArm struct { + name string + submitBeforeClose bool + hole bool +} + +// theftHit reports whether the sibling's accept of a candidate as number +// accepted tests the theft of A's number n. It does when the accept was given +// n (the close released it, and B's recv is the one a stale recv can rob), or +// when n was still A's socket while the closer was parked (the close kept it, +// so no accept could be given it, and B must be served). An accept given +// another number while n was free tests nothing: it filled a hole below n, +// which is still free, so B is served with no theft possible on any tree. +// Counting it as a hit passed a tree without the fix (celeris#793 review +// round 2: 1 of 101 trials, b_fd=13 under fd=27). The caller skips it and +// dials the next candidate, whose accept can be given n. +func theftHit(accepted, n int, heldByA bool) bool { + return accepted == n || heldByA +} + +// runRecvTheftTrial drives attempts until the sibling worker's accept of a +// fresh connection B tests the theft (theftHit), then releases the closer, +// then the sibling, and reports what happened to B's request. +func runRecvTheftTrial(t *testing.T, arm theftArm) theftResult { + e, port := startRecvTheftEngine(t) + sa := &unix.SockaddrInet4{Port: port, Addr: [4]byte{127, 0, 0, 1}} + r := theftResult{hole: -1, accepts0: e.metrics.acceptCount.Load()} + for r.attempts < theftMaxAttempts { + r.attempts++ + if done := recvTheftAttempt(t, e, sa, arm, &r); done { + return r + } + } + return r +} + +func recvTheftAttempt(t *testing.T, e *Engine, sa *unix.SockaddrInet4, arm theftArm, r *theftResult) bool { + waitEngineQuiet(t, e, r.accepts0, r.dialed) + fillFDHoles(t) + // B's candidate sockets exist before A is accepted, so they hold numbers + // below A's and none of them can take the number A's close frees. + var cands []int + defer func() { + for _, fd := range cands { + _ = unix.Close(fd) + } + }() + for range theftCandidates { + fd, err := unix.Socket(unix.AF_INET, unix.SOCK_STREAM|unix.SOCK_CLOEXEC, 0) + if err != nil { + t.Fatalf("socket: %v", err) + } + cands = append(cands, fd) + } + + witness0 := recvtheft.CloseWithUnsubmittedRecv() + stale0 := e.metrics.handoffLoss.staleRecvDataClosed.Load() + tr := recvtheft.Arm(recvtheft.Options{ + SubmitBeforeClose: arm.submitBeforeClose, + PromoteGate: 2 * time.Second, + HoldMax: 10 * time.Second, + }) + defer tr.Disarm() + + a, err := net.DialTimeout("tcp", sockaddrString(sa), time.Second) + if err != nil { + t.Fatalf("dial A: %v", err) + } + r.dialed++ + defer func() { _ = a.Close() }() + if _, err := a.Write([]byte(theftRequestAHead)); err != nil { + t.Fatalf("write A: %v", err) + } + ce, ok := tr.WaitClose(3 * time.Second) + if !ok { + r.missNoClose++ + t.Logf("RECVTHEFT715 arm=%s attempt=%d miss=no-close-hold gate=%v witness=+%d", arm.name, r.attempts, tr.GateUsed(), + recvtheft.CloseWithUnsubmittedRecv()-witness0) + return false + } + + // W1 is parked right after its close path, with A's recv unsubmitted. + // Did that close release N? Read it before any candidate is dialed: the + // number still names A's socket (its peer is A's local address), or it + // was released. + aTarget := fdTarget(ce.FD) + r.heldOpen = strings.HasPrefix(aTarget, "socket:") + r.heldByA = r.heldOpen && namesPeer(ce.FD, a.LocalAddr().String()) + if arm.hole { + // Open a hole under N: close the last candidate, never dialed, whose + // number was allocated before A's. + r.hole = cands[len(cands)-1] + cands = cands[:len(cands)-1] + if r.hole > ce.FD { + t.Fatalf("the hole candidate's number %d is not below A's %d", r.hole, ce.FD) + } + _ = unix.Close(r.hole) + } + + // Dial B candidates until the sibling's accept of one tests the theft + // (theftHit). One that hashes to W1's listener waits in W1's accept + // queue. The sibling's accept is given the lowest free number: N if the + // close released N and no hole lies below it, and then the sibling is + // parked too (its recv for B prepared, not submitted); a hole below N if + // there is one, and then that B is served, N is still free, and the next + // candidate is dialed; another number if N is still A's, and then B is + // served with no hold. + var hit *recvtheft.AcceptEvent + var hitFD int + for i, fd := range cands { + if err := unix.Connect(fd, sa); err != nil { + t.Fatalf("connect candidate %d: %v", i, err) + } + r.dialed++ + if _, err := unix.Write(fd, []byte(theftRequestB)); err != nil { + t.Fatalf("write candidate %d: %v", i, err) + } + ev, ok := tr.NextAccept(theftAcceptWait) + if !ok { + r.missQueued++ + continue + } + if ev.Worker == ce.Worker { + r.missOtherFD++ + t.Logf("RECVTHEFT715 arm=%s attempt=%d candidate=%d accepted by the closer %d as fd %d (target %d)", arm.name, r.attempts, i, ev.Worker, ev.FD, ce.FD) + continue + } + if !theftHit(ev.FD, ce.FD, r.heldByA) { + r.missHole++ + t.Logf("RECVTHEFT715 arm=%s attempt=%d candidate=%d accepted by the sibling %d as fd %d, a hole below A's released %d: skipped", arm.name, r.attempts, i, ev.Worker, ev.FD, ce.FD) + continue + } + ev0 := ev + if ev.FD == ce.FD { + held, ok := tr.WaitAcceptHeld(time.Second) + if !ok { + t.Fatalf("accept of fd %d by worker %d reported but its hold did not fire", ev.FD, ev.Worker) + } + ev0 = held + r.reused = true + } + hit, hitFD = &ev0, fd + r.candidatesHit = i + 1 + break + } + if hit == nil { + t.Logf("RECVTHEFT715 arm=%s attempt=%d miss=no-sibling-accept fd=%d closer=%d held_by_a=%v queued=%d closer_accepts=%d hole_accepts=%d", + arm.name, r.attempts, ce.FD, ce.Worker, r.heldByA, r.missQueued, r.missOtherFD, r.missHole) + return false + } + r.hit, r.fd, r.closer, r.sibl, r.bFD, r.gate = true, ce.FD, ce.Worker, hit.Worker, hit.FD, tr.GateUsed() + + // Release W1: its next submit issues whatever it still holds for N. + tr.ReleaseClose() + for deadline := time.Now().Add(theftSubmitWait); time.Now().Before(deadline) && len(tr.Stale()) == 0; { + time.Sleep(time.Millisecond) + } + // Then the sibling, if parked: it submits B's own recv. + tr.ReleaseAccept() + r.answer, r.answerErr = readHead(hitFD, theftAnswerWait) + r.answered = strings.HasPrefix(r.answer, "HTTP/1.1 200") + // A number kept open at the close must be released once A's recv has + // ended: closed, or by now naming something else. + for deadline := time.Now().Add(theftReleaseWait); ; { + if cur := fdTarget(ce.FD); !r.heldOpen || cur != aTarget { + r.released = true + break + } + if time.Now().After(deadline) { + break + } + time.Sleep(time.Millisecond) + } + + r.witness = recvtheft.CloseWithUnsubmittedRecv() - witness0 + r.staleClosed = e.metrics.handoffLoss.staleRecvDataClosed.Load() - stale0 + r.stale = tr.Stale() + for _, s := range r.stale { + if s.FD == r.fd && int(s.Res) == len(theftRequestB) && len(s.Head) > 0 && + bytes.Equal(s.Head, []byte(theftRequestB)[:len(s.Head)]) { + r.stolen = true + } + } + return true +} + +func logTheftResult(t *testing.T, arm string, r theftResult) { + t.Helper() + var heads []string + for _, s := range r.stale { + heads = append(heads, fmt.Sprintf("{worker=%d fd=%d gen=%d res=%d head=%q}", s.Worker, s.FD, s.Gen, s.Res, s.Head)) + } + errText := "" + if r.answerErr != nil { + errText = r.answerErr.Error() + } + t.Logf("RECVTHEFT715 arm=%s result attempts=%d hit=%v fd=%d closer=%d sibling=%d b_fd=%d reused=%v held_open=%v held_by_a=%v released=%v hole=%d gate=%v candidates=%d witness=+%d stale_recv_data_closed=+%d stolen=%v answered=%v answer_err=%q stale=[%s] misses{no_close=%d queued=%d closer_accepts=%d hole_accepts=%d}", + arm, r.attempts, r.hit, r.fd, r.closer, r.sibl, r.bFD, r.reused, r.heldOpen, r.heldByA, r.released, r.hole, r.gate, r.candidatesHit, r.witness, r.staleClosed, r.stolen, r.answered, + errText, strings.Join(heads, " "), r.missNoClose, r.missQueued, r.missOtherFD, r.missHole) +} + +func judgeTheftTrial(t *testing.T, arm string, r theftResult) { + t.Helper() + logTheftResult(t, arm, r) + if !r.hit { + skipOrFail656(t, "INCONCLUSIVE: no sibling accept that tests the theft while the closer was parked, in %d attempts", r.attempts) + } + if !r.reused && !r.heldByA { + t.Fatalf("a hit that tests nothing: B was given %d, A's number %d was neither reused nor kept as A's socket (see theftHit)", r.bFD, r.fd) + } + if r.witness == 0 { + t.Fatalf("the close hold fired but close_with_unsubmitted_recv did not move") + } + if r.stolen || !r.answered { + t.Fatalf("B (fd %d on worker %d; A's number %d reused=%v, kept open at the close=%v) lost its request: stolen by the closed conn's recv=%v (stale_recv_data_closed +%d), answered=%v (%v)", + r.bFD, r.sibl, r.fd, r.reused, r.heldOpen, r.stolen, r.staleClosed, r.answered, r.answerErr) + } + if !r.released { + t.Fatalf("A's number %d, kept open at the close, still names A's socket %v after the closer's release: the kept descriptor leaked", r.fd, theftReleaseWait) + } +} + +// TestRecvTheft715ArmA is hypothesis (a) on the tree as it is. It asserts the +// property the celeris#685 close-path fix must restore, and fails on main by +// design when (a) holds: B's request read by A's unsubmitted recv. +func TestRecvTheft715ArmA(t *testing.T) { + requireRecvTheft715(t) + judgeTheftTrial(t, "A", runRecvTheftTrial(t, theftArm{name: "A"})) +} + +// TestRecvTheft715ArmAHole is arm A with a hole opened under A's number once +// the closer is parked (see theftArm). On a tree that releases the number at +// the close the sibling's first accept fills the hole and is skipped, and the +// next is given A's number: it must fail exactly as arm A does. A verdict that +// counted the hole's accept as a hit passed that tree (celeris#793 review +// round 2). +func TestRecvTheft715ArmAHole(t *testing.T) { + requireRecvTheft715(t) + judgeTheftTrial(t, "A-hole", runRecvTheftTrial(t, theftArm{name: "A-hole", hole: true})) +} + +// TestRecvTheft715Control is arm A's trial with the close paths submitting the +// ring before they close (recvtheft.Options.SubmitBeforeClose): A's recv is +// issued while N still names A's socket. Predicted: B answered, nothing stolen. +func TestRecvTheft715Control(t *testing.T) { + requireRecvTheft715(t) + judgeTheftTrial(t, "control", runRecvTheftTrial(t, theftArm{name: "control", submitBeforeClose: true})) +} + +// TestRecvTheft715ArmC is hypothesis (c): a request the promoted connection's +// own recv read but that never reached its dispatch goroutine. The window +// between the worker's asyncInMu unlock and its Signal / goroutine start is +// widened to 2 ms on every hand-off (recvtheft.SetWakeHold) while keep-alive +// and pipelined requests hit an async route. Asserts every one is answered and +// that the hold actually ran. +func TestRecvTheft715ArmC(t *testing.T) { + requireRecvTheft715(t) + _, port := startRecvTheftEngine(t) + recvtheft.SetWakeHold(2 * time.Millisecond) + defer recvtheft.SetWakeHold(0) + holds0 := recvtheft.WakeHolds() + + const conns, rounds, pipeline = 8, 20, 3 + req := "GET /async HTTP/1.1\r\nHost: recv-theft-715\r\n\r\n" + var mu sync.Mutex + var lost []string + answered := 0 + var wg sync.WaitGroup + for c := range conns { + wg.Go(func() { + conn, err := net.DialTimeout("tcp", "127.0.0.1:"+strconv.Itoa(port), time.Second) + if err != nil { + mu.Lock() + lost = append(lost, fmt.Sprintf("conn %d: dial: %v", c, err)) + mu.Unlock() + return + } + defer func() { _ = conn.Close() }() + buf := make([]byte, 0, 4096) + tmp := make([]byte, 4096) + for round := range rounds { + n := 1 + if round%4 == 3 { + n = pipeline + } + _ = conn.SetDeadline(time.Now().Add(2 * time.Second)) + if _, err := conn.Write([]byte(strings.Repeat(req, n))); err != nil { + mu.Lock() + lost = append(lost, fmt.Sprintf("conn %d round %d: write: %v", c, round, err)) + mu.Unlock() + return + } + for got := 0; got < n; { + if i := bytes.Index(buf, []byte("\r\n\r\nok")); i >= 0 { + buf = buf[i+len("\r\n\r\nok"):] + got++ + mu.Lock() + answered++ + mu.Unlock() + continue + } + k, err := conn.Read(tmp) + if err != nil { + mu.Lock() + lost = append(lost, fmt.Sprintf("conn %d round %d: %d of %d answered: %v", c, round, got, n, err)) + mu.Unlock() + return + } + buf = append(buf, tmp[:k]...) + } + } + }) + } + wg.Wait() + holds := recvtheft.WakeHolds() - holds0 + want := conns * (rounds/4*(3+pipeline) + rounds%4) + t.Logf("RECVTHEFT715 arm=C result conns=%d requests=%d answered=%d lost=%d wake_holds=+%d", conns, want, answered, len(lost), holds) + for _, l := range lost { + t.Logf("RECVTHEFT715 arm=C lost %s", l) + } + if holds == 0 { + t.Fatal("the hand-off window hold never ran: the async hand-off was not exercised") + } + if len(lost) > 0 || answered != want { + t.Fatalf("%d of %d requests answered with the hand-off window widened; lost: %v", answered, want, lost) + } +} diff --git a/engine/iouring/recv_theft_prod_linux_test.go b/engine/iouring/recv_theft_prod_linux_test.go new file mode 100644 index 00000000..5561b831 --- /dev/null +++ b/engine/iouring/recv_theft_prod_linux_test.go @@ -0,0 +1,34 @@ +//go:build linux && !validation + +package iouring + +import ( + "testing" + "unsafe" + + "github.com/goceleris/celeris/internal/recvtheft" +) + +// TestRecvTheftWitnessCompilesAway pins that the celeris#715 witness costs a +// production build nothing: recvtheft.Enabled is false, so its call sites +// compile away, and connState.recvArmSeq is zero-size and not the last field +// (a trailing zero-size field is the one kind Go may pad for), so +// kernelInflight, the field after it, sits where its own alignment puts it and +// the connState layout is unchanged. Measured at the time of writing: +// linux/amd64 and linux/arm64 both 600 bytes, with and without the field. +func TestRecvTheftWitnessCompilesAway(t *testing.T) { + if recvtheft.Enabled { + t.Fatal("recvtheft.Enabled is true in a build without the validation tag") + } + var cs connState + if n := unsafe.Sizeof(cs.recvArmSeq); n != 0 { + t.Fatalf("connState.recvArmSeq is %d bytes in production, want 0", n) + } + at, next, align := unsafe.Offsetof(cs.recvArmSeq), unsafe.Offsetof(cs.kernelInflight), unsafe.Alignof(cs.kernelInflight) + if want := (at + align - 1) &^ (align - 1); next != want { + t.Fatalf("connState.recvArmSeq at offset %d moves kernelInflight to %d, want %d (its alignment, %d, alone)", at, next, want, align) + } + if last := unsafe.Offsetof(cs.recvOutstanding); at >= last { + t.Fatalf("connState.recvArmSeq (offset %d) must not be the last field (recvOutstanding is at %d): a trailing zero-size field can add padding", at, last) + } +} diff --git a/engine/iouring/ring.go b/engine/iouring/ring.go index 63ab1576..4c99aba1 100644 --- a/engine/iouring/ring.go +++ b/engine/iouring/ring.go @@ -394,6 +394,20 @@ func retryPending(n uint32, errno unix.Errno) uint32 { // Pending returns the number of SQEs submitted but not yet sent to the kernel. func (r *Ring) Pending() uint32 { return r.pending } +// sqPlaced returns the SQ ring's tail: the sequence number the next GetSQE +// will take, so the SQE placed last has sqPlaced()-1. +func (r *Ring) sqPlaced() uint32 { + if r.singleIssuer { + return *(*uint32)(r.sqTail) + } + return atomic.LoadUint32((*uint32)(r.sqTail)) +} + +// sqConsumed returns the SQ ring's head, which the kernel advances as it +// consumes SQEs: an SQE placed at sequence s has not reached the kernel yet +// while int32(s-sqConsumed()) >= 0 (celeris#715). +func (r *Ring) sqConsumed() uint32 { return atomic.LoadUint32((*uint32)(r.sqHead)) } + // ClearPending resets the pending counter without issuing a syscall. Used with // SQPOLL where the kernel thread submits SQEs automatically. func (r *Ring) ClearPending() { r.pending = 0 } diff --git a/engine/iouring/worker.go b/engine/iouring/worker.go index 71933c01..be138d70 100644 --- a/engine/iouring/worker.go +++ b/engine/iouring/worker.go @@ -26,6 +26,7 @@ import ( "github.com/goceleris/celeris/internal/ctxkit" "github.com/goceleris/celeris/internal/deferlinger" "github.com/goceleris/celeris/internal/platform" + "github.com/goceleris/celeris/internal/recvtheft" "github.com/goceleris/celeris/internal/sockopts" "github.com/goceleris/celeris/internal/wakefd" "github.com/goceleris/celeris/internal/zcwindow" @@ -178,10 +179,20 @@ func clampBufRingCount(n int) int { // goroutine's defer that reads cs.fd / cs.asyncInBuf. (The goroutine's // own closure references remain visible to GC after we drop ours, so // dropping the ref once the kernel ops drained is safe.) +// +// holdsFD marks an entry whose close path left its descriptor, fd, OPEN +// because the kernel still owed an op on it (celeris#685, see closeFDOwed); +// drainPendingRelease closes it at the same point it releases cs: when the +// last owed op has delivered its terminal CQE. A flag rather than an fd of -1 +// so that the zero entry holds nothing. Both sit in detached's padding, so the +// entry stays 24 bytes on 64-bit platforms +// (TestPendingReleaseEntryStaysTwentyFourBytes). type pendingReleaseEntry struct { cs *connState releaseAtNanos int64 detached bool + holdsFD bool + fd int32 } // closedOpsEntry is the Worker.closedOps value: the conn(s) closed under @@ -200,7 +211,18 @@ type closedOpsEntry struct { // (TestClosedOpsEntryStaysThirtyTwoBytes). 32-bit platforms have no // padding there, and the entry grows from 16 to 20 bytes. handoff bool - conns []*connState + // fdOps is the part of inflight that still names the descriptor + // (celeris#798): all of it but the SEND_ZC notifications whose send has + // completed. noteClosedInflight adds each conn's fdOps; staleConnCQE + // takes one off at a recv's or a plain send's terminal CQE and at a + // SEND_ZC's first CQE, and none at its notification. closedFDNamed + // reads it, so a close that kept its descriptor lets go of it once only + // notifications are owed, while the connState waits for inflight. A + // conn owes two such ops at most (a recv and a send), so int16 is ample + // even under a collision; it sits in the same padding as handoff, and + // the entry keeps its size on every platform. + fdOps int16 + conns []*connState } // pendingReleaseHoldNanos is the WALL-CLOCK BACKSTOP for releasing a @@ -441,6 +463,14 @@ type Worker struct { // releaseAtNanos is only the anomaly backstop — see // pendingReleaseHoldNanos. pendingRelease []pendingReleaseEntry + // closeFDOwed counts the pendingRelease entries that still hold their + // descriptor open (holdsFD, celeris#685). The worker does not park + // while it is non-zero: the terminal CQEs those closes wait for arrive + // only through the ring, and a parked worker enters no ring. Worker + // thread only. closeFDDeferredBatch is the count of such closes not yet + // added to handoffLoss.closeFDDeferred, flushed once per loop iteration. + closeFDOwed int + closeFDDeferredBatch uint64 // closedOps routes terminal recv/send CQEs that arrive AFTER their // conn was closed (w.conns[fd] already nil or reused) back to the @@ -801,6 +831,70 @@ func (w *Worker) noteRecvPlaced(cs *connState) { if cs.recvOutstanding >= 2 { w.recvArm.noteDoubleArmed() } + // celeris#715, validation builds only (recvtheft.Enabled is a false + // constant otherwise): both placement sites call this right after the + // recv's GetSQE, so the SQE placed last is the recv. + if recvtheft.Enabled { + cs.recvArmSeq.Set(w.ring.sqPlaced() - 1) + } +} + +// recvUnsubmitted reports whether cs's armed recv SQE is still in the SQ +// ring, not yet consumed by the kernel: it was placed after this worker's +// last submit. Validation builds only: recvArmSeq is recorded only there. +// +// Why the close paths ask (celeris#715 hypothesis (a), the celeris#685 +// class). Such a recv names the descriptor NUMBER (fixed files are off), and +// the kernel resolves the number when the next submit issues the recv. +// finishClose and finishCloseDetached queued the recv's ASYNC_CANCEL behind it +// and closed the descriptor at once. If another thread's accept was given the +// freed number before this worker's next submit, and its connection's request +// was already in the socket (TCP_DEFER_ACCEPT hands over only connections that +// have sent), the recv read that request and completed under the closed +// conn's (fd, generation): staleConnCQE dropped it as stale_recv_data_closed, +// and the new connection's own recv then waited on an empty socket. The close +// paths now keep the number while the recv is owed (fdOwed); under +// -tags=validation they still +// - count the close (recvtheft.CloseWithUnsubmittedRecv, the witness of +// the precondition), +// - park the worker when the close path returns, when a recvtheft trial is +// armed (recvtheft.HoldAfterClose), and +// - in the trial's control arm, submit the ring before the close +// (recvtheft.SubmitBeforeClose), so the recv is issued while the number +// still names this conn's socket. +// +// None of it exists in production: recvtheft.Enabled is a false constant +// there and cs.recvArmSeq is zero-size. +func (w *Worker) recvUnsubmitted(cs *connState) bool { + return cs.recvArmed && w.ring != nil && int32(cs.recvArmSeq.Get()-w.ring.sqConsumed()) >= 0 +} + +// recvLinkedOwed reports whether cs's armed recv is the one chained behind a +// SEND (flushSendLink) and has not completed: the kernel consumed its SQE with +// the SEND's, but issues it, and resolves its descriptor number, only after +// the SEND completes, as task work on a DEFER_TASKRUN ring, which can still +// be queued when the SEND's CQE is read (celeris#685). recvLinked is cleared +// only by that recv's own completion, so the pair is exact. +func recvLinkedOwed(cs *connState) bool { + return cs.recvArmed && cs.recvLinked +} + +// recvTheftWitness counts a close path's celeris#715 / celeris#685 +// preconditions (validation builds only; the caller is guarded by +// recvtheft.Enabled) and reports whether the close hold applies, and in +// which form: an unsubmitted recv (linked false) or a linked one still owed +// (linked true). The caller defers recvtheft.HoldAfterClose itself, so the +// hold runs when the close path returns. +func (w *Worker) recvTheftWitness(cs *connState) (hold, linked bool) { + switch { + case w.recvUnsubmitted(cs): + recvtheft.NoteCloseWithUnsubmittedRecv() + return true, false + case recvLinkedOwed(cs): + recvtheft.NoteCloseWithLinkedRecv() + return true, true + } + return false, false } // retireRecvCancel accounts for one of cs's outstanding backpressure-pause @@ -1322,6 +1416,13 @@ func (w *Worker) run(ctx context.Context) { w.zc.noteRingBytes(w.ringBytesBatch) w.ringBytesBatch = 0 } + // Same cadence for the celeris#685 deferred-close rate. + if w.closeFDDeferredBatch > 0 { + if w.handoffLoss != nil { + w.handoffLoss.closeFDDeferred.Add(w.closeFDDeferredBatch) + } + w.closeFDDeferredBatch = 0 + } // Same cadence for the celeris#607 link-arm exposure witness. if w.linkArmBatch > 0 { if w.recvArm != nil { @@ -1355,6 +1456,13 @@ func (w *Worker) run(ctx context.Context) { // queuePendingRelease docstring). w.iterCount++ if len(w.pendingRelease) > 0 { + // An idle iteration with a descriptor still kept for an owed op + // (celeris#685) refreshes the clock the backstop reads: cachedNow + // moves only with CQE traffic or the timeout sweep, and a kept + // descriptor also keeps this worker from parking. + if w.closeFDOwed > 0 && w.emptyIters > 0 { + w.cachedNow = time.Now().UnixNano() + } w.drainPendingRelease() } @@ -1439,9 +1547,15 @@ func (w *Worker) run(ctx context.Context) { // re-checked under wakeMu below, and every driver-action enqueue // kicks a parked worker through wakeIfSuspended; see there for why // the pair cannot lose a wakeup. + // + // closeFDOwed gate (celeris#685): a close that left its descriptor + // open until the kernel's last op on it completes is finished by + // drainPendingRelease at that op's terminal CQE, which only an + // iteration of this loop reads. Parking first would hold the + // descriptor, and the socket with it, for as long as the park lasts. if w.listenFD < 0 && w.connCount == 0 && !w.hasDriverConns.Load() && w.driverActionPending.Load() == 0 && w.detachQPending.Load() == 0 && - w.acceptPaused.Load() { + w.closeFDOwed == 0 && w.acceptPaused.Load() { // Submit what this iteration queued before parking (celeris#657, // A5). The iteration that closes or hands off the last conn // queues its close-path cancels (the header timer's, a send's) @@ -1529,12 +1643,17 @@ func (w *Worker) staleConnCQE(c *completionEntry, fd int, ud uint64) bool { // under-count the live conn (and may clear recvArmed above // with its recv still kernel-armed), so its own close can // skip the cancel and release early — the UAF class this - // accounting exists to prevent. Reachability is narrow: - // non-SQPOLL task-work ordering posts the close-path - // -ECANCELED during the submit syscall, before the fd can be - // re-accepted; the window needs a dropped cancel SQE (full SQ - // ring) or SQPOLL's decoupled completion ordering ON TOP of - // the gen collision. When it fires, the closed conn's + // accounting exists to prevent. Reachability is narrow: a + // close path keeps the number allocated until the last op + // that names it has ended (celeris#685), so on this worker the + // number cannot be re-occupied while a recv or send CQE of the + // closed conn is still to come. Only a SEND_ZC notification + // can be (celeris#798: it names no descriptor, and a stalled + // peer holds it), which leaves this window where it was + // before celeris#685 for that one CQE; any other needs the + // pendingRelease backstop to have closed a number with an op + // still owed (CloseFDForced, must stay 0). Either way the + // gen collision comes ON TOP. When it fires, the closed conn's // closedOps entry is left orphaned and the 5 s backstop WARN // in drainPendingRelease is the production signal. if cs.kernelInflight > 0 { @@ -1550,9 +1669,20 @@ func (w *Worker) staleConnCQE(c *completionEntry, fd int, ud uint64) bool { // identity (celeris#657). if op == udRecv && c.Res > 0 { w.noteStaleRecvData(ud) + // celeris#715, validation builds only: record what the stale recv + // read, while closedOps still holds its connState. + if recvtheft.Enabled { + w.noteStaleRecvExemplar(c, fd, ud) + } } if terminalOp { - w.noteStaleTerminalOp(ud) + // A notification ends an op that named no descriptor any more + // (celeris#798); every other terminal CQE ends one that did. + w.noteStaleTerminalOp(ud, !cqeIsNotif(c.Flags)) + } else if op == udSend { + // F_MORE on a send is a SEND_ZC's first CQE: the send is done, so it + // names the descriptor no more, and its notification is still owed. + w.noteStaleSendCompleted(ud) } if cqeHasBuffer(c.Flags) && w.bufRing != nil { w.bufRing.PushBuffer(cqeBufferID(c.Flags)) @@ -1566,7 +1696,9 @@ func (w *Worker) staleConnCQE(c *completionEntry, fd int, ud uint64) bool { // CQE's (fd, generation) identity, releasing them for drainPendingRelease // once the kernel owes them nothing. No-op when the identity is unknown // (conn closed with zero in-flight ops, or already backstop-released). -func (w *Worker) noteStaleTerminalOp(ud uint64) { +// namedFD says whether the op still named the descriptor until this CQE +// (closedOpsEntry.fdOps): false only for a SEND_ZC notification. +func (w *Worker) noteStaleTerminalOp(ud uint64, namedFD bool) { if len(w.closedOps) == 0 { return } @@ -1576,6 +1708,9 @@ func (w *Worker) noteStaleTerminalOp(ud uint64) { return } e.inflight-- + if namedFD && e.fdOps > 0 { + e.fdOps-- + } if e.inflight > 0 { return } @@ -1585,6 +1720,19 @@ func (w *Worker) noteStaleTerminalOp(ud uint64) { delete(w.closedOps, key) } +// noteStaleSendCompleted takes a closed identity's SEND_ZC off its count of +// ops that name the descriptor at the send's first CQE (celeris#798): the op +// is done with the descriptor, and only its notification, which still +// counts in inflight, is owed. Worker thread only. +func (w *Worker) noteStaleSendCompleted(ud uint64) { + if len(w.closedOps) == 0 { + return + } + if e := w.closedOps[connOpKey(ud)]; e != nil && e.fdOps > 0 { + e.fdOps-- + } +} + func (w *Worker) processCQE(ctx context.Context, c *completionEntry, now int64) { ud := c.UserData op := decodeOp(ud) @@ -2035,6 +2183,13 @@ func (w *Worker) onAcceptedFD(ctx context.Context, newFD int, now int64, isFixed cs.needsRecv = true w.markDirty(cs) } + // celeris#715, validation builds only: report the accept to an armed + // recvtheft trial, which parks this worker here (its recv prepared, not + // submitted) when it was given the number another worker's held close + // released. + if recvtheft.Enabled { + recvtheft.AfterAccept(w.id, newFD) + } } // stepAcceptPause runs one step of the accept pause on the worker thread, @@ -2238,11 +2393,13 @@ func (w *Worker) hijackConn(fd int) (net.Conn, error) { w.connCount-- w.activeConns.Add(-1) w.closeCount.Add(1) - // Cancel-then-release discipline, hijack variant: a single-shot - // recv SQE is virtually always still armed on cs.buf here. The fd - // lives on under the caller's net.Conn, so an uncancelled recv would - // not only pin cs.buf past release (the #256-class UAF) but also - // STEAL the first bytes the hijacker tries to read. Cancel it by its + // Cancel-then-release discipline, hijack variant: any op still armed + // on cs (a multishot recv stays armed across its request; the + // single-shot recv that brought the request has completed, measured by + // TestRecvTheft685HijackSingleShot) targets cs.buf. The fd lives on + // under the caller's net.Conn, so an uncancelled recv would not only + // pin cs.buf past release (the #256-class UAF) but also STEAL the + // first bytes the hijacker tries to read. Cancel it by its // generation-tagged user_data and defer the pool release until the // terminal CQE arrives, exactly like finishClose. // @@ -2256,9 +2413,39 @@ func (w *Worker) hijackConn(fd int) (net.Conn, error) { // cs.buf and then leaves it to the garbage collector, which frees the // buffers once the last view of them is gone. The cost is one connState // allocation per hijack. + // + // celeris#685 hijack witness and hold, validation builds only: count a + // hijack with an op still owed on the socket (kernelInflight > 0), and + // hold the worker thread before it returns, and so before its next + // io_uring_enter (recvtheft.SetHijackHold). + if recvtheft.Enabled { + if cs.kernelInflight > 0 { + recvtheft.NoteHijackWithOpOwed() + } + defer recvtheft.HijackHold() + } w.cancelConnOps(fd, cs) w.noteClosedInflight(cs) w.queuePendingReleaseDetached(cs) + // The fd-lifetime rule, hijack variant (celeris#685). The socket lives + // on under the hijacker's net.Conn, so no op of this worker may read it + // once the hijacker has it, and none may resolve the original number, + // which the f.Close below releases. The cancel above reaches the kernel + // only at the next submit. Until then an owed recv that is already + // issued (a multishot recv stays armed across its request) can take the + // hijacker's first bytes on a ring whose completions run at any syscall + // exit, and one whose SQE is still in the ring would resolve the number + // after the close. So when an op is owed, submit now: an issued recv is + // cancelled before it can read, and a recv SQE still in the ring is + // issued first, against this socket and not a reused number, then + // cancelled. No hijack path leaves such an SQE: a hijack runs inside + // its request's processing, after that request's recv completed. Nor a + // linked recv, which rides behind a SEND: a hijack with a send pending + // is refused above. One io_uring_enter per hijack with an op owed; none + // otherwise, which is every hijack on single-shot recv. + if fdOwed(cs) && !w.sqpoll { + _, _ = w.ring.Submit() + } f := os.NewFile(uintptr(fd), "tcp") c, err := net.FileConn(f) _ = f.Close() @@ -2931,6 +3118,11 @@ func (w *Worker) handleRecv(c *completionEntry, fd int, now int64) { w.bufRing.PushBuffer(providedBufID) w.hasBufReturns = true } + // celeris#715 hypothesis (c), validation builds only: the same + // window as promoteConnToAsync's (recvtheft.SetWakeHold). + if recvtheft.Enabled { + recvtheft.WakeHold() + } if starting { w.asyncWG.Add(1) go w.runAsyncHandler(cs) @@ -3890,6 +4082,7 @@ func (w *Worker) noteClosedInflight(cs *connState) { w.closedOps[key] = e } e.inflight += cs.kernelInflight + e.fdOps += int16(fdOps(cs)) e.conns = append(e.conns, cs) } @@ -3912,10 +4105,26 @@ func (w *Worker) dropClosedOps(cs *connState) { // fires). See Worker.pendingRelease docstring for the kernel-buffer-lifetime // invariant this enforces. func (w *Worker) queuePendingRelease(cs *connState) { + w.queuePendingReleaseFD(cs, false, -1) +} + +// queuePendingReleaseFD is queuePendingRelease (detached false) or +// queuePendingReleaseDetached (detached true) for a close path that may also +// hand the descriptor over: keptFD >= 0 is closed by drainPendingRelease when +// cs is released, not by the caller (celeris#685, closeFDOwed); -1 means the +// caller closes it. Worker thread only. +func (w *Worker) queuePendingReleaseFD(cs *connState, detached bool, keptFD int) { w.pendingRelease = append(w.pendingRelease, pendingReleaseEntry{ cs: cs, releaseAtNanos: time.Now().UnixNano() + pendingReleaseHoldNanos, + detached: detached, + holdsFD: keptFD >= 0, + fd: int32(keptFD), }) + if keptFD >= 0 { + w.closeFDOwed++ + w.closeFDDeferredBatch++ + } } // queuePendingReleaseDetached holds cs alive past the kernel's recv-SQE @@ -3933,11 +4142,7 @@ func (w *Worker) queuePendingRelease(cs *connState) { // visible on behalf of the kernel's invisible recv pointer, so it must // outlive every pending op targeting cs.buf // span-corruption class). func (w *Worker) queuePendingReleaseDetached(cs *connState) { - w.pendingRelease = append(w.pendingRelease, pendingReleaseEntry{ - cs: cs, - releaseAtNanos: time.Now().UnixNano() + pendingReleaseHoldNanos, - detached: true, - }) + w.queuePendingReleaseFD(cs, true, -1) } // drainPendingRelease releases queued connStates whose kernel-held ops @@ -3967,6 +4172,14 @@ func (w *Worker) drainPendingRelease() { entry := &w.pendingRelease[i] cs := entry.cs if cs.kernelInflight > 0 { + // A kept descriptor goes as soon as no owed op names it + // (celeris#798): what is left may be SEND_ZC notifications only, + // and one of those can wait on a stalled peer for as long as the + // socket is open. cs itself stays until they arrive: they say the + // kernel is done with its send buffer. + if entry.holdsFD && !w.closedFDNamed(cs) { + w.releaseKeptFD(entry) + } if entry.releaseAtNanos > w.cachedNow { kept = append(kept, *entry) continue @@ -3975,9 +4188,25 @@ func (w *Worker) drainPendingRelease() { if w.logger != nil { w.logger.Warn("releasing connState with kernel ops unaccounted for after backstop hold", "worker", w.id, "fd", cs.fd, "generation", cs.generation, - "inflight", cs.kernelInflight, "detached", entry.detached) + "inflight", cs.kernelInflight, "detached", entry.detached, + "holds_fd", entry.holdsFD) } w.dropClosedOps(cs) + // The descriptor goes too, though an op may still name it: a + // descriptor held forever is a leak with no end. The close path + // shut the socket's read side down, so an owed recv the kernel + // issues on THIS socket ends at once; one issued after the + // number is reused would not, which is why this is counted and + // must stay 0 (celeris#685). + if entry.holdsFD { + w.handoffLoss.noteCloseFDForced() + } + } + // The close the close path left for this moment (celeris#685): + // every op that named the descriptor has delivered its terminal + // CQE, so none can resolve the number any more. + if entry.holdsFD { + w.releaseKeptFD(entry) } if !entry.detached { releaseConnState(cs) @@ -4030,6 +4259,9 @@ func (w *Worker) finishClose(fd int) { // Capture close-path decisions before queueing cs for deferred release. fixedFile := cs != nil && cs.fixedFile fastClose := cs != nil && engine.Protocol(cs.protocol.Load()) == engine.HTTP1 && cs.h1State != nil && !cs.h1State.Detached.Load() + // The fd-lifetime rule (celeris#685): while the kernel still owes an op + // that names fd, the number is not released here. See fdOwed. + owed := fdOwed(cs) // Cancel-then-release discipline (v1.4.15/7beebb9 corruption fix): ASYNC_CANCEL any kernel-held // op still targeting cs's buffers (the single-shot recv is virtually // ALWAYS armed here — closing the fd below does NOT complete it), then @@ -4040,9 +4272,19 @@ func (w *Worker) finishClose(fd int) { // straggler bytes into memory Go has repurposed — the #256 stackalloc // SIGSEGV / Green-Tea-GC span-corruption class. if cs != nil { + // celeris#715 witness, hold and control, validation builds only: + // see recvUnsubmitted and recvLinkedOwed. + if recvtheft.Enabled { + if hold, linked := w.recvTheftWitness(cs); hold { + defer recvtheft.HoldAfterClose(w.id, fd, linked) + } + } w.cancelConnOps(fd, cs) w.noteClosedInflight(cs) - w.queuePendingRelease(cs) + w.queuePendingReleaseFD(cs, false, keptFD(fd, owed)) + if recvtheft.Enabled && recvtheft.SubmitBeforeClose() { + _, _ = w.ring.Submit() + } } if fixedFile { @@ -4085,23 +4327,32 @@ func (w *Worker) finishClose(fd int) { // // forceRSTClose (slowloris-defence): SHUT_RDWR + close. See // finishCloseDetached for the rationale. + // + // Each branch below ends in closeUnlessOwed: with an op still owed the + // descriptor stays open, and drainPendingRelease closes it at that op's + // terminal CQE (celeris#685). A branch that did not shut the read side + // down already does so first, so the owed recv ends as soon as the + // kernel issues it (fdOwed). if cs != nil && cs.forceRSTClose { _ = unix.Shutdown(fd, unix.SHUT_RDWR) - _ = unix.Close(fd) + closeUnlessOwed(fd, owed) return } // fastClose: plain H1 no-body conn → close() alone suffices. if fastClose { - _ = unix.Close(fd) + if owed { + _ = unix.Shutdown(fd, fastCloseShutdownHow(cs)) + } + closeUnlessOwed(fd, owed) return } - _ = unix.Shutdown(fd, unix.SHUT_WR) + _ = unix.Shutdown(fd, shutdownHow(owed)) raddr := "" if cs != nil { raddr = cs.remoteAddr } sockopts.CloseDrain(fd, "iouring/finishClose", raddr) - _ = unix.Close(fd) + closeUnlessOwed(fd, owed) } // finishCloseAny dispatches to finishCloseDetached for detached connections @@ -4140,6 +4391,8 @@ func (w *Worker) finishCloseDetached(fd int, cs *connState) { } fixedFile := cs.fixedFile + // The fd-lifetime rule (celeris#685), as in finishClose: see fdOwed. + owed := fdOwed(cs) // Do NOT call releaseConnState — goroutine closures still reference cs. // // Cancel-then-release discipline (v1.4.15/7beebb9 corruption fix): ASYNC_CANCEL the armed recv @@ -4156,9 +4409,19 @@ func (w *Worker) finishCloseDetached(fd int, cs *connState) { // "s.allocCount != s.nelems" span corruption (v1.4.15/7beebb9: the old 100 ms // wall-clock hold sat below TCP's 200 ms RTO_MIN, so retransmitted // POST segments landed after release). + // celeris#715 witness, hold and control, validation builds only: see + // recvUnsubmitted and recvLinkedOwed. + if recvtheft.Enabled { + if hold, linked := w.recvTheftWitness(cs); hold { + defer recvtheft.HoldAfterClose(w.id, fd, linked) + } + } w.cancelConnOps(fd, cs) w.noteClosedInflight(cs) - w.queuePendingReleaseDetached(cs) + w.queuePendingReleaseFD(cs, true, keptFD(fd, owed)) + if recvtheft.Enabled && recvtheft.SubmitBeforeClose() { + _, _ = w.ring.Submit() + } if fixedFile { sqe := w.ring.GetSQE() @@ -4178,9 +4441,12 @@ func (w *Worker) finishCloseDetached(fd int, cs *connState) { // LINGER on iouring. The SHUT_RDWR + LINGER combo wins because // SHUT_RDWR drops the receive queue too (close() with LINGER // only drops TX). Walker observes the abortive close immediately. + // + // Each branch ends in closeUnlessOwed, and shuts the read side down too + // when an op is still owed (shutdownHow), exactly as finishClose does. if cs.forceRSTClose { _ = unix.Shutdown(fd, unix.SHUT_RDWR) - _ = unix.Close(fd) + closeUnlessOwed(fd, owed) return } // Async-mode HTTP1 conns are NOT truly detached (no WS/SSE middleware @@ -4200,17 +4466,17 @@ func (w *Worker) finishCloseDetached(fd int, cs *connState) { // the drain syscall and the close syscall left a multi-µs window in // which a fresh walker drip would queue, making the close → RST). if cs.h1State != nil && !cs.h1State.Detached.Load() { - _ = unix.Shutdown(fd, unix.SHUT_WR) - _ = unix.Close(fd) + _ = unix.Shutdown(fd, shutdownHow(owed)) + closeUnlessOwed(fd, owed) return } // Truly detached (WS/SSE): graceful half-close so middleware-queued // close-frame echoes flush before the FIN. Sync unix.Close (not via // io_uring) to avoid the async-SQE pile-up that plagued the pre-patch // version. - _ = unix.Shutdown(fd, unix.SHUT_WR) + _ = unix.Shutdown(fd, shutdownHow(owed)) sockopts.CloseDrain(fd, "iouring/finishCloseDetached", cs.remoteAddr) - _ = unix.Close(fd) + closeUnlessOwed(fd, owed) } // runAsyncHandler is the dispatch goroutine for an HTTP1 conn when @@ -4238,6 +4504,11 @@ func (w *Worker) promoteConnToAsync(cs *connState, _ int, stashed []byte, c *com cs.asyncRun = true } cs.asyncInMu.Unlock() + // celeris#715 hypothesis (c), validation builds only: widen the window + // between the unlock and the goroutine's wake-up (recvtheft.SetWakeHold). + if recvtheft.Enabled { + recvtheft.WakeHold() + } if starting { w.asyncWG.Add(1) go w.runAsyncHandler(cs) @@ -4254,6 +4525,13 @@ func (w *Worker) promoteConnToAsync(cs *connState, _ int, stashed []byte, c *com w.markDirty(cs) } } + // celeris#715 hypothesis (a), validation builds only: with a trial armed, + // let the dispatch goroutine queue its close before this iteration + // reaches drainDetachQueue, so the recv just re-armed is still + // unsubmitted when closeConn runs (recvtheft.Options.PromoteGate). + if recvtheft.Enabled { + recvtheft.AfterPromoteArm(func() bool { return w.detachQPending.Load() != 0 }) + } } // asyncHeaderDeadline returns the header deadline the worker should arm a @@ -5654,6 +5932,13 @@ func (w *Worker) shutdown() { // the wakeup eventfd is closed, because addAdoptAction signals it under // the same lock (celeris#658). w.closeAdoptQueue() + // The fd-lifetime rule at shutdown (celeris#685): end every op the kernel + // still owes on a connection's descriptor before any of those + // descriptors is closed (endOwedOpsAtShutdown). Before shutdownDrivers: + // the drain submits what the SQ ring still holds, and shutdownDrivers + // closes the driver descriptors on the promise that nothing is + // submitted after it. + w.endOwedOpsAtShutdown() // Fire onClose for every registered driver conn before tearing down // ring/listen fd. Otherwise driver callbacks are silently dropped. Then // wait for the driver closes handed off the worker before the shutdown, @@ -5710,6 +5995,8 @@ func (w *Worker) shutdown() { if cs.h2State != nil { conn.CloseH2(cs.h2State) } + // endOwedOpsAtShutdown has ended the ops owed on fd, so no op can + // resolve the number once it is free (celeris#685). if !cs.fixedFile { _ = unix.Close(fd) } diff --git a/internal/recvtheft/doc.go b/internal/recvtheft/doc.go new file mode 100644 index 00000000..c69d88dc --- /dev/null +++ b/internal/recvtheft/doc.go @@ -0,0 +1,49 @@ +// Package recvtheft holds the witness and the hold points of the celeris#715 +// recv-theft measurement (hypothesis (a) of that issue, the celeris#685 class). +// Everything here is compiled in only under -tags=validation (recvtheft.go); +// production builds get the no-ops in recvtheft_off.go, whose false Enabled +// constant compiles the engine's call sites away, and whose ArmSeq is a +// zero-size struct. It is internal, like internal/zcwindow, so nothing outside +// this module can call it. +// +// The window. The io_uring worker only PREPARES a recv SQE; it reaches the +// kernel at the next io_uring_enter, at the top of the next loop iteration. +// Fixed files are off, so the SQE names the descriptor NUMBER, and the kernel +// resolves that number when it issues the op, not when the SQE was written. +// finishClose and finishCloseDetached queued an ASYNC_CANCEL behind such an +// unsubmitted recv and then closed the descriptor synchronously. If another +// thread's accept was given the freed number before this worker submitted, +// the recv read the new connection's request, completed under the old +// connection's (fd, generation), and was dropped as stale; the new connection +// then waited on an empty socket. A recv linked behind a SEND has the same +// window between the SEND's CQE and the task work that issues it. Since +// celeris#685 the close paths keep the number allocated while such an op is +// owed, so the sibling is given another number; the trials assert that the +// new connection is answered either way. +// +// The pieces: +// - witness counters: [CloseWithUnsubmittedRecv], a close reached while the +// connection's recv SQE had not been consumed by the kernel, and +// [CloseWithLinkedRecv], one reached while its recv was linked behind a +// SEND and had not completed; +// - a close hold ([HoldAfterClose]) that parks the closing worker when its +// close path returns (for one form or the other, [Options.LinkedRecv]), +// and an accept hold ([AfterAccept]) that parks a sibling worker right +// after it accepted the closed descriptor's number, before it submits its +// own recv for it. Together they force the one ordering the theft needs: +// the sibling's accept after the close, the closing worker's submit +// before the sibling's; +// - a gate ([AfterPromoteArm]) that makes the dispatch goroutine's close +// land in the same loop iteration as the promote's recv re-arm; +// - the control switch ([Options.SubmitBeforeClose]): submit before the +// close, so the recv is issued while the number still names the closing +// connection's socket; +// - an exemplar record of every stale recv completion that carried data +// ([NoteStaleRecvData]), with its first bytes; +// - a separate hold for hypothesis (c) ([WakeHold]): the dispatch +// goroutine's wake-up delayed between the worker's asyncInMu unlock and +// its Signal / goroutine start; +// - the hijack witness ([HijackWithOpOwed]) and hold ([HijackHold]): a +// Hijack made while the kernel still owed the connection an op, and the +// worker held right after it, before its next io_uring_enter. +package recvtheft diff --git a/internal/recvtheft/recvtheft.go b/internal/recvtheft/recvtheft.go new file mode 100644 index 00000000..aa00f6fe --- /dev/null +++ b/internal/recvtheft/recvtheft.go @@ -0,0 +1,355 @@ +//go:build validation + +package recvtheft + +import ( + "sync" + "sync/atomic" + "time" +) + +// Enabled is true in a -tags=validation build and false (a constant, so +// guarded code compiles away) in production (recvtheft_off.go). +const Enabled = true + +// ArmSeq records the SQ ring sequence number at which a connection's latest +// recv SQE was placed (the ring's tail minus one right after the placement). +// Compared with the kernel's SQ head it says whether the kernel has consumed +// that SQE yet. Worker-thread only, like every other recv field of the +// connection. Zero-size in production. +type ArmSeq struct{ seq uint32 } + +// Set records the sequence number of the recv SQE just placed. +func (a *ArmSeq) Set(seq uint32) { a.seq = seq } + +// Get returns the recorded sequence number. +func (a *ArmSeq) Get() uint32 { return a.seq } + +// closeWithUnsubmittedRecv is the witness: closes (finishClose, +// finishCloseDetached) reached while the connection's recv SQE was still in +// the SQ ring, not yet consumed by the kernel. +var closeWithUnsubmittedRecv atomic.Uint64 + +// NoteCloseWithUnsubmittedRecv counts one such close. Engine hook. +func NoteCloseWithUnsubmittedRecv() { closeWithUnsubmittedRecv.Add(1) } + +// CloseWithUnsubmittedRecv returns the witness count since process start. +func CloseWithUnsubmittedRecv() uint64 { return closeWithUnsubmittedRecv.Load() } + +// closeWithLinkedRecv is the witness of the linked form (celeris#685): closes +// reached while the connection's recv was chained behind a SEND +// (IOSQE_IO_LINK) and had not completed. Such a recv was submitted, so the +// witness above does not count it, but the kernel issues it, and resolves +// its descriptor number, only after the SEND completes. +var closeWithLinkedRecv atomic.Uint64 + +// NoteCloseWithLinkedRecv counts one such close. Engine hook. +func NoteCloseWithLinkedRecv() { closeWithLinkedRecv.Add(1) } + +// CloseWithLinkedRecv returns the linked witness count since process start. +func CloseWithLinkedRecv() uint64 { return closeWithLinkedRecv.Load() } + +// Options configures one [Trial]. +type Options struct { + // SubmitBeforeClose is the control arm: the close paths submit the SQ + // ring after queueing their cancels and before closing the descriptor, + // so a recv prepared earlier in the iteration is issued while its + // number still names the closing connection's socket. + SubmitBeforeClose bool + // PromoteGate bounds how long a worker that has just re-armed a + // promoted connection's recv (promoteConnToAsync) waits for the + // dispatch goroutine to queue a close. Zero disables the gate. The + // gate fires once per trial. + PromoteGate time.Duration + // HoldMax bounds each hold, so a test that stops driving the trial + // cannot park a worker for longer than this. Zero means 5 s. + HoldMax time.Duration + // LinkedRecv makes the close hold fire on a close with a LINKED recv + // owed (the celeris#685 linked form, [CloseWithLinkedRecv]) instead of + // one with an unsubmitted recv. + LinkedRecv bool +} + +// CloseEvent is the close hold firing: worker Worker closed descriptor FD +// with a recv SQE for it still unsubmitted, and is parked. +type CloseEvent struct{ Worker, FD int } + +// AcceptEvent is an accept the engine completed while a trial had a target: +// worker Worker accepted a connection as descriptor FD. +type AcceptEvent struct{ Worker, FD int } + +// StaleRecv is one stale recv completion that carried data: the recv's +// identity (FD, Gen), its result and the first bytes it wrote. +type StaleRecv struct { + Worker int + FD int + Gen uint32 + Res int32 + Head []byte +} + +// Trial is one armed run of the measurement. At most one is armed at a time. +type Trial struct { + opts Options + + gateUsed atomic.Bool + + closeFired atomic.Bool + closeCh chan CloseEvent + releaseClose chan struct{} + relCloseOnce sync.Once + target atomic.Int64 + holder atomic.Int64 + + acceptCh chan AcceptEvent + acceptFired atomic.Bool + acceptHeldCh chan AcceptEvent + releaseAccept chan struct{} + relAcceptOnce sync.Once + + mu sync.Mutex + stale []StaleRecv +} + +var current atomic.Pointer[Trial] + +// Arm arms a new trial and returns it. A trial still armed is disarmed +// (released) first. +func Arm(o Options) *Trial { + if o.HoldMax <= 0 { + o.HoldMax = 5 * time.Second + } + t := &Trial{ + opts: o, + closeCh: make(chan CloseEvent, 1), + releaseClose: make(chan struct{}), + acceptCh: make(chan AcceptEvent, 64), + acceptHeldCh: make(chan AcceptEvent, 1), + releaseAccept: make(chan struct{}), + } + t.target.Store(-1) + t.holder.Store(-1) + if old := current.Swap(t); old != nil { + old.release() + } + return t +} + +// Disarm releases every hold of t and disarms it. +func (t *Trial) Disarm() { + current.CompareAndSwap(t, nil) + t.release() +} + +func (t *Trial) release() { + t.ReleaseClose() + t.ReleaseAccept() +} + +// WaitClose waits up to d for the close hold to fire. +func (t *Trial) WaitClose(d time.Duration) (CloseEvent, bool) { + select { + case ev := <-t.closeCh: + return ev, true + case <-time.After(d): + return CloseEvent{}, false + } +} + +// ReleaseClose lets the worker parked by the close hold continue. +func (t *Trial) ReleaseClose() { t.relCloseOnce.Do(func() { close(t.releaseClose) }) } + +// NextAccept waits up to d for the next accept completed since the close +// hold fired. +func (t *Trial) NextAccept(d time.Duration) (AcceptEvent, bool) { + select { + case ev := <-t.acceptCh: + return ev, true + case <-time.After(d): + return AcceptEvent{}, false + } +} + +// WaitAcceptHeld waits up to d for the accept hold to fire. +func (t *Trial) WaitAcceptHeld(d time.Duration) (AcceptEvent, bool) { + select { + case ev := <-t.acceptHeldCh: + return ev, true + case <-time.After(d): + return AcceptEvent{}, false + } +} + +// ReleaseAccept lets the worker parked by the accept hold continue. +func (t *Trial) ReleaseAccept() { t.relAcceptOnce.Do(func() { close(t.releaseAccept) }) } + +// Stale returns a copy of the stale recv completions with data recorded while +// t was armed. +func (t *Trial) Stale() []StaleRecv { + t.mu.Lock() + defer t.mu.Unlock() + return append([]StaleRecv(nil), t.stale...) +} + +// GateUsed reports whether the promote gate fired in this trial. +func (t *Trial) GateUsed() bool { return t.gateUsed.Load() } + +// SubmitBeforeClose reports whether the armed trial is the control arm. +// Engine hook, worker thread. +func SubmitBeforeClose() bool { + t := current.Load() + return t != nil && t.opts.SubmitBeforeClose +} + +// AfterPromoteArm is called by promoteConnToAsync after it re-armed the +// connection's recv. The first time in a trial with a PromoteGate, it waits +// until queued reports that a detach-queue entry is pending (the dispatch +// goroutine's close) or the gate expires, so the close is drained in the same +// loop iteration as the re-arm. Engine hook, worker thread. +func AfterPromoteArm(queued func() bool) { + t := current.Load() + if t == nil || t.opts.PromoteGate <= 0 || t.closeFired.Load() || !t.gateUsed.CompareAndSwap(false, true) { + return + } + deadline := time.Now().Add(t.opts.PromoteGate) + for !queued() && time.Now().Before(deadline) { + time.Sleep(20 * time.Microsecond) + } +} + +// HoldAfterClose is deferred by finishClose / finishCloseDetached when they +// found the connection's recv SQE unsubmitted (linked false), or its recv +// linked behind a SEND and not completed (linked true), so it runs when the +// close path returns: right after the descriptor is closed, on a tree that +// closes it there. The first time in a trial whose [Options.LinkedRecv] +// matches linked, it records (worker, fd) as the trial's target and parks the +// worker until [Trial.ReleaseClose] or HoldMax. Engine hook, worker thread. +func HoldAfterClose(worker, fd int, linked bool) { + t := current.Load() + if t == nil || t.opts.LinkedRecv != linked || !t.closeFired.CompareAndSwap(false, true) { + return + } + t.holder.Store(int64(worker)) + t.target.Store(int64(fd)) + t.closeCh <- CloseEvent{Worker: worker, FD: fd} + select { + case <-t.releaseClose: + case <-time.After(t.opts.HoldMax): + } +} + +// AfterAccept is called by onAcceptedFD after it set up the new connection +// and prepared its first recv, before the worker's next submit. Once the +// trial has a target it reports every accept; the first accept of the target +// number by a worker other than the one holding the close parks that worker +// until [Trial.ReleaseAccept] or HoldMax. Engine hook, worker thread. +func AfterAccept(worker, fd int) { + t := current.Load() + if t == nil { + return + } + target := t.target.Load() + if target < 0 { + return + } + ev := AcceptEvent{Worker: worker, FD: fd} + select { + case t.acceptCh <- ev: + default: + } + if int64(fd) != target || int64(worker) == t.holder.Load() || !t.acceptFired.CompareAndSwap(false, true) { + return + } + t.acceptHeldCh <- ev + select { + case <-t.releaseAccept: + case <-time.After(t.opts.HoldMax): + } +} + +// NoteStaleRecvData records a stale recv completion that carried data while +// a trial is armed. head is copied (at most 64 bytes). Engine hook, worker +// thread. +func NoteStaleRecvData(worker, fd int, gen uint32, res int32, head []byte) { + t := current.Load() + if t == nil { + return + } + if len(head) > 64 { + head = head[:64] + } + t.mu.Lock() + t.stale = append(t.stale, StaleRecv{Worker: worker, FD: fd, Gen: gen, Res: res, Head: append([]byte(nil), head...)}) + t.mu.Unlock() +} + +// wakeHoldNanos is the hypothesis (c) hold. See [SetWakeHold]. +var ( + wakeHoldNanos atomic.Int64 + wakeHolds atomic.Uint64 +) + +// SetWakeHold makes every io_uring worker sleep for d between releasing a +// promoted connection's asyncInMu (after appending received bytes to +// asyncInBuf) and waking or starting its dispatch goroutine; d <= 0 turns it +// off. It widens the hand-off window of hypothesis (c) of celeris#715. +func SetWakeHold(d time.Duration) { + if d < 0 { + d = 0 + } + wakeHoldNanos.Store(int64(d)) +} + +// WakeHolds returns how many times [WakeHold] slept. +func WakeHolds() uint64 { return wakeHolds.Load() } + +// WakeHold sleeps for the duration set by [SetWakeHold], if any. Engine hook, +// worker thread. +func WakeHold() { + if d := wakeHoldNanos.Load(); d > 0 { + wakeHolds.Add(1) + time.Sleep(time.Duration(d)) + } +} + +// hijackWithOpOwed is the hijack witness (celeris#685): hijacks that handed +// the socket over while the kernel still owed the connection an op on it (a +// multishot recv stays armed across its request; a single-shot recv has +// completed by the time its request's handler hijacks). +var hijackWithOpOwed atomic.Uint64 + +// NoteHijackWithOpOwed counts one such hijack. Engine hook. +func NoteHijackWithOpOwed() { hijackWithOpOwed.Add(1) } + +// HijackWithOpOwed returns the hijack witness count since process start. +func HijackWithOpOwed() uint64 { return hijackWithOpOwed.Load() } + +// hijackHoldNanos is the hijack hold. See [SetHijackHold]. +var ( + hijackHoldNanos atomic.Int64 + hijackHolds atomic.Uint64 +) + +// SetHijackHold makes hijackConn sleep for d on the worker thread just before +// it returns the hijacked connection, that is before the worker's next +// io_uring_enter; d <= 0 turns it off. It widens the window in which an op +// the kernel still owes the connection can read the hijacker's first bytes +// (celeris#685). +func SetHijackHold(d time.Duration) { + if d < 0 { + d = 0 + } + hijackHoldNanos.Store(int64(d)) +} + +// HijackHolds returns how many times [HijackHold] slept. +func HijackHolds() uint64 { return hijackHolds.Load() } + +// HijackHold sleeps for the duration set by [SetHijackHold], if any. Engine +// hook, worker thread. +func HijackHold() { + if d := hijackHoldNanos.Load(); d > 0 { + hijackHolds.Add(1) + time.Sleep(time.Duration(d)) + } +} diff --git a/internal/recvtheft/recvtheft_off.go b/internal/recvtheft/recvtheft_off.go new file mode 100644 index 00000000..9e3a01d6 --- /dev/null +++ b/internal/recvtheft/recvtheft_off.go @@ -0,0 +1,55 @@ +//go:build !validation + +package recvtheft + +// Enabled is false in production: the engine's call sites are guarded by it +// and compile away. +const Enabled = false + +// ArmSeq is zero-size in production: a connection carries no recv sequence. +type ArmSeq struct{} + +// Set is the production no-op. +func (*ArmSeq) Set(uint32) {} + +// Get is the production no-op. +func (*ArmSeq) Get() uint32 { return 0 } + +// NoteCloseWithUnsubmittedRecv is the production no-op. +func NoteCloseWithUnsubmittedRecv() {} + +// CloseWithUnsubmittedRecv is always 0 in production. +func CloseWithUnsubmittedRecv() uint64 { return 0 } + +// SubmitBeforeClose is always false in production. +func SubmitBeforeClose() bool { return false } + +// NoteCloseWithLinkedRecv is the production no-op. +func NoteCloseWithLinkedRecv() {} + +// CloseWithLinkedRecv is always 0 in production. +func CloseWithLinkedRecv() uint64 { return 0 } + +// HoldAfterClose is the production no-op. +func HoldAfterClose(int, int, bool) {} + +// AfterAccept is the production no-op. +func AfterAccept(int, int) {} + +// AfterPromoteArm is the production no-op. +func AfterPromoteArm(func() bool) {} + +// NoteStaleRecvData is the production no-op. +func NoteStaleRecvData(int, int, uint32, int32, []byte) {} + +// WakeHold is the production no-op. +func WakeHold() {} + +// NoteHijackWithOpOwed is the production no-op. +func NoteHijackWithOpOwed() {} + +// HijackWithOpOwed is always 0 in production. +func HijackWithOpOwed() uint64 { return 0 } + +// HijackHold is the production no-op. +func HijackHold() {}