2046555 - Crash when joining Meet

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Jim Mathies [:jimm]]

https://crash-stats.mozilla.org/report/index/61cd303f-5465-41e0-8f86-e0b990260610
MozPromise::ThenValue created from 'SetSinkId' destroyed without being either disconnected, resolved, or rejected (dispatchRv: not dispatched)

Comment 1

[Karl Tomlinson (:karlt)]

The MDSM task queue is no longer available after audio stream initialization on the background task queue.
This is a diagnostic assert that could fail after unmuting a media element or after changing the audio output device.
In release builds, I expect this would be observable in that a setSinkId() Promise does not resolve.

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: Bug 2046555 Await async init tasks in AudioSinkWrapper::Shutdown() r?padenot

This ensures that any pending tasks have the MediaDecoderStateMachine
TaskQueue available to settle dependent promises.

The async init task Then() callback will determine that any audio sink is not
needed.

This is simpler than keeping track of all unsettled Promises and disconnecting.
Stream init is already often synchronous, where wanted.

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: Bug 2046555 Abort during stream init for setSinkId() crashtest r?padenot

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/af1fa256513c
https://hg.mozilla.org/integration/autoland/rev/044b60b678cd
Await async init tasks in AudioSinkWrapper::Shutdown() r=padenot

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/bd7f7d223a9d
https://hg.mozilla.org/integration/autoland/rev/b0ee34b21aec
Abort during stream init for setSinkId() crashtest r=padenot

Comment 6

[amarc]

https://hg.mozilla.org/mozilla-central/rev/044b60b678cd

Comment 7

[Karl Tomlinson (:karlt)]

Attachment: Bug 2046555 Await async init tasks in AudioSinkWrapper::Shutdown()

This ensures that any pending tasks have the MediaDecoderStateMachine
TaskQueue available to settle dependent promises.

The async init task Then() callback will determine that any audio sink is not
needed.

This is simpler than keeping track of all unsettled Promises and disconnecting.
Stream init is already often synchronous, where wanted.

Original Revision: https://phabricator.services.mozilla.com/D306387

Comment 8

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined/Reason for urgency: This will at least address some crashes where MOZ_DIAGNOSTIC_ASSERT_ENABLED is set on early beta.

For later betas/release, this will affect whether some content Promises are resolved. We know that the path is being hit on Google Meet, but we don't know whether or not the effects on the web app interfere with usability.

• Code covered by automated testing?: yes
• Fix verified in Nightly?: no
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: Simple patch.
• String changes made/needed?: None
• Is Android affected?: no

Comment 9

[Karl Tomlinson (:karlt)]

Attachment: Bug 2046555 Abort during stream init for setSinkId() crashtest

Original Revision: https://phabricator.services.mozilla.com/D306388

Comment 10

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/fbed6589710f
https://hg.mozilla.org/releases/mozilla-beta/rev/a5b533012f5c
https://github.com/mozilla-firefox/firefox/commit/e680a80bc668
https://hg.mozilla.org/releases/mozilla-beta/rev/91e9e629badb

Comment 11

[Cosmin Sabou [:CosminS]]

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