Repository navigation
fix(net): backport withdrawing IETF advertisements without send credit to release - #7922
Conversation
Backport of #7414's behavior. The unsolicited announce loop awaited each PUBLISH_NAMESPACE and REQUEST_UPDATE write, so a peer withholding send credit pinned it, along with every withdrawal queued behind. Buffer the message instead and write and read it from the loop's own event wait: withdrawing a request still writing or awaiting its answer resets it in both directions, and the answer deadline starts once the message is written. Hold an early REQUEST_OK until the write completes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 55000ad
No actionable introduced bugs found. Buffering and driving writes from the namespace event loop is a reasonable backport of #7414; the distinct writing and answer states and cancellation handling appear justified by the stalled-credit cases. The change keeps the release API rather than introducing a larger redesign.
Static source review only; I did not run tests. GitHub Check and Release gate were queued when inspected.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
I read the complete diff and traced the new per-request announce machinery end to end, focusing on the waker/deadline ordering and on whether the recorded announce state can diverge from what the peer actually holds. The release base's peer-FIN/reset-ends-the-loop behavior and the drafts 14-16 control-stream path both hold up.
- Buffered announcements —
advertise_namespace/update_namespacenowbuffertheir message and recordPending::Writinginstead of awaiting the write and the answer, so a stalled namespace no longer pins the loop or the withdrawals behind it. - Per-request polling —
Woken/RequestWaker/poll_answers/poll_requestflush and read each woken request on its own stream, with the answer read even while the write is stalled;Framedpreserves the[type][size][body]framing and the existing stats accounting. - Answer deadline after the write —
Pending::Answer(at)is only set oncepoll_flushcompletes, tracked ininflightand enforced by thelatedeadline; backpressure alone never times a request out. - One message in flight — the
sync_namespaceearly return coalesces route changes behind the in-flight message, an earlyRequestOkis held until the write flushes, andansweredapplies OK/refusal/Lateby re-syncing, terminating gracefully, or resetting both directions. - Withdrawal — drafts 17+ reset both directions when a message is in flight; drafts 14-16 keep
PUBLISH_NAMESPACE_DONEbecause their virtual writes never wait for credit. - Tests —
ScriptedSession::with_send_windowplus woken scripted readers, the staged withdrawal/refusal/FIN/reset matrix over drafts 17-22, the write-only deadline, and the early-OK hold.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

Problem
On
release, the unsolicited announce loop awaits every PUBLISH_NAMESPACE and REQUEST_UPDATE write. A peer that withholds send credit on one request stream pins the loop on that write, so withdrawing the namespace, and every other announce, withdrawal, and retry behind it, never runs for the rest of the session.Reproduced on
release(f48874e434) with the new test below: withdrawing a draft-17 advertisement written with zero credit produced no reset at all (moq-transport-17, FirstWrite, partial=0, Local:left: [],right: [1]).Tracking quest: quest/m0/backport-release-hardening.md, claimed in moq-dev/moq#7596. Backports moq-dev/moq#7414.
Approach
Hand-port of #7414's behavior;
release's publisher is async and lacks main's per-request event machinery, so the layout differs.advertise_namespaceandupdate_namespacebuffer their message and record it as pending instead of awaiting the write and the answer.ADVERTISE_TIMEOUT) starts once the message is fully written. Backpressure alone never times a request out.UnexpectedMessage.Impact
sh/api-diff/run.sh --deny rustagainstorigin/release: see Validation.Alternatives
ADVERTISE_TIMEOUT, like the open and the answer read: it still holds the whole loop for the timeout per stalled write, and times out a peer that is merely slow to grant credit. fix(net): withdraw IETF advertisements without send credit #7414 settled that backpressure alone starts no timeout.Choices made without asking
release's handling of a request stream that the peer FINs or resets before answering: it still ends the announce loop with an error. Main ends only that advertisement, a change that predates fix(net): withdraw IETF advertisements without send credit #7414 there; the new test pins that the stall no longer hides the peer's end.releasehas no advertisement epochs or auth links.releasespeaks (17 through 22).ScriptedSession::with_send_window(each bidi stream sends only its first N bytes, then parks for good) and readers parked on an exhausted script are woken bypushandclose.Validation
Head
0fcb088cb93ad7ce138b23e58df60086fa77a597mergesorigin/releaseat6d6f264492(after moq-dev/moq#7920) into55000adf43. The merge was conflict-free and touched onlymoq-relayand its doc;git diff origin/release 0fcb088cb9is byte-identical togit diff f48874e434 55000adf43. Each run throughquest gate nix develop --command ...:just ci check origin/release: pass.just ci test origin/release: pass. Rust: 5189 passed, 11 skipped; 387 passed, 3 skipped; 133 passed (moq-cli). JS suites pass.releaselackssh/api-diff/; ranmain'ssh/api-diff/run.sh --denyagainst this head): Rust passes, all 30 published cratesunchanged, includingmoq-net0.3.20. JS reported every@moq/*surface asnew, not a break:main's renderer drew no baseline fromrelease, so it compared nothing. The PR touches no JS. FFI could not run:main'sffi.shbuildsmoq-c, whichreleasenameslibmoq. The PR touches neitherrs/libmoqnorrs/moq-ffi, and the Rust diff showsmoq-net's API unchanged.Earlier, on
55000adf43:release,stalled_advertisements_observe_local_and_peer_endsfails at its first case,moq-transport-17, FirstWrite, partial=0, Local: no reset (left: [],right: [1]). With the fix it passes for drafts 17 through 22, every stage (initial write, initial answer, update write), zero and one byte of credit, and each end (local withdrawal, refusal, FIN, reset).an_unanswered_advertisement_is_given_up_at_its_deadline) and the early-OK hold (an_early_ok_admits_nothing_behind_a_stalled_write).a_silent_answer_still_lets_the_next_namespace_be_advertisednow requires both advertisements without advancing time.just rs uring: pass. 333 passed, 2 skipped, plus 78 passed.quest check --base origin/release: pass.Review
Codex,
gpt-6.1-sol, medium effort, round 1 (quest review --base origin/release). Claude Code wrote this change, so Codex is the other vendor. Reviewed head:55000adf43cf6a4b3ab10639e0c36c1c609e286a. No findings. The current head0fcb088cb9adds only a conflict-free merge oforigin/release, which leaves the PR's diff identical, so it needs no new review.Follow-ups
None identified. This PR belongs in the already planned combined patch release; the maintainer chooses versions and cuts it separately.
(Written by Claude Opus 5.5)
🤖 Generated with Claude Code