1443803 - Intermittent application crashed [@ mozilla::Atomic<bool, (mozilla::MemoryOrdering)2u, void>::operator bool] in test_peerConnection_transceivers.html

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect, P1)

Comment 1

[Byron Campen [:bwc]]

This is by far our worst intermittent, occurring somewhere between 30-50 times a day right now.

Comment 2

[Byron Campen [:bwc]]

Happened for the first time when bug 1440169 landed. Happens most often on linux64 QuantumRender debug, so retriggering there on the previous push that ran the mda tests.

https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=5889cac267e270e4762fe579e8bec0b4b0920532&selectedJob=165481680

Comment 3

[Byron Campen [:bwc]]

Pushing back search to https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=784928adcf87fadbbbcfc2a28345d94b6cffb31c

Comment 4

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b85e461e5398

Comment 5

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f3b440c155be

Comment 6

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=23d89bfddf8d

Comment 7

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=efad9c1e402f

Comment 8

[Intermittent Failures Robot]

6 failures in 747 pushes (0.008 failures/push) were associated with this bug in the last 7 days.    

Repository breakdown:
* try: 4
* mozilla-inbound: 2

Platform breakdown:
* linux64-qr: 5
* linux32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1443803&startday=2018-03-05&endday=2018-03-11&tree=all

Comment 9

[Andreas Pehrson [:pehrsons]]

I'm stealing this given that Byron is on PTO this week. Thanks for the investigation so far Byron, it's very helpful!

From the last try push we see:
> [task 2018-03-12T12:31:56.750Z] 12:31:56     INFO - GECKO(2769) | [2827:Unnamed thread 0x7f8b4d7df280]: E/MediaStreamGraph MediaStream 0x7f8b3d2b5d20 unset graph
> [task 2018-03-12T12:31:56.758Z] 12:31:56     INFO - GECKO(2769) | [2827:Unnamed thread 0x7f8b44f05280]: E/MediaManager InsertInGraph: GraphImpl in stream 0x7f8b3d2b5d20 is null!

The first line is from MediaStream::DestroyImpl() (1) which wins in a race with MediaEngineWebRTCMicrophoneSource::InsertInGraph() (2) which needs access to MediaStream::GraphImpl().

The race occurs after a DOMMediaStream is destroyed on main thread. There are two paths we go through to clean up on the MediaStreamGraph thread, similar to the race between (1) and (2) above.
1: Main thread [1.1] -> MSG thread [1.2]
2: Main thread [2.1] -> MediaManager thread [2.2] (which sets state under a mutex, that's accessed on the MSG thread under the same mutex)

Usually the media manager thread would run its runnable first as MSG always waits for stable state before dispatching. That's not a guarantee however, and if the media manager thread is busy enough, the MSG might win, like in (1).


[1.1] https://searchfox.org/mozilla-central/rev/a70da6775d5341a9cca86bf1572a5e3534909153/dom/media/DOMMediaStream.cpp#493
[1.2] https://searchfox.org/mozilla-central/rev/a70da6775d5341a9cca86bf1572a5e3534909153/dom/media/MediaStreamGraph.cpp#2079

[2.1] https://searchfox.org/mozilla-central/rev/a70da6775d5341a9cca86bf1572a5e3534909153/dom/media/MediaStreamTrack.cpp#160
[2.2] https://searchfox.org/mozilla-central/rev/a70da6775d5341a9cca86bf1572a5e3534909153/dom/media/MediaManager.cpp#3984

Comment 10

[Andreas Pehrson [:pehrsons]]

I've tried keeping the MediaStream from being destroyed in the MSG until MediaManager has stopped the sources properly, but that had other implications: https://treeherder.mozilla.org/#/jobs?repo=try&revision=803220aa3002495086083056c6e51b8114ae1a69

I'll see if I can get a simpler patch working instead, though it won't be providing the same guarantees as this one.

Comment 11

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1443803 - Protect against destroyed MediaStream in MediaEngineWebRTCMicrophoneSource.

Review commit: https://reviewboard.mozilla.org/r/227748/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/227748/

Comment 12

[Paul Adenot (:padenot)]

Comment on attachment 8958880 [details]
Bug 1443803 - Protect against destroyed MediaStream in MediaEngineWebRTCMicrophoneSource.

https://reviewboard.mozilla.org/r/227748/#review233848

Sad.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/908d9d7f5c3e
Protect against destroyed MediaStream in MediaEngineWebRTCMicrophoneSource. r=padenot

Comment 14

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/908d9d7f5c3e

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 16

[Andreas Pehrson [:pehrsons]]

Comment on attachment 8958880 [details]
Bug 1443803 - Protect against destroyed MediaStream in MediaEngineWebRTCMicrophoneSource.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1426718
[User impact if declined]: Crashes in debug builds on try and for users (if any, but jesup was at least hit)
[Is this code covered by automated tests?]: Yes, see bug 1432025 for an example
[Has the fix been verified in Nightly?]: No, the test in question in bug 1432025 was backed out
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: All it does is add early exit guards that will get triggered only in shutdown
[String changes made/needed]: None

Comment 17

[Andreas Pehrson [:pehrsons]]

(In reply to Andreas Pehrson [:pehrsons] from comment #16)
> [Has the fix been verified in Nightly?]: No, the test in question in bug
> 1432025 was backed out
s/backed out/disabled/

Comment 18

[Julien Cristau [:jcristau]]

Comment on attachment 8958880 [details]
Bug 1443803 - Protect against destroyed MediaStream in MediaEngineWebRTCMicrophoneSource.

webrtc crash fix in debug builds, beta60+

Comment 19

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/releases/mozilla-beta/rev/4626b30f8c29