Skip to content

Conversation

@kixelated
Copy link
Collaborator

Summary

  • The spawned unused() cleanup task removes the producer entry by track name, which could evict a newer producer inserted at the same key after a stale one was replaced (race from Fix stale TrackProducer returned from cache #945)
  • Guard the removal with an is_clone() identity check so only the original producer instance is evicted; if a different producer now occupies the slot, reinsert it

Test plan

  • cargo test -p moq-lite — all 142 tests pass
  • Existing stale_producer and requested_unused tests cover the relevant paths

🤖 Generated with Claude Code

The spawned unused() cleanup task removes by track name, which could
evict a newer producer inserted at the same key after a stale one was
replaced. Guard the removal with an identity check so only the
original producer instance is evicted.

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

coderabbitai bot commented Feb 12, 2026

Walkthrough

The change updates BroadcastConsumer::subscribe_track in the broadcast module to alter cleanup when a tracked producer becomes unused. The code now acquires the shared state lock and, instead of unconditionally removing the entry, conditionally reinserts a current producer if it exists and is not the same clone being dropped. No public APIs were changed.

🚥 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 clearly and concisely describes the main fix: addressing a race condition in the cleanup task of the subscribe_track function.
Description check ✅ Passed The description comprehensively explains the race condition, the solution using is_clone() check, and confirms testing with all 142 tests passing.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/cleanup-task-race

No actionable comments were generated in the recent review. 🎉


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) February 12, 2026 16:16
@kixelated kixelated merged commit 21beb8b into main Feb 12, 2026
1 check passed
@kixelated kixelated deleted the fix/cleanup-task-race branch February 12, 2026 16:26
This was referenced Feb 12, 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