1851056 - Crash in [@ mozilla::gmp::GMPContentParent::ChromiumCDMDestroyed]

Status: RESOLVED FIXED

(Core :: Audio/Video: GMP, defect)

Description

[Andrew McCreight [:mccr8]]

Crash report: https://crash-stats.mozilla.org/report/index/c8029370-0ea5-4010-b700-630290230831

MOZ_CRASH Reason: MOZ_DIAGNOSTIC_ASSERT(false) (mChromiumCDMs.RemoveElement(aCDM))

Top 10 frames of crashing thread:

0  xul.dll  mozilla::gmp::GMPContentParent::ChromiumCDMDestroyed  dom/media/gmp/GMPContentParent.cpp:61
1  xul.dll  mozilla::gmp::ChromiumCDMParent::ActorDestroy  dom/media/gmp/ChromiumCDMParent.cpp:1038
2  xul.dll  mozilla::ipc::IProtocol::DestroySubtree  ipc/glue/ProtocolUtils.cpp:598
3  xul.dll  mozilla::gmp::PGMPContentParent::SendPChromiumCDMConstructor  ipc/ipdl/PGMPContentParent.cpp:352
4  xul.dll  mozilla::gmp::GMPContentParent::GetChromiumCDM  dom/media/gmp/GMPContentParent.cpp:158
5  xul.dll  mozilla::gmp::GeckoMediaPluginService::GetCDM::<lambda_1>::operator const  dom/media/gmp/GMPService.cpp:270
5  xul.dll  mozilla::MozPromise<RefPtr<mozilla::gmp::GMPContentParentCloseBlocker>, mozilla::MediaResult, 1>::InvokeMethod  xpcom/threads/MozPromise.h:654
5  xul.dll  mozilla::MozPromise<RefPtr<mozilla::gmp::GMPContentParentCloseBlocker>, mozilla::MediaResult, 1>::InvokeCallbackMethod  xpcom/threads/MozPromise.h:685
5  xul.dll  mozilla::MozPromise<RefPtr<mozilla::gmp::GMPContentParentCloseBlocker>, mozilla::MediaResult, 1>::ThenValue<`lambda at /builds/worker/checkouts/gecko/dom/media/gmp/GMPService.cpp:263:11', `lambda at /builds/worker/checkouts/gecko/dom/media/gmp/GMPService.cpp:284:11'>::DoResolveOrRejectInternal  xpcom/threads/MozPromise.h:870
6  xul.dll  mozilla::MozPromise<mozilla::net::RemoteStreamInfo, nsresult, 0>::ThenValueBase::ResolveOrRejectRunnable::Run  xpcom/threads/MozPromise.h:490

The volume seems low, but it is a diagnostic assert so I figured I'd file it.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The bug is linked to a topcrash signature, which matches the following criterion:

• Top 10 content process crashes on beta

For more information, please visit BugBot documentation.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:jimm, could you have a look please?

For more information, please visit BugBot documentation.

Comment 3

[Andrew Osmond [:aosmond] (he/him)]

I'm not sure why it spiked but they appear to be a race condition where our IPC fails to send during the SendPChromiumCDMConstructor call:
https://searchfox.org/mozilla-central/rev/48b6992e03fa66f77ac9688ba61c95d31a451bc1/dom/media/gmp/GMPContentParent.cpp#165

If the call returns nullptr we obviously don't add it to the array. But if we destroy before we return, then it is also missing:
https://searchfox.org/mozilla-central/rev/48b6992e03fa66f77ac9688ba61c95d31a451bc1/dom/media/gmp/GMPContentParent.cpp#61

Comment 4

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1851056 - Fix assert when GMPContentParent::SendChromiumCDMConstructor fails.

If SendChromiumCDMConstructor fails, then the diagnostic assert in
ChromiumCDMDestroyed will fail because we have yet to add the
ChromiumCDMParent to the array. Now in this patch we just add the parent
right away, and rely upon the ActorDestroy machinery to remove it as in
all other cases.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Based on the topcrash criteria, the crash signature linked to this bug is not a topcrash signature anymore.

For more information, please visit BugBot documentation.

Comment 6

[Dianna Smith [:diannaS] - on Leave, back on August 9th]

:aosmond do you plan on uplifting this to 119 or is this going to ride the 120 trains when it lands?

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d57c8b63407e
Fix assert when GMPContentParent::SendChromiumCDMConstructor fails. r=media-playback-reviewers,padenot

Comment 8

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/d57c8b63407e

Comment 9

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 9354558 [details]
Bug 1851056 - Fix assert when GMPContentParent::SendChromiumCDMConstructor fails.

Beta/Release Uplift Approval Request

• User impact if declined: Low volume crash
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simple, easily understood patch
• String changes made/needed:
• Is Android affected?: No

Comment 10

[Donal Meehan [:dmeehan]]

Comment on attachment 9354558 [details]
Bug 1851056 - Fix assert when GMPContentParent::SendChromiumCDMConstructor fails.

Approved for 119.0b7

Comment 11

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/52bc619257d3