[kixelated]

Problem

track::Producer::abort cleared the whole cache unless the declared end had already settled (#4351), and so did a producer dropped without finish(). A reader slower than the network lost groups that had fully arrived before the abort: the publisher's session dies one group short of the end, and the relay's readers skip everything they had not pulled yet.

The clearing existed so a stale consumer could not pin the cache forever. That guard was also incomplete elsewhere: an ended track's latest group was always exempt from the pool's idle expiry, and the pool sweep never reached an ended track at all. Its channel is closed, so the account's kio::Weak::upgrade returned None. A stale consumer on a finished track whose producer was dropped kept every one of its groups.

Approach

• commit_abort and the producer-drop teardown keep the finished groups and drop only the open ones, which nobody will finish. A consumer that already pulled an open group keeps its own handle, as before. Datagrams stay too, bounded at 64.
• poll_next_in_range (the ordered and spliced cursor) returns cached in-range groups before the abort, matching poll_recv_group, which already did. Readers get what finished, then the abort, or a clean end when the end had settled. The settled special case collapses into this.
• Once no producer remains (aborted, sealed, or dropped), the track is closed: the latest group joins the eviction order, so idle expiry reclaims it like any other group.
• The pool sweep reaches an ended track through a new kio::Weak::read, and TrackState::expire_closed aborts idle groups in place, since the closed channel refuses a write. That releases their frames; the leftover slots are skipped by every read path.

The pool's idle expiry (DEFAULT_EXPIRY, 30s for every origin and relay) is the bound. A pool with expiry disabled keeps idle content, as it does today for finished tracks.

Tests

• abort_keeps_finished_groups_for_a_slow_reader: finished groups, then the abort, on both the arrival and ordered cursors; the open group is gone. It fails on main.
• dropped_producer_keeps_finished_groups_for_a_slow_reader: the same for a dropped producer. It fails on main.
• closed_track_expires_its_latest_group: two sweeps reclaim the latest group of an aborted track and of a sealed one held by stale consumers, while a live track keeps its latest. It fails on main for the sealed track, which the sweep never reached.
• kio weak_reads_a_closed_channel.
• Updated abort_drops_open_groups (was abort_clears_cached_groups), teardown_ignores_a_settling_group (now uses an open group, which the teardown still drops), and moq-json's a_dropped_failed_edit_aborts_the_track (the subscriber now drains the finished snapshot before FrameTooLarge).
• cargo nextest run --workspace: 5073 passed. just check passes.

Impact

• Public API: adds kio::Weak::read, which reads the state while another handle keeps it allocated, closed or not, without counting as a producer or consumer. Nothing else.
• Behavior (Rust): an aborted track, or one whose producer dropped without finish(), keeps its finished groups readable, and consumers see them before the error. Open groups are still dropped. An ended track's groups, its latest included, are reclaimed by the pool's idle expiry.
• Wire: none.
• JS: not changed; @moq/net has no equivalent yet and needs a separate look.

Refs #4351, #4361.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

[coderabbitai]

Warning

Review limit reached

Next included review available in 12 minutes.

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: 3f1ba53c-387c-4ad5-8959-636d4a5ea4bf

📥 Commits

Reviewing files that changed from the base of the PR and between ca47661 and df27928.

📒 Files selected for processing (4)

• rs/kio/src/weak.rs
• rs/moq-json/src/snapshot/mod.rs
• rs/moq-net/src/model/cache.rs
• rs/moq-net/src/model/track.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.}

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-28T17:43:39.514143Z	df27928	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[kixelated]

MERGE

Positive improvement, worth the complexity, and the approach looks right for this lifetime/expiry corner of the cache.

Positive improvement

Yes. Two real bugs, both with regressions that fail on main:

1. Slow readers lost finished work on abort/drop. commit_abort / producer-drop used to clear the whole cache (unless the end had already settled). A publisher dying one group short of the end, or a relay reader behind the network, then skipped finished groups that had already arrived. Keeping finished groups and dropping only open ones, and letting poll_next_in_range drain the cache before surfacing the abort (matching poll_recv_group), is the correct consumer-facing behavior for live fanout.

2. Ended tracks could pin RAM forever. An ended track's channel is closed, so the pool sweep's upgrade returned None and never touched it; the latest group was also always exempt. A stale consumer on a sealed/aborted track held every group. closed + demoting the latest into eviction order, kio::Weak::read, and expire_closed close that hole without reopening the channel.

Worth the complexity

Yes. The surface area is small and local:

• drop_open_groups / close_cache / protects replace the blunt clear_cache + settled special case with a clearer rule: finished stays, open goes, live edge is unprotected once no producer remains.
• Weak::read is a minimal public API (&T while allocated, closed or not, no producer/consumer count) with a focused unit test.
• expire_closed aborting in place (instead of removing slots) is the right tradeoff under a closed channel that refuses writes; read paths already treat aborted slots as absent, and frame charges still release via group.abort → release → charge.clear. Same track-then-group lock order as the existing evict_expired_scan abort path.

Idle expiry (DEFAULT_EXPIRY, 30s) as the bound for retained finished groups is the right policy: it reuses the pool's wall-clock LRU instead of inventing a second retention path. Leaving datagrams (already capped at 64) is consistent with "keep what finished."

Different approach?

Not really. Clearing on abort and only special-casing settled is what caused #4351. Holding a strong handle just so the sweep can write would be heavier than Weak::read + in-place abort. Collapsing sealed/abort into a single "ended" flag might tidy naming later, but it is not needed for this fix — closed is the right orthogonal bit ("no producer remains → latest is evictable").

Nits (non-blocking)

• drop_open_groups leaves stale arrival / evict hints for removed open groups; existing trim/skip logic handles that, same as other remove paths. A follow-up retain/trim there would be cosmetic only.
• After removing an open latest, latest_group can briefly name a missing sequence until nothing cares; protects is false once closed, so eviction is fine. Clearing latest_group when the slot is gone would make the invariant prettier, not required.
• CI was still pending at review time; author reports workspace nextest + just check green. Worth a glance at the checks before merge.

Refs #4351, #4361.

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Merge summary

Merging as-is at df27928; no changes pushed.

• CI: Check, Test, WASM, Android all green. Branch is two docs/tooling commits behind main with no conflicts, so no merge from main was needed.
• Codex: thumbs up on df27928, no findings.
• CodeRabbit: rate-limited, no review.
• AI review nits (non-blocking), not addressed: stale arrival/evict hints after drop_open_groups, and latest_group briefly naming a removed open group. Both are already tolerated by the existing trim/skip paths and protects is false once closed, so they are cosmetic.
• Follow-up: @moq/net has no equivalent of this abort/expiry behavior yet (tracked alongside fix(net): age out an ended JS track's groups, match Rust abort semantics #4385).

Maintainer approved merging in-session.

(Written by Opus 5.5)