Skip to content

Conversation

@sgodin
Copy link
Contributor

@sgodin sgodin commented Dec 15, 2025

If we receive a stream header with an unknown track alias, then wait for up to 1 second for SubscribeOk to arrive

-remove panic if Fetch stream header

…for up to 1 second for SubscribeOk to arrive

-remove panic if Fetch stream header
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a race condition where stream headers with unknown track aliases could arrive before the corresponding SubscribeOk message. The solution introduces a notification-based waiting mechanism with a 1-second timeout, and also removes a panic when receiving Fetch stream headers by adding proper error handling.

Key Changes:

  • Added tokio::sync::Notify to signal when the subscribe alias map is updated
  • Refactored get_subscribe_id_by_alias to optionally wait up to 1 second for aliases to appear
  • Added early validation to reject non-SUBGROUP stream types with proper error instead of panicking

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
moq-transport/src/session/subscriber.rs Implements race condition fix with notification-based waiting, adds subscribe alias notification field, refactors get_subscribe_id_by_alias to support async waiting, adds non-SUBGROUP stream type validation, makes recv_datagram async, and fixes spelling error
moq-transport/src/session/mod.rs Updates recv_datagram call to handle async signature

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@englishm englishm merged commit 0a854f7 into cloudflare:main Dec 18, 2025
7 checks passed
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.

3 participants