Skip to content

Abort retrieveManifest requests after reset#5027

Open
robertbryer wants to merge 1 commit into
Dash-Industry-Forum:developmentfrom
bbc:cancelManifestLoad
Open

Abort retrieveManifest requests after reset#5027
robertbryer wants to merge 1 commit into
Dash-Industry-Forum:developmentfrom
bbc:cancelManifestLoad

Conversation

@robertbryer
Copy link
Copy Markdown
Contributor

Calling reset() whilst a request from retrieveManifest() is running does not abort the request.

This can cause a second retrieveManifest call to succeed with the manifest from the first call, like so:

  1. retrieveManifest(url1)
  2. reset()
  3. retrieveManifest(url2)
  4. mpd at url1 plays

Copilot AI review requested due to automatic review settings April 30, 2026 09:52
Copy link
Copy Markdown

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

Fixes an edge case where calling reset() while an app-driven retrieveManifest() is in flight can allow stale manifest results to be used after a subsequent retrieveManifest() call.

Changes:

  • Track the ManifestLoader created by retrieveManifest() so reset() can abort it.
  • Use the shared loader reference inside retrieveManifest() and reset it after completion.

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

Comment on lines 2188 to +2193
};

eventBus.on(Events.INTERNAL_MANIFEST_LOADED, handler, self);

uriFragmentModel.initialize(url);
manifestLoader.load(url);
retrieveManifestLoader.load(url);
Comment on lines 2176 to 2188
@@ -2179,13 +2184,13 @@ function MediaPlayer() {
callback(null, e.error);
}
eventBus.off(Events.INTERNAL_MANIFEST_LOADED, handler, self);
manifestLoader.reset();
retrieveManifestLoader.reset();
};
}

if (retrieveManifestLoader) {
retrieveManifestLoader.reset();
@dsilhavy dsilhavy added this to the 5.2.1 milestone May 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants