[go: up one dir, main page]

Page MenuHomePhabricator

Bug 2080876 - Avoid deadlocking EME requests against GMP service initialization. r?#media-playback-reviewers!
Needs RevisionPublic

Authored by padenot on Mon, Oct 12, 4:00 PM.
Referenced Files
Unknown Object (File)
Tue, Oct 13, 2:36 AM
Unknown Object (File)
Tue, Oct 13, 1:26 AM
Unknown Object (File)
Tue, Oct 13, 12:33 AM
Unknown Object (File)
Tue, Oct 13, 12:19 AM
Unknown Object (File)
Tue, Oct 13, 12:19 AM
Subscribers

Details

Reviewers
alwu
Group Reviewers
media-playback-reviewers
Commits
rFIREFOXAUTOLANDb84f2e9c41d3: Bug 2080876 - Avoid deadlocking EME requests against GMP service initialization.
Bugzilla Bug ID
2080876
Merge Conflict Status
Landing may fail: this does not merge cleanly into the target branch
Conflicts when merged against target branch tip f04a3a31abe2, starting from 799727a82fb5, with 1 parent revision(s) applied first.
Last checked Tue, Oct 13, 7:36 PM
Diff 1403631, based on 799727a82fb5
Summary

A MediaCapabilities decoder probe can start creating the GMP service on a
background thread and wait for main to initialize it. If an EME request
then calls HaveGMPFor on main, it waits for that pending service creation
and prevents the initialization runnable from executing.

Fix this by moving EME's GMP support checks to the PDM thread pool, so main
remains free to initialize the service. Assert that HaveGMPFor runs off
main to catch callers that could reintroduce the deadlock.

To consume those checks asynchronously, return a promise from
GetKeySystemStatus and continue request processing on main when it
resolves. Adapt CDM installation retries to await the status too, and
ignore results for requests that have expired or been shut down. Filter
supported key systems off main before assembling their configurations on
main; DOM promise settlement also stays on main.

Diff Detail

Event Timeline

phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".
phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".
phab-bot removed a project: secure-revision.
alwu added a subscriber: alwu.

All feedbacks here can be addressed later, as we want to land this and fix the issue ASAP.

dom/media/eme/KeySystemConfig.cpp
82

nits : use SUPERVISOR?

216

Same here.

225

If I understand it correctly, Supports() call HaveGMPFor which causes the deadlock, so we wants to move Supports() to async and go back to main thread later.

As I feel going to async on the pool, and back to main thread later seems a workaround. Do you think of the fix of launching GMP process earlier and store it as a singleton? We can do it as early as the first call of requestMediaKeySystemAccess, not launching GMP in the deep of the process. Or that will still cause a deadlock?

This revision is now accepted and ready to land.Mon, Oct 12, 5:30 PM
This revision is now accepted and ready to land.Mon, Oct 12, 9:17 PM
alwu requested changes to this revision.Mon, Oct 12, 11:35 PM

See the failure and the analysis. I will put a new fix later.

This revision now requires changes to proceed.Mon, Oct 12, 11:35 PM