Failure to exit when quitting with an active getUserMedia camera stream

VERIFIED FIXED in Firefox 20

Status

()

Core
WebRTC: Audio/Video
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: jesup, Assigned: jesup)

Tracking

({hang})

unspecified
mozilla21
All
Mac OS X
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox19 unaffected, firefox20+ fixed, firefox21+ fixed)

Details

(Whiteboard: [getUserMedia], [blocking-gum+])

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

5 years ago
Appears to be Mac-only (need to test on PC).  Latest inbound; leaving a local getUseMedia stream active and attached to a video element causes Mac Quit to hang.  (Not sure if being attached has anything to do with it; probably not).

Updated

5 years ago
Duplicate of this bug: 837788
We enabled GUM in Firefox 20 by default. So shouldn't we track this for the release given that any website could trigger that?
tracking-firefox20: --- → ?
tracking-firefox21: --- → ?
Hardware: x86 → All
(In reply to Henrik Skupin (:whimboo) [away 02/09 - 02/017] from comment #2)
> We enabled GUM in Firefox 20 by default. So shouldn't we track this for the
> release given that any website could trigger that?

Yeah, we should. We're planning on shipping on FF 20. So we probably need to track on that one.
This hang doesn't seem to happen in a debug build. Quitting Firefox with an active gUM stream works fine.
Created attachment 711976 [details]
activity monitor sample

Updated

5 years ago
status-firefox19: --- → unaffected
status-firefox20: --- → affected
status-firefox21: --- → affected
tracking-firefox20: ? → +
tracking-firefox21: ? → +
(Assignee)

Comment 6

5 years ago
Created attachment 713803 [details] [diff] [review]
release video capture device on MainThread (mac only)
(Assignee)

Comment 7

5 years ago
Created attachment 713813 [details] [diff] [review]
release video capture device on MainThread (mac only)
(Assignee)

Updated

5 years ago
Attachment #713803 - Attachment is obsolete: true
(Assignee)

Comment 8

5 years ago
We were releasing the GUM streams in GC (or thereabouts) in shutdown, before the xpcom-shutdown notification, from the MediaManager thread, and hanging somewhere inside of [_captureDevice release].  (ObjC)  This meant later we couldn't shut down the thread, and hung.

So the source of the problem turns out to be that QTKit uses sends to the main CFRunLoop thread to implement thread-safety.  The reason this stops working is that when we're exiting (mExiting is true in the MainThread nsAppShell), we stop processing native events:

http://mxr.mozilla.org/mozilla-central/source/widget/xpwidgets/nsBaseAppShell.cpp#294

We can't easily and safely hook in at an earlier point since we can't guarantee we process and finish it before shutdown proceeds too far.

The solution is to remove the need to do so by proxying the delete to the MainThread such that the native event send isn't needed.  (Thanks to bsmedberg for helps diagnose this)
(Assignee)

Updated

5 years ago
Attachment #713813 - Flags: review?(benjamin)
(Assignee)

Comment 9

5 years ago
We'll want to uplift this to Aurora ASAP.

Also note we trigger the Mac WritePoisoning code due to the webrtc trace impl flushing at exit if you set NSPR_LOG_MODULES to webrtc_trace:xxxxx.  This isn't new, but I hit it while testing this.  I'll file a separate bug on that.
Whiteboard: [getUserMedia], [blocking-gum+] → [getUserMedia], [blocking-gum+][webrtc-uplift]

Comment 10

5 years ago
Comment on attachment 713813 [details] [diff] [review]
release video capture device on MainThread (mac only)

Please use

nsCOMPtr<nsIThread> mainThread = do_GetMainThread();

or the even simpler NS_DispatchToMainThread(WrapRunnable(...), NS_DISPATCH_SYNC)
(Assignee)

Comment 11

5 years ago
Created attachment 713990 [details] [diff] [review]
release video capture device on MainThread (mac only)
(Assignee)

Updated

5 years ago
Attachment #713813 - Attachment is obsolete: true
Attachment #713813 - Flags: review?(benjamin)
(Assignee)

Comment 12

5 years ago
Comment on attachment 713990 [details] [diff] [review]
release video capture device on MainThread (mac only)

revised per comment, added comments about NS_DISPATCH_SYNC.  Will retest on mac but no surprises expected
Attachment #713990 - Flags: review?(benjamin)

Updated

5 years ago
Attachment #713990 - Flags: review?(benjamin) → review+
(Assignee)

Comment 13

5 years ago
https://hg.mozilla.org/mozilla-central/rev/ebeaa97d710d
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-firefox21: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
(Assignee)

Comment 14

5 years ago
Some references:
http://stackoverflow.com/questions/5703754/addinput-method-of-qtcapturesession-not-returning
http://developer.apple.com/library/mac/#technotes/tn2125/_index.html
http://code.google.com/p/chromium/issues/detail?id=157725
http://lists.apple.com/archives/quicktime-api/2009/Apr/msg00101.html

Updated

5 years ago
Keywords: verifyme
A different bug is tracking the work already for implementing automation here.
Flags: in-testsuite-
(Assignee)

Comment 16

5 years ago
Comment on attachment 713990 [details] [diff] [review]
release video capture device on MainThread (mac only)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A (ever since we generally fixed shutdown to properly shut down)

User impact if declined: Failure to exit if you have a video camera active

Testing completed (on m-c, etc.): on m-c since 2/14.  Tested repeatedly by multiple developers.

Risk to taking this patch (and alternatives if risky): Low risk.  I verified the underlying webrtc.org code for invoking the release is threadsafe (uses critical sections, etc), so releasing it from mainthread is safe, and I verified we never DISPATCH_SYNC from mainthread to the mediamanager thread.

String or UUID changes made by this patch: none
Attachment #713990 - Flags: approval-mozilla-aurora?

Updated

5 years ago
Attachment #713990 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 17

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/a581fbd356de
status-firefox20: affected → fixed
Not seeing a hang on shutdown on FF nightly on 2/22 build. Verified.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Whiteboard: [getUserMedia], [blocking-gum+][webrtc-uplift] → [getUserMedia], [blocking-gum+]
You need to log in before you can comment on or make changes to this bug.