778745 - crash occurs when quitting firefox with a video/audio stream running from MediaStream [@ PR_DestroyCondVar]

Status: RESOLVED WORKSFORME

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

Description

[Jason Smith [:jsmith]]

Attachment: Basic Video Test

This bug was filed from the Socorro interface and is 
report bp-cd70bd61-ff6d-42a1-9897-7e6032120730 .
============================================================= 

Steps:

1. Load the attached test case
2. Quit Firefox

Expected:

The camera should stop being in use. Firefox process should stop running. No crash should occur.

Actual:

Firefox crashes on shutdown.

Comment 1

[Jason Smith [:jsmith]]

This is a reproducible crash. Don't see why the keyword is removed here.

Comment 2

[Scoobidiver (away)]

(In reply to Jason Smith [:jsmith] from comment #1)
> This is a reproducible crash. Don't see why the keyword is removed here.
reproducible is when there's no testcase but manual STR.

Comment 3

[Jason Smith [:jsmith]]

(In reply to Scoobidiver from comment #2)
> (In reply to Jason Smith [:jsmith] from comment #1)
> > This is a reproducible crash. Don't see why the keyword is removed here.
> reproducible is when there's no testcase but manual STR.

I thought reproducible was used when you can reliably reproduce a crash? Technically, you can reproduce this crash with having any use of getUserMedia with a video and quitting firefox. The test case attached just provides an example.

Comment 4

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

We will not be able to test this with any framework we have in the tree, but we can make use of Mozmill to get such a test.

Comment 5

[Jason Smith [:jsmith]]

Something apparently fixed this bug, cause I can't reproduce this anymore. Don't know what did though...

Comment 6

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

Clearing testsuite request and closing as WFM, which should be used if no check-in is known.

Comment 7

[Jason Smith [:jsmith]]

(In reply to Henrik Skupin (:whimboo) from comment #6)
> Clearing testsuite request and closing as WFM, which should be used if no
> check-in is known.

Ehh....I still might suggest getting a mozmill test case for this anyways if it's possible. This was consistently reproducible before.

Comment 8

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

If that's the case, sure that we can have it to proof we don't crash.

Comment 9

[Jason Smith [:jsmith]]

Looks like I was wrong. I just reproduced this crash intermittently during shutdown while a gum video was running:

https://crash-stats.mozilla.com/report/index/bp-07050638-7bf9-4661-8f32-c22db2121231

Comment 10

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

roc - I looked at this, and this looks strongly like we're somehow calling delete on a NULL mGraph/MediaStreamGraphImpl from the MediaStreamGraphShutdownRunnable.  It's crashing trying to delete the Monitor, which has a CondVar inside of it.  How this happens.... 

Does this sound like a reasonable analysis from this stack trace? I haven't been able to reproduce this myself, though perhaps I haven't tried enough.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Presumably mGraph is not just null, otherwise "delete mGraph" would do nothing.

Would be interesting to have the log from a debug build, to see if any assertions fire leading up to the crash.

Comment 12

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

If anyone sees anything like this after some other cleanup patches land today (bug 827007 and bug 826576), please flag it.  No proof they're involved, but it's conceivable.

Comment 13

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

per comment 12, I think this is now fixed.  Marking qawanted

Comment 14

[Jason Smith [:jsmith]]

Can't reproduce on 1/29 build.

Comment 15

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

Nils, can you please check which kind of test would be good here, or even still necessary? Thanks.

Comment 16

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

I missed the 'shutdown' part here. So in that case Mozmill would be required. Nils, do you think it is worth creating a test?

Comment 17

[Nils Ohlmeier [:drno]]

Please see https://bugzilla.mozilla.org/show_bug.cgi?id=799142#c54 for my request of writing one Mozmill test to cover these scenarios.