fix(srt): keep the listener alive until a rejected caller sees the verdict

[kixelated]

Problem

dial::tests::publish_rejection_carries_unauthorized_code and its sibling subscribe_rejection_carries_unavailable_code fail intermittently in CI with TimedOut instead of ConnectionRefused (e.g. run 35731120127, on a PR touching no SRT code).

Root cause

ConnectionRequest::reject in srt-tokio only pushes AccessControlResponse::Rejected onto the listener's response channel. The rejection packet is sent later, when SrtListenerState::run_loop drains that channel. That loop's WaitForInput arm selects the response channel against close_recvr, which resolves as soon as the SrtListener is dropped.

The test moved the Server into a spawned task that returned the instant reject() did, so the listener was dropped with the verdict still queued. When the run loop woke, both branches were ready and it could break out before transmitting. The caller then saw nothing and hit the SRT connect timeout.

Instrumented run showing it:

[3.466ms] rejecting publish
[3.554ms] rejected publish
[3.566ms] server task done (dropping listener)
[3.004s]  caller returned io:            <- TimedOut

The two accept tests are structurally immune: request.accept() awaits settings_receiver, which only resolves once the listener has acted on the request, and the task then keeps the Server alive for the connection's lifetime.

Candidates ruled out

• free_udp_addr() port race: cannot produce this symptom. srt-tokio's bind_socket sets no SO_REUSEADDR/SO_REUSEPORT, so a stolen port fails Server::bind(...).unwrap() loudly rather than silently swallowing datagrams. Left as is; handing the pre-bound socket to SrtListener via .socket() would bypass srt-tokio's own bind_socket and silently drop its 64 KiB UDP send/recv buffer sizing, which is not worth it for a test-only benefit.
• Reject racing the connect timeout under load: the server rejected at 3.5 ms against a 3 s timeout. Not a margin problem.

Fix

Drive the server with tokio::join! instead of a spawned task, so the Server stays owned by the test body for the whole handshake, and document on both reject() methods that the verdict is handed to the listener rather than sent, so the Server has to outlive the client's handshake.

moq_srt::run was already correct: its Server lives for the entire accept loop, so no production path rejects and then tears the listener down.

Verification

	before	after
suite, 4 test threads	19/20 failed	0/20
suite, 4/8/16 threads, 12 busy cores	—	0/60
cargo nextest run -p moq-srt, 16 threads, loaded	—	0/15

just check and just test pass.

Public API and wire impact

None. Doc-comment text only on Publish::reject / Subscribe::reject; the rest is test code. No wire change.

Regression test

The fixed test is the regression test: it was already asserting the right thing and now does so deterministically. There is no separate production defect to cover, since the only reject-then-drop sequence was in the test.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

Next included review available in 1 minute.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 7a32d564-d4e1-44d3-9fc5-7ca71b87dfa4

📥 Commits

Reviewing files that changed from the base of the PR and between 296295b and 466b177.

📒 Files selected for processing (2)

• rs/moq-srt/src/dial.rs
• rs/moq-srt/src/server.rs

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[moq-bot]

No issues found.
Verdict: approve
(Written by Muse Spark)

opencode session  |  github run

[kixelated]

Assessment

• Positive improvement? Yes. Clear root cause for intermittent TimedOut instead of ConnectionRefused: reject() only queues the verdict, and spawning a task that returned immediately dropped the Server before the listener flushed the packet.
• Worth the complexity? Yes. Tiny, targeted change—tokio::join! so the listener outlives the handshake, plus accurate docs on Publish::reject / Subscribe::reject. Production moq_srt::run already kept the server alive; this only fixes the flaky test pattern.
• Different approach better? Pushing a flush-before-close into srt-tokio would be heavier and outside this crate’s scope. Fixing the test ownership (and documenting the contract) is the right layer.

MERGE — deterministic flake fix with no production behavior change.
This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Auto-merge enabled (squash).

Summary of this PR and the merge checks:

• Change: test-only fix in rs/moq-srt. ConnectionRequest::reject only queues the verdict on the listener's response channel, and the old spawned server task dropped the Server (and its SrtListener) before the run loop drained it, so a rejected caller saw TimedOut instead of ConnectionRefused. The tests now use tokio::join! so the Server outlives the handshake, and the lifetime rule is documented on Publish::reject / Subscribe::reject.
• Files: rs/moq-srt/src/dial.rs, rs/moq-srt/src/server.rs (1 commit, no production path change, no wire change).
• CI: Check, Test, and meta-review all green; mergeable and CLEAN against main.
• Reviews: moq-bot approved ("No issues found"), CodeRabbit was rate-limited with no findings, automated maintainer review returned MERGE.

This also unblocks #3730, whose Test check was failing on this exact flake.

(Written by MiMo)