fix(net): a warm copy's expiry no longer aborts the relay's latest group

[Dryvnt]

Problem

On release, a relay can lose the latest group of a track whose newest group has been idle for longer than the cache expiry (cache::DEFAULT_EXPIRY, 30 s). After that, every new subscriber skips that group and waits for one that may never come. A hang catalog is exactly such a track: one group per catalog version, written only when the catalog changes. Once this fires, every viewer who joins later gets no catalog and therefore no tracks, while the broadcast still looks announced and healthy. Only restarting the publisher or the relay clears it.

The trigger is everyday operation. A relay keeps a track up for one session while a second session, speaking moq-lite, reads the latest group and leaves after that group has been idle for more than 30 s. The second session might be a restarted service, or a browser tab closed after half a minute.

The leaving session's front parks the track. warm_copy (model/origin.rs) adopts the cached groups onto a new local track, sharing the relay copy's group::Producer. Nobody reads that warm copy, so it is dropped and its track finishes. The cache sweep then calls TrackState::expire_closed on the finished track, which aborts every group idle past the expiry. That aborts the shared producer, which the relay's live copy also holds.

main is not affected: #4741 removed warm_copy. #4741 is too broad to backport (see Alternatives), so this fixes the line itself.

Approach

The pool's rule is that only a live track's latest group is exempt from idle expiry. With shared groups, that has to hold across tracks, so the group now counts the live tracks holding it as their latest: the latest slot holds a cache::Protection guard, dropped when the slot is demoted, removed, or the track closes (close_cache). expire_closed aborts an idle group unless that count is above zero, so an ended warm copy leaves the relay's live edge alone, and groups only ended tracks share still expire.

The regression test is #4923's leaving_after_the_cache_window_keeps_the_latest_group, ported to release's rejoin.rs with tokio's paused clock in place of moq_net_sim (which release lacks). It runs over moq-lite-05, 06 and 07-wip, and moq-transport-19 and 22.

Impact

• Public API: none. Wire: none.
• Each cached slot and each group's cache::Access grow by one word (track::CACHE_OVERHEAD follows Slot's size).

Verification

• Without the fix: leaving_after_the_cache_window_keeps_the_latest_group fails (moq-lite-05: reader 5 never got the latest group).
• With it: the test passes for every version, and just check origin/release passes.
• closed_track_expires_a_shared_group_unless_live_edge (unit): an ended track keeps a shared group while it is a live track's latest and expires it once demoted; a group only ended tracks share expires while stale consumers hold them all. It fails on release as shipped (the live edge expires) and with abort_if_last (the ended tracks pin it).
• Relay over real QUIC: a moq-relay test where a fresh reader joins after an idle catalog reader has left fails without the fix and passes with it. It is not included here because it takes about 100 s.

Alternatives

• Backporting fix(net)!: resume route changes by reading the routes' copies; a path is one broadcast #4741: semver-breaking, and it stalls un-epoched republishes. It is the reason the next release from main is gated on the broadcast-epoch line.
• abort_if_last in expire_closed (skip a group any other handle owns): one line, but a group shared by two ended tracks then stays pinned for as long as stale consumers hold both.

Follow-ups

• The back-merge into main should keep main's side of this change (expire_closed and the protection count), since main has no warm_copy and test(net): a reader leaving after the cache window keeps the relay's latest group #4923 already covers it there.
• moq-gst's a_stale_completion_cannot_end_the_next_publication failed once locally on release and passed on every rerun; it looks flaky and is unrelated to this change.
• Requesting this for the 0.17.3 cut: it is a regression from 0.14.x relays, whose moq-net 0.2.x has no warm_copy.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 6c2b733

• [P2] rs/moq-net/src/model/track.rs:771: abort_if_last counts producer handles, not live track owners. After two fronts park and resume, their finished warm tracks can remain as resume segments, each holding the same completed group. Once the source also ends, retained consumers keep those closed track states alive. Every sweep then sees multiple slot-held handles and skips the abort; no closed slot relinquishes its clone, so idle payloads never expire despite having no live owner. Exempt actual live ownership or safely release closed slots' references. Add a regression that adopts one finished group into two tracks, retains both consumers, drops all producers, and verifies reclamation after multiple expiry windows.

Overall direction: the focused release fix is appropriate, but it must preserve ended-track cleanup as well as the live relay's latest group. No API, wire, or dependency changes are needed.

Verification: source/lifecycle inspection only. GitHub Check/Test, Android, Platform, and WASM passed; the Test log confirms the new regression passed on merge commit 5188ebf. I did not independently run tests, the two-closed-owner reproducer, or real QUIC.

[Dryvnt]

Re the automated review's P2 on rs/moq-net/src/model/track.rs:771:

Confirmed: with this change, a finished group shared by two closed tracks that stale consumers keep alive never expires (reproduced: adopt into a second track, finish both, keep consumers, sweep).

It's narrower than the bug fixed here: every owner must be closed, it pins only groups the live copy hadn't evicted before it ended, and they go once all but one holder drops. A full fix needs the group to know whether a live track still holds it as its latest (a per-group count kept by insert, demotion and close_cache). I'd rather not put that on release days before the cut; main has no warm_copy, so the case goes with the next release from main. Happy to add it here if you'd rather.

(Written by Claude Opus 5.5)

[kixelated]

Automated review of 6c2b733b (first Grok review on this PR)

A one-line fix on release for a real field bug: a warm copy that nobody reads finishes, the cache sweep's expire_closed aborts its idle groups, and because the warm copy shares the relay copy's group::Producer, that abort kills the relay's latest group too. For a catalog track that means every later viewer gets no catalog. Switching to abort_if_last (the same rule HeldGroup::drop uses) is the smallest correct fix, and the ported regression test covers lite-05/06/07-wip and transport-19/22. CI is green (Check, Test, Windows, macOS, WASM, Android).

Findings, all minor:

1. abort_if_last counts every Producer clone, not just track owners (group.rs:779-784). Arc::strong_count(&self.alive) > 1 is also true while any short-lived clone exists (an in-flight serve or write task). In that window the finished track skips the abort, so the group's frames live until the next sweep or until the finished track's slots drop. That's a delayed release, not a leak, as long as the slot stays in evict and is retried on the next sweep. Worth a one-line comment in expire_closed saying a skipped slot is retried (or released when the track drops), so nobody "fixes" it later by removing the slot.
2. Two finished owners never abort each other. If the relay copy also finishes while the warm copy still holds the group, both expire_closed calls see a count of 2 and skip it. Frames are then released only when one TrackState drops (Alive's drop ends the group). That's fine if a finished track's state is dropped soon after the sweep, but the test doesn't cover it. A short assertion that the group closes once both tracks are gone would pin it down.
3. Back-merge note. The PR body already says main should keep its own expire_closed. Since main has no warm_copy, taking this hunk there would be harmless but misleading. Make sure the release-to-main merge resolves to main's version.

Verdict: MERGE (head 6c2b733b). A good candidate for the 0.17.3 cut.

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

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 2564923

Compared with 6c2b733: one direct follow-up commit, unchanged base and production behavior; the new lifecycle test and comments clarify the tradeoff. No new findings.

• Still open [P2]: the previous closed-owner retention finding at rs/moq-net/src/model/track.rs:772 is documented, not fixed. The new test at lines 7309–7317 deliberately keeps the shared group past expiry, then drops one stale consumer before checking reclamation. It establishes recovery when an owner disappears, but two retained closed tracks still keep the payload indefinitely. This matches the author's confirmed reproducer and deferral. Resolving it requires distinguishing live ownership from closed-slot handles and checking payload reclamation while both stale consumers remain.

Overall direction: the focused release fix addresses the catalog outage; shipping with the remaining P2 is an explicit maintainer tradeoff. No broader API or wire change is needed.

Verification: source/lifecycle inspection only. Current-head Check/Test, Platform, WASM and Android CI are still running; the earlier green run does not validate this new test. I did not independently run tests or real QUIC.

[Dryvnt]

The P2 is now fixed in f9b7982 rather than deferred. A group counts the live tracks holding it as their latest, and expire_closed aborts an idle group unless that count is above zero, so groups only ended tracks share expire even while stale consumers hold them all. closed_track_expires_a_shared_group_unless_live_edge covers both halves.

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: f9b7982

Compared with 2564923: one direct follow-up commit, unchanged release base; substantive live-edge ownership fix.

Fixed: the previous P2 closed-owner retention finding. Protection now follows live latest slots and is released on demotion, close, and removal. rs/moq-net/src/model/track.rs:7293–7338 checks live-edge preservation, expiry after demotion, and expiry while both stale consumers remain. No new actionable findings.

Overall direction: the scoped guard approach addresses both the catalog outage and ended-track cleanup, which the simpler producer-handle check could not. No public API, wire, or dependency changes; no broader backport is needed for this fix.

Verification: GitHub/source and lifecycle inspection only. Current-head Android CI passed; Check/Test, macOS, Windows, and WASM are still running. I did not independently run tests, concurrency models, or real QUIC.

[kixelated]

Merge summary for head f9b7982e018f40a225149579dd4ca61ecf9cef13:

• Fix: a group counts the live tracks holding it as their latest, via a cache::Protection guard on the latest slot (released on demotion, removal, and close_cache). expire_closed skips an idle group only while that count is above zero, so an ended warm copy no longer aborts the relay's live edge, and groups shared only by ended tracks still expire.
• Reviews: the OpenAI review of this head reports the earlier P2 (closed-owner retention) fixed and no new findings. Grok's findings on 6c2b733b targeted the superseded abort_if_last approach.
• CI: Check, Test, Android, WASM, Windows, and macOS pass on this head.
• Forward-port: none needed. main has no warm_copy and already carries test(net): a reader leaving after the cache window keeps the relay's latest group #4923's leaving_after_the_cache_window_keeps_the_latest_group. The release-to-main back-merge should keep main's side of expire_closed.
• Public API and wire: unchanged.

Enabling auto-merge (merge commit, as release PRs use).

(Written by Claude Opus 5.5)