fix(iouring): never arm a second recv on a connection that already has one (#484)

[FumingPower3925]

Fixes #484

The defect

The WebSocket backpressure pause submits an ASYNC_CANCEL for the armed recv and marks the connection paused. The cancel has not landed yet. A handler that drains below the low watermark before it does makes drainDetachQueue take the resume branch, which called prepareRecv unconditionally — leaving two recvs in flight on one socket.

Both target cs.buf, the per-connection buffer, so the kernel's second write lands on top of the first one's unread bytes. The WebSocket parser then reads a length field at the wrong offset and reports a garbage connection index:

invalid conn index in payload: 2478706466968590878
1 frame parse error(s) observed — frames were corrupted

Multishot is immune, which is why the bug only ever showed on the default single-shot path: each completion carries its own provided-ring buffer, so two in-flight recvs cannot overwrite each other.

How it was found

A detector in prepareRecv that logs whenever it is asked to arm a recv on a connection that already has one:

LEDGER double-arm fd=71  gen=33 bufring=false paused=true needsRecv=false inflight=2
LEDGER double-arm fd=164 gen=57 bufring=false paused=true needsRecv=false inflight=2

paused=true while the arm is in progress pins the call site to exactly one place: the resume branch assigns cs.recvPaused = desired after arming, so the log still sees the old value. The SQ-ring-full path that skips the cancel fired 0 times in the same runs, so a full submission queue is not involved.

The change

• prepareRecv declines the second arm and reports success. "A recv is armed for this conn" is the postcondition every caller acts on, so returning false would make them set needsRecv and retry from the dirty list forever. The driver path at engine/iouring/driver.go has carried the same guard since it was written.
• The dirty-list retry gets an explicit recvArmed check of its own, because it calls pickRecvTarget, which mutates cs.recvIntoBody. Computing a target for an arm that is about to be declined would route the in-flight recv's CQE through the direct-body path.
• New connState.recvCancelPending. A pause withdrawn before the cancel lands used to leave -ECANCELED arriving with recvPaused already false, so handleRecv fell through to the generic negative-result path and closed a healthy connection. The flag marks a cancel this worker asked for so that CQE re-arms instead. Every other cancelConnOps caller nils w.conns[fd] or sets closing first, so no other cancellation can reach this branch.

Measurement

io_uring single-shot variant run on its own, which reproduces far more readily than the three-variant test. 24 runs each, 4 workers, memlock 128 MiB, seccomp=unconfined.

	before	after
runs reporting frame corruption	2 of 24	0 of 24
total single-shot failures	8 of 24	4 of 24
double-arm detector firings	6	1

The four remaining failures all report parseErr=0. They carry closeTimeout and per-connection protocol errors, plus one frame-count mismatch — a different signature from the corruption this PR removes. Those are being characterized separately and will get their own issue; this PR does not claim to address them.

engine/iouring/recv_arming_test.go fails on a control build with the guard removed (ring pending 1→2) and passes with it.

[FumingPower3925]

Correcting the measurement table in the description

The before/after table above is wrong and I am withdrawing it. The mechanism, the fix and the unit-test negative control all stand; the failure-rate numbers do not.

The trap. inbound_sequence_linux_test.go prints its counter summary at line 324, and calls settle() — which drains the handler goroutines — at line 331. The assertions read the settled counters at line 338 and after. Every number I quoted came from that summary line, so it is a snapshot taken before the drain and it understates parseErr. Six runs whose summary said parseErr=0 went on to fail the assertion at line 340 with "frame parse error(s) observed". The authoritative signal is the assertion line, not the summary.

Re-measured properly, 24 runs of the single-shot variant on a clean tree carrying only this change, capturing every test log line:

	count
single-shot runs failing	6 of 24
...reporting invalid conn index in payload	0
...reporting parse error on conn N: unexpected EOF	6 (7 connections total)
multishot runs failing	0 of 24

So the two signatures separate cleanly. invalid conn index in payload: <garbage 64-bit value> is the mid-stream corruption this PR is about: a length field read at the wrong offset, which is exactly what two kernel writes into one cs.buf produce. It does not appear once in 24 runs here. unexpected EOF is a different thing — the frame stream ends part-way through a frame, which is truncation at teardown, not a wrong offset.

I do not yet have a clean-tree baseline measured the same way, because the containers holding the earlier logs were removed before I noticed the summary-line problem. A 24-run baseline on origin/main with identical capture is running now. Do not merge until that lands — the mechanism is proven and the unit test is a real regression guard, but the claim about the failure rate needs a like-for-like comparison behind it.

The unexpected EOF residual is a separate defect and will get its own issue rather than being folded in here.

[FumingPower3925]

Like-for-like baseline, and what it does and does not show

24 runs of the single-shot variant on a clean origin/main tree, same capture script, tallied from the assertion lines rather than the summary:

	clean main	this PR
single-shot runs failing	5 of 24	6 of 24
...reporting invalid conn index in payload	0	0
...reporting parse error on conn N: unexpected EOF	6	6
multishot runs failing	0 of 24	0 of 24

So the corruption signature does not reproduce on either clean tree at this sample size. The two runs that showed it were on the ledger-instrumented build, where the detector's stderr writes changed the timing enough to widen the window — the same instrumentation moved the multishot failure rate from 0 of 24 to 5 of 24. I am not going to claim a failure-rate improvement from a signature that the baseline never produced.

What the measurement does support, and what this PR rests on:

1. The double arm is real and was observed directly. The detector fired five times across instrumented baseline runs, each time with paused=true, which places the call at the resume branch and nowhere else. It fired once with the guard in place, and the guard declined it.
2. Two recvs into one cs.buf is a defect on its own terms. Whether it manifests once in twelve runs or once in a thousand, the kernel writing into a buffer whose earlier contents have not been read is not something to leave in the engine. The driver path has always refused it.
3. The negative control is decisive. recv_arming_test.go fails with the guard removed (ring pending 1→2) and passes with it.
4. The -ECANCELED path is a separate real defect fixed here: a pause withdrawn before its cancel landed made handleRecv close a healthy connection.

The unexpected EOF residual sits at 5-6 of 24 on both trees, so it predates this change and is not affected by it. It gets its own issue.

Race check: the full websocket suite under -race reports no DATA RACE and no deadlock. It does fail two tests on the protocol error on unfailed client signature; a -race baseline on clean main is running to confirm that is also pre-existing.

[FumingPower3925]

Race baseline: the -race failures are pre-existing

The full websocket suite under -race on clean origin/main also fails:

--- FAIL: TestBackpressureInboundSequenceIntegrity/io_uring/multishot_recv (17.80s)
    conn 29: frame count mismatch: in=62710, sent=64000
    conn 67: frame count mismatch: in=59470, sent=64000
FAIL  github.com/goceleris/celeris/middleware/websocket  995.195s

Neither run reports a DATA RACE or a deadlock — not on this branch and not on main. The suite is simply flaky under -race at this concurrency, on both trees, and the failing subtest differs run to run.

One caveat I am not going to paper over: on the branch, the -race run failed TestBackpressurePauseDoesNotCancelInflightSend/io_uring with protoErr=1, and the baseline run did not. That is one observation each, and that test exercises exactly the pause/cancel path this PR touches. A targeted 24-run comparison of that test alone is running on both trees now. I will hold the merge until it comes back.

The unexpected EOF residual is now #562. It is io_uring-single-shot-specific: 5 of 24 on clean main, 6 of 24 here, 0 of 24 on multishot, 0 of 24 on epoll.

[FumingPower3925]

Targeted control on the pause/cancel test: pre-existing, and no worse here

TestBackpressurePauseDoesNotCancelInflightSend/io_uring run 24 times on each tree, on its own:

tree	failures
clean origin/main	4 of 24
this branch	3 of 24

Same assertion on both, N handler(s) saw a protocol error on the read side, with N ranging 1–6 on main and 1–16 here. The single -race observation that made me hold the merge does not survive a real sample: the test is flaky on main at roughly the same rate, and this branch is if anything marginally better.

That was the last thing gating this PR. Summary of what the merge rests on:

• The double arm was observed directly, five times, always with paused=true, which places the call at the resume branch and nowhere else.
• Two recvs armed on one socket both write into cs.buf. That is a defect independent of how often it surfaces, and the driver path has always refused it.
• recv_arming_test.go fails with the guard removed (ring pending 1→2) and passes with it.
• No DATA RACE and no deadlock under -race, on either tree.
• No failure-rate claim. invalid conn index in payload is 0 of 24 on both clean trees; it only appeared on the instrumented build, whose stderr writes widened the window.

The unexpected EOF residual is #562 and the protocol-error flake above is separate from both.