1763630 - Crash in [@ mozilla::AudioProcessingTrack::DisconnectDeviceInput]

Status: RESOLVED FIXED

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

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

~180 crashes for Firefox 98.0.x, 11 for 99.0 so far

Crash report: https://crash-stats.mozilla.org/report/index/f06b43d9-2b74-4a08-a05e-4f3770220407

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames of crashing thread:

0 xul.dll mozilla::AudioProcessingTrack::DisconnectDeviceInput dom/media/webrtc/MediaEngineWebRTCAudio.cpp:1358
1 xul.dll mozilla::AudioProcessingTrack::Destroy dom/media/webrtc/MediaEngineWebRTCAudio.cpp:1172
2 xul.dll mozilla::DeviceListener::Stop dom/media/MediaManager.cpp:3967
3 xul.dll mozilla::LocalTrackSource::Stop dom/media/MediaManager.cpp:764
4 xul.dll mozilla::dom::MediaStreamTrackSource::UnregisterSink dom/media/MediaStreamTrack.h:232
5 xul.dll mozilla::dom::MediaStreamTrack::SetReadyState dom/media/MediaStreamTrack.cpp:546
6 xul.dll mozilla::dom::AudioStreamTrack::SetReadyState dom/media/AudioStreamTrack.cpp:69
7 xul.dll mozilla::dom::MediaStreamTrack::Stop dom/media/MediaStreamTrack.cpp:309
8 xul.dll mozilla::dom::MediaStreamTrack_Binding::stop dom/bindings/MediaStreamTrackBinding.cpp:3598
9 xul.dll mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions> dom/bindings/BindingUtils.cpp:3307

Comment 1

[C.M.Chang[:chunmin]]

Ah, MediaInputPort::GetSource() is graph-thread only but AudioProcessingTrack::DisconnectDeviceInput() calls it on main thread. I'll fix this next week.

Comment 2

[C.M.Chang[:chunmin]]

Attachment: Bug 1763630 - Connect device input via a main-thread only NativeInputTrack

We should keep a main-thread only NativeInputTrack member variable in
AudioProcessingTrack and use it to connect and disconnect the device
input which open and close audio microphone in MTG.

Comment 3

[C.M.Chang[:chunmin]]

When investigating this issue, I find we should make MediaInputPort::Get{Source, Destination} grapoh thread only, so I filed bug 1764186.

Comment 4

[C.M.Chang[:chunmin]]

(In reply to C.M.Chang[:chunmin] from comment #1)
Another possible cause is that we allocate an empty MediaInputPort since the port is allocated after the AudioProcessingTrack has been destroyed. In that case, we will still allocate a NativeInputTrack but we didn't deallocate it due to the crash. The code in D143412 should be able to resolve this as well, if it's the cause.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/094c8649451f
Connect device input via a main-thread only NativeInputTrack r=padenot

Comment 6

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/094c8649451f

Comment 7

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

The patch landed in nightly and beta is affected.
:chunmin, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 8

[C.M.Chang[:chunmin]]

Comment on attachment 9271799 [details]
Bug 1763630 - Connect device input via a main-thread only NativeInputTrack

Beta/Release Uplift Approval Request

• User impact if declined: Firefox may cause crashes
• Is this code covered by automated tests?: No
• 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): The patch simply saves a value and uses the value when it needs.
• String changes made/needed:

Comment 9

[Dianna Smith [:diannaS]]

Comment on attachment 9271799 [details]
Bug 1763630 - Connect device input via a main-thread only NativeInputTrack

Approved for 100.0b8

Comment 10

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3b3a264692de