Follow-ups from #799: assert the h2c arms' outcome, write down the exit contract, a rerunHandOff witness, an unattributed merged-tree failure, two flakes on main

[FumingPower3925]

The review of #799 (celeris#780) had no blocking findings. It left these minor findings and nits. Each says what to do. Line numbers are at the PR's head, a83e045. The review's probes and logs are copied to evidence/lanes-20260927/EP-3/review-r1/ (correctness/ and evidence/).

1. The h2c arms do not check what should happen instead of the hand-off (minor)

The h2c retry arm and the two landing arms assert only that no hand-off happened (handoff_exit_window_test.go:185-197 and :210-224). They do not assert the intended outcome: the H2C conn stays in its slot, and the queued asyncH2Promoted entry registers it. The body's "the queued entry finishes the upgrade (h2Conns=[6])" is logged but never asserted.

So a "fix" that closes every exiting conn would pass all 9 tests. The review's mutant Mkill replaces if exiting { return } in fd_lifetime.go with if exiting { w.closeConn(fd); return }, and it passes 9 of 9. Its log reads H2C-RETRY adopted=0 reaps=0 h2Conns=[] stale_dirty=false (review-r1/evidence/logs/799-probe-kill-m8.log). With that mutant, every upgraded conn that reaches this path would be killed, and no test would fail.

To do:

• After the drain in the h2c retry arm, assert f.w.conns[f.fd] == f.cs and slices.Contains(f.w.h2Conns, f.fd).
• In the landing arms, assert that the conn still owns its slot and that its recv is re-armed (reapOutcome re-arms it).

2. The exit contract the fix relies on is not written down (nit)

#780's item 3 asked for a whitelist: act only on a hand-off taken from a drained claim. It was not done. The PR blocks the known exits instead: exiting := cs.asyncClosed.Load() || engine.Protocol(cs.protocol.Load()) != engine.HTTP1 (fd_lifetime.go:226, and transplant_source.go:111 for tryTransplant). That is correct for every exit today:

• panic;
• processErr;
• an error after Detach;
• h2c, switched or failed.

A claim that was then refused (asyncRun=false, nothing published) is rightly run again. But the invariant this depends on is not written where a future exit would be added. Every exit other than a claim must publish asyncClosed, or a protocol other than HTTP/1, before endDispatch (conn.go:578) clears asyncRun.

To do: do either of these:

• state the contract in endDispatch's doc comment;
• pin it with a test that drives every exit of runAsyncHandler and checks that rerunHandOff and tryTransplant refuse the conn at each one.

3. The retry arms have no witness that they reach rerunHandOff (nit)

On the fixed tree, the retry arms log reaps=0 adopted=0. retryReaps would log exactly the same if it skipped the conn at one of its early continues (closing || !recvArmed, or a nil drain). Only the review's mutants show the arms are not vacuous today. Mnoexit, Mproto, Mclosed and Mtry are each killed on exactly their own arms (4, 2, 2 and 2 subtests, review-r1/evidence/logs/799-mutants-m8.log).

To do: add a log line or a test counter in rerunHandOff's exiting branch, and assert on it in the retry arms, so the arms stay non-vacuous if the fixture drifts.

4. One unattributed failure on the lane's three PRs merged (nit)

Merged onto main 00d985c (34b1df5), a full -race run of ./engine/iouring in m8 failed once in TestHandoffHasNothingInFlight/async. The failure was client read timeouts (celeris657 load conns=128 ok=86678 errs=39 classes=[read_timeout=39]), not the ENOMEM signature. The review could not reproduce it: 1 failure in 37 runs on the merged tree, against 0 in 27 runs on main. That does not show a regression. The test uses a 1 s client read deadline under -race, and other lanes' containers were running at the time.

To do: include TestHandoffHasNothingInFlight in the cluster row that runs the lane's merged tree, and file a separate issue if it recurs on bare metal.

5. Two flakes on main, not caused by #799 (for the record)

The review could not find either of these filed:

• TestHeldRecvIsReArmedWhenTheHandOffDoesNotHappen/target_refuses (sync mode, which none of the lane's PRs touch). On main 00d985c, -race -count=10 in m8 gave 9 PASS and 1 FAIL: ok=4187 errs=64 classes=[read_timeout=64] held=21 reaps=43. Every one of the 64 conns timed out (review-r1/correctness/logs/main-heldrecv-race-m8.log).
• TestFlapConvergesPollSync (sync, epoll to io_uring promote). It failed once in one adaptive run on the merged tree: FLAPPOLL PLACEMENT: flap 3 (promote) left 8 of 64 conns ... (want <= 2) (review-r1/correctness/logs/combined-adaptive-race-unl.log). It then passed 10 of 10 on both main and the merged tree.

To do: triage each one. File a separate issue for any that reproduces.

[FumingPower3925]

6. CodeRabbit (minor): a parent-commit negative control for the new tests (already met, no action)

CodeRabbit's review of #799 at a83e045 (thread) asked for the new tests to be run against the parent dfd044f: the two retry arms, both landing arms (with h2c_exit_reap_lands_before_the_drain on its own), and both TestTryTransplantLeavesAClosingAsyncConnAlone arms, with the control passing on both trees.

The PR body's "Failing-first" section already has that run. It is main dfd044f with the PR's test file added by -overlay (sha256 in 780/logs/ff-test.sha256), -race -v -count=3, in both m8 and unl:

• all 6 window arms FAIL 3/3 in each shape, h2c_exit_reap_lands_before_the_drain among them;
• the control passes 3/3.

Logs: 780/logs/ff-dfd044f-{m8,unl}.log. The NEG mutant (main's two files over the fix head) repeats it: KILLED on all 6 arms, with the control passing (780/logs/controls-m8.log).

To do: nothing. This is recorded here so the finding has a home.

[FumingPower3925]

7. CodeRabbit (nit): a measurement for the new atomic reads in rerunHandOff

CodeRabbit's second review of #799, at the merge-from-main head 49ac1e7 (review), asked for a -benchmem benchmark or a probatorium measurement of the new atomic reads, on what it called the receive-CQE path through rerunHandOff (fd_lifetime.go:228).

rerunHandOff is not on the per-request receive path. It runs only for a reap retry or a reap landing, and those exist only while an io_uring-to-epoll drain is set. tryTransplant runs after a completion only while w.transplant.Load() != nil. Each function gains one or two atomic loads inside an asyncInMu section it already takes, with no new lock and no read-modify-write (the PR body's "Cost" section).

To do: if the engine guideline is read as requiring a number for every new atomic, add a -benchmem micro-benchmark of rerunHandOff's exiting branch on an fdlFixture conn. The v1.6.0 perf checkpoint covers the steady-state request path.