Skip to content

Cascade close and select on group.closed() in subscribers#1013

Merged
kixelated merged 2 commits intomainfrom
verify-unsub
Mar 1, 2026
Merged

Cascade close and select on group.closed() in subscribers#1013
kixelated merged 2 commits intomainfrom
verify-unsub

Conversation

@kixelated
Copy link
Collaborator

Summary

  • Add Producer::closed() / GroupProducer::closed() to await explicit state closure
  • Add TrackWeak::close() that cascades close to all child groups
  • Update BroadcastProducer::close() to cascade to child tracks and pending requests
  • Add select! on group.closed() in both lite and IETF subscriber recv_group, so we stop reading from the QUIC stream when the group is closed (e.g. due to subscription cancellation)

Test plan

  • All 149 moq-lite tests pass (including updated broadcast cascade test)
  • just check passes
  • Manual testing with live streams to verify subscriptions are cleaned up promptly

🤖 Generated with Claude Code

kixelated and others added 2 commits February 28, 2026 20:18
Ensure that closing a broadcast cascades to child tracks, and that
subscriber recv_group tasks terminate promptly when a group is closed
(e.g. due to subscription cancellation) by selecting on group.closed().

- Add Producer::closed() to await explicit state closure
- Add GroupProducer::closed() forwarding to state
- Add TrackWeak::close() that cascades to child groups
- Update BroadcastProducer::close() to cascade to tracks and requests
- Add select on group.closed() in lite subscriber recv_group
- Replace unused() with closed() in IETF subscriber recv_group

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 1, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 20c0cfe and b24449f.

📒 Files selected for processing (6)
  • rs/moq-lite/src/ietf/subscriber.rs
  • rs/moq-lite/src/lite/subscriber.rs
  • rs/moq-lite/src/model/broadcast.rs
  • rs/moq-lite/src/model/group.rs
  • rs/moq-lite/src/model/state.rs
  • rs/moq-lite/src/model/track.rs

Walkthrough

This pull request introduces concurrency improvements and error cascading through the broadcast hierarchy. The subscriber modules now use tokio::select! to race between producer closure and group task completion instead of awaiting directly. The run_group method signature changes to accept an owned GroupProducer. New closed() async methods are added to GroupProducer and Producer<T> with backing implementation in the state layer. BroadcastProducer::close now cascades errors to all child tracks and pending dynamic track requests. A new close(err) method is added to TrackWeak to propagate errors upward through associated groups.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 13.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Cascade close and select on group.closed() in subscribers' directly describes the main changes: cascading close operations and adding select! on group.closed() in subscriber code.
Description check ✅ Passed The description is well-aligned with the changeset, detailing the new Producer/GroupProducer closed() methods, TrackWeak::close() cascade behavior, and subscriber select! changes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch verify-unsub

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@kixelated kixelated enabled auto-merge (squash) March 1, 2026 04:36
@kixelated kixelated merged commit d11fe81 into main Mar 1, 2026
1 check passed
@kixelated kixelated deleted the verify-unsub branch March 1, 2026 04:37
@moq-bot moq-bot bot mentioned this pull request Mar 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant