fix: release upstream subscriptions for idle cached tracks, and stop FIN-ing subgroup streams mid-object

[englishm-cloudflare]

What this changes

Two independent bugs found while working on #191, plus cleanup.

Fixes #191.

1. fix: reset subgroup streams instead of FIN-ing mid-object

quinn::SendStream::drop calls finish() — a clean FIN at the current offset. subscribed.rs built SubgroupOutput::Stream(Writer::new(send_stream)) and never called finish() explicitly, so every early return from the forwarding loop sent a FIN wherever it happened to stop.

When that offset is inside an object, the receiver has already been promised payload_length bytes by the object header. It sees a subgroup that ended cleanly mid-object and treats the track as malformed.

Two triggers in a relay:

• Downstream UNSUBSCRIBE — recv_unsubscribe → remove_subscribe → ObjectForwarderRecv dropped → the state.lock_mut().ok_or(ServeError::Done)? in serve_subgroup_objects fails after the object header was already encoded → Writer dropped → FIN mid-object.
• Upstream failing mid-object — SubgroupObjectWriter dropped with remain != 0 sets ServeError::Size, producing the same truncating FIN downstream.

draft-16 §10.4.3 permits a FIN only when every object in the subgroup was delivered, and explicitly lists early termination due to UNSUBSCRIBE as a case that MUST use RESET_STREAM.

The fix:

• A SubgroupStream wrapper that resets on drop unless explicitly finished, so the spec-safe outcome is the default and async cancellation is covered without a separate code path.
• Track owed payload bytes and refuse to FIN while mid-object.
• Move the liveness/location check ahead of the object header encode, so a cancellation in that window stops on an object boundary rather than after the promise.
• Add the §13.4.4 data stream reset codes (DataStreamResetCode) and map failures onto them: CANCELLED for a subscription ending early, MALFORMED_TRACK for an upstream object shorter than declared, SESSION_CLOSED / INTERNAL_ERROR otherwise.

The regression test was verified to actually catch the bug: reverting only the check reordering makes unsubscribe_mid_subgroup_resets_at_an_object_boundary fail with "no partial object should follow the last complete one".

2. fix: release upstream subscriptions for idle cached tracks (#191)

A relay caches tracks it does not publish itself so several downstream subscribers can share one upstream subscription. Nothing counted how many subscribers were actually using a cached reader, so the upstream subscription lived until the upstream session died — after the last subscriber left, the relay kept receiving a track nobody was watching.

New TrackInterest / TrackInterestGuard: a reference count backed by a watch channel that can also await "unwatched for a while". Subscribers hold a guard while being served, so the count is maintained by RAII and a cancelled subscriber cannot leak a reference. Once an entry has been unwatched for the grace period it is evicted and the upstream Subscribe is dropped, sending UNSUBSCRIBE.

Both cache layers are covered: the local PUBLISH_NAMESPACE pull-through cache in Locals, and the cross-relay track cache in RemoteManager.

Three details this depends on:

• Guards hold a strong reference to the counter, not a cache key. An outstanding guard from an evicted entry decrements the counter it came from rather than making a same-named replacement look busy.
• Guards are created under the same lock the idle check is made under. A subscriber racing eviction is therefore either counted (eviction abandoned) or misses the cache and requests a fresh entry.
• The entry is cleared before the upstream subscription is dropped, so a subscriber arriving in that window re-subscribes rather than attaching to a reader about to go silent.

Grace period defaults to 30s, configurable via --cache-idle-timeout; 0 restores the previous behaviour.

The timeout is plumbed through a new Relay::new_with_cache_idle_timeout constructor rather than a RelayConfig field, specifically so that exhaustive struct-literal construction of RelayConfig keeps compiling for embedders. See Compatibility below for the API surface this does break.

3. perf: stream namespace paths into the formatter

Display for TrackNamespace called namespace_path() internally and built a String, so it was no cheaper than to_utf8_path() — and the %namespace.to_utf8_path() pattern at tracing call sites allocated on every call, including at log levels that were not enabled. Display now streams into the formatter; to_utf8_path() delegates to it so there is one implementation of the path format. Tracing call sites become %namespace, which is both lazy and allocation-free.

4. feat: add metrics for lock poisoning and broadcast lag

Lease registry lock poisoning means a PrefixInterest can never be retired, so the upstream namespace pull it owns leaks for the process lifetime — the visible symptom is upstream subscriptions that never go away, which is hard to trace back to one log line. Broadcast Lagged is recovered from with a full resync (correct, but silent). Both are now counted. Also registers moq_relay_cache_idle_evictions_total, which this branch emits but which had no describe_counter! entry and so shipped with no Prometheus HELP text.

Testing

cargo fmt --check, cargo clippy --no-deps, cargo test (386 passing), cargo machete — all clean.

New coverage: subgroup FIN-vs-RESET termination and reset-code mapping; interest counting, grace-period restart and generation identity; idle eviction, warm-cache reuse within the grace period, cancelled-requester safety, zero-timeout opt-out, and stale-guard isolation on both the local and remote paths.

Compatibility

This is a source-breaking change to moq-relay-ietf. Three exported methods change return type so that a downstream-interest guard travels with the reader:

Method	Before	After
Locals::register_namespace	mpsc::Receiver<TrackWriter>	mpsc::Receiver<TrackRequest>
Locals::get_or_request_track	Option<TrackReader>	Option<(TrackReader, Option<TrackInterestGuard>)>
RemoteManager::subscribe	Result<Option<TrackReader>>	Result<Option<(TrackReader, TrackInterestGuard)>>

Intentional. release-plz picks up breaking API changes at release prep and bumps semver accordingly.

Preserved deliberately: RelayConfig is unchanged, so embedders constructing it as an exhaustive struct literal are unaffected. That is why the idle timeout went through a new constructor instead of a config field, and it was verified by compiling a downstream embedder against this branch.

Guard-less variants of the three methods above were not kept as an additive migration path. The design's correctness depends on the interest guard being created under the same lock as the idle check, so a method that returns a cached reader without a guard hands back an entry that registers no interest — it looks idle immediately and can be evicted while it is still being served. That is the bug class this PR fixes, so a compile error at each call site is preferable to an API that silently reintroduces it.

Open questions (not addressed here)

Worth flagging for discussion rather than blocking this PR:

1. Should a relay start writing an object downstream before it has the whole object from upstream? Today it does — it commits to payload_length and then streams chunks. Good for latency, but it commits to a length it cannot yet guarantee.
2. What should a relay do when the remainder of an already-committed object cannot be obtained? Once the header is written there is no legal way to un-promise those bytes. Resetting the subgroup (what this does) discards objects that were fine. MALFORMED_TRACK is arguably the wrong code — the track isn't malformed, the upstream delivery failed — and INTERNAL_ERROR is no better. There may be a gap in the §13.4.4 registry for "relay could not obtain the rest of an object it had already committed to", which is a normal relay condition rather than a fault in either the track or the relay.

Notes

• draft-ietf-moq-transport-14-based relay: propagate downstream interest loss with warm-cache linger + upstream UNSUBSCRIBE #180 overlaps with part 2 and independently converged on TrackRequest { writer, lease }. It targets an obsolete base with a failing build; differences to reconcile if it is revived are that it places interest tracking in moq_transport::serve rather than the relay crate, uses a 3s linger, and does not appear to cover RemoteManager.