808546 - WebRTC crash [@nsDOMMediaStream::GetStream]

Status: RESOLVED FIXED

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

Description

[Christoph Diehl [:posidron]]

Attachment: callstack

Tested with m-c changeset: 112272:ab099c9e1a09

Comment 1

[Christoph Diehl [:posidron]]

Attachment: NSPR_LOG

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

From the log:

565325824[1311df480]: VcmSipccBinding vcmRxStartICE(7f1a88324fc9ac82)
565325824[1311df480]: VcmSipccBinding Making new transport flow for level=1 rtcp=false
565325824[1311df480]: VcmSipccBinding Making new transport flow for level=1 rtcp=true
565325824[1311df480]: WebrtcAudioSessionConduit Create 
565325824[1311df480]: WebrtcAudioSessionConduit Init 
1987719552[1037e9e80]: PeerConnectionImpl ShutdownMedia Disconnecting media streams from PC
556646400[13121f480]: cpr SIPCC-CC_API: 8/15, cc_int_onhook: UI -> GSM: ONHOOK              
1987719552[1037e9e80]: PeerConnectionImpl ShutdownMedia Disconnecting transport
1987719552[1037e9e80]: PeerConnectionImpl ShutdownMedia Media shut down
1987719552[1037e9e80]: PeerConnectionImpl ShutdownMedia Disconnecting media streams from PC
556646400[13121f480]: cpr SIPCC-CC_API: 10/16, cc_int_onhook: UI -> GSM: ONHOOK              
1987719552[1037e9e80]: PeerConnectionImpl ShutdownMedia Disconnecting transport
1987719552[1037e9e80]: PeerConnectionImpl ShutdownMedia Media shut down
565325824[1311df480]: WebrtcAudioSessionConduit Init Channel Created 0 
565325824[1311df480]: WebrtcAudioSessionConduit Init AudioSessionConduit Initialization Done
565325824[1311df480]: WebrtcAudioSessionConduit Create Successfully created AudioConduit 

I'd guess this is due to a race between startup of a session and an almost immediate shutdown.  Without local vars I can't be sure, but I'd bet the stream_ dis null or bad.

Did asan indicate what sort of failure it was?

Comment 3

[Jason Smith [:jsmith]]

Can you give steps to reproduce?

Comment 4

[Christoph Diehl [:posidron]]

Oh, sorry that line got cut of. It's a null pointer dereference. It's reproducible but sadly not always at the same place therefore I couldn't provide a testcase.

Comment 5

[Jason Smith [:jsmith]]

Not a candidate for a crashtest.

Comment 6

[Jason Smith [:jsmith]]

Verify this exists when peer connection shutdown lands.

Comment 7

[Christoph Diehl [:posidron]]

Attachment: testcase

To reproduce reload the testcase a few times. The testcase will lead to crashes at non determinable locations.

Tested with m-c changeset:   113558:9a6d708faf3f

Comment 8

[Jason Smith [:jsmith]]

And...now it's a candidate for a crashtest with the testcase given.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This testcase doesn't reproduce the crash for me with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:20.0) Gecko/20.0 Firefox/20.0 ID:20121122030751 and newer.

Comment 10

[Christoph Diehl [:posidron]]

Hm, still works here, in fact it can crash at the same place as https://bugzilla.mozilla.org/show_bug.cgi?id=810626 therefore marking s-s.

Tested with m-c changeset: 114110:541ccce39563

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

What type of build do you have? Can you also see it with an official nightly build? Is it opt only?

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Henrik Skupin (:whimboo) from comment #11)
> build? Is it opt only?

I meant debug not-optimized, sorry.

Comment 13

[Christoph Diehl [:posidron]]

It is an ASan enabled debug, -O1 build. It's used through out all tests.

Comment 14

[Randell Jesup [:jesup] (needinfo me)]

This will crash with a debug non-ASAN build.

I strongly think this is another "Big Lock" bug, where something is deleting the stream while we're still initing the conduit.  My GDB backtrace shows that stream (GetLocalStream(0)) is non-null, but stream->GetMediaStream() is NULL (and what causes the crash).

Comment 15

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Testcase that loops

Comment 16

[Randell Jesup [:jesup] (needinfo me)]

I added locally MOZ_ASSERT()s to vcmTxStartICE for stream and stream->GetMediaStream(), and a short run of the new testcase will kick out in the GetMediaStream() assert.

Comment 17

[David Bolter [:davidb] (NeedInfo me for attention)]

Assigning to Randell on a guess that he'll be okay with that.

Comment 18

[Randell Jesup [:jesup] (needinfo me)]

Attachment: callstack with patch from bug 792175

Comment 19

[Randell Jesup [:jesup] (needinfo me)]

Tested with current inbound (with the fix for bug 792175 (and others)) for about 5 hours; no problems.  I believe this is fixed as expected from comment 14.

Comment 20

[Lukas Blakk [:lsblakk] use ?needinfo]

This landed on 20 before it merged to Aurora on 1/7/2012 so marking status as fixed.