Skip to content

Fix IETF subscriber race cancelling groups before consumers attach#1012

Merged
kixelated merged 1 commit intomainfrom
fix/ietf-subscriber-group-unused
Mar 1, 2026
Merged

Fix IETF subscriber race cancelling groups before consumers attach#1012
kixelated merged 1 commit intomainfrom
fix/ietf-subscriber-group-unused

Conversation

@kixelated
Copy link
Collaborator

Summary

  • The Producer/Consumer refactor (a0f55d06) made unused() fire immediately when consumers == 0, which is the initial state for GroupProducer and FrameProducer
  • The IETF subscriber's tokio::select! with unused() was a race condition: groups were randomly cancelled before any downstream publisher could call next_group() to create a consumer
  • This broke cross-version relay forwarding (e.g. IETF Draft14 publisher → relay → moq-lite-03 subscriber), with catalog.json groups being silently dropped
  • Removed the group.unused() and frame.unused() checks from the IETF subscriber, matching the lite subscriber which already skips them ("written to a cache")

Test plan

  • Run just dev with publisher using moq-transport-14 and subscriber using moq-lite-03 — catalog.json should reliably reach the subscriber
  • Run just check to verify no regressions

🤖 Generated with Claude Code

The Producer/Consumer refactor (a0f55d0) made `unused()` return
immediately when consumers == 0 (the initial state). Since
GroupProducer/FrameProducer start with 0 consumers—consumers are
only created when a downstream publisher calls next_group()—the
IETF subscriber's select! with unused() was a race condition that
randomly cancelled groups before any data was read.

This matches the lite subscriber which already skips these checks
with the comment "written to a cache".

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@kixelated kixelated enabled auto-merge (squash) March 1, 2026 03:21
@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 28fc538 and bb98c20.

📒 Files selected for processing (1)
  • rs/moq-lite/src/ietf/subscriber.rs

Walkthrough

This change refactors the control flow in the subscriber module by replacing tokio::select! blocks with direct await calls. Previously, the code used select! to concurrently monitor producer.unused() and frame.unused() futures as cancellation triggers. The refactor removes this intermediate branching, instead directly awaiting run_group() and run_frame() calls. The linear await-based approach maintains identical error handling semantics where errors close frames or groups and successful completions finish them. No public API changes were made.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 directly addresses the main bug fix: removing race condition logic that was cancelling groups before consumers could attach.
Description check ✅ Passed The description clearly explains the root cause (Producer/Consumer refactor), the bug (race condition with tokio::select!), impact (cross-version relay failures), and the solution (removing unused() checks).

✏️ 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 fix/ietf-subscriber-group-unused

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 merged commit 47743fd into main Mar 1, 2026
1 check passed
@kixelated kixelated deleted the fix/ietf-subscriber-group-unused branch March 1, 2026 03:39
@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