Closed
Bug 798829
Opened 12 years ago
Closed 12 years ago
"ASSERTION: Using observer service off the main thread!" with getUserMedia
Categories
(Core :: WebRTC, defect)
Core
WebRTC
Tracking
()
RESOLVED
FIXED
mozilla19
Tracking | Status | |
---|---|---|
firefox17 | --- | unaffected |
firefox18 | + | fixed |
firefox19 | + | fixed |
People
(Reporter: jruderman, Assigned: jesup)
References
Details
(Keywords: assertion, testcase, Whiteboard: [getUserMedia], [blocking-gum+], [qa-])
Attachments
(2 files, 1 obsolete file)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review |
1. Set
user_pref("media.navigator.enabled", true);
2. Load the testcase.
3. Quit Firefox.
Result:
###!!! ASSERTION: Using observer service off the main thread!: 'Error', file xpcom/ds/nsObserverService.cpp, line 95
mozilla::MediaManager::Get [nsCOMPtr.h:490]
mozilla::GetUserMediaRunnable::Run [MediaManager.cpp:339]
nsThread::ProcessNextEvent [nsThread.cpp:612]
NS_ProcessNextEvent_P [nsThreadUtils.cpp:220]
nsThread::ThreadFunc [nsThread.h:58]
_pt_root [ptthread.c:159]
libsystem_c.dylib + 0x4e8bf
(With a smaller loop bound, you might get memory leaks instead?)
Assignee | ||
Updated•12 years ago
|
Whiteboard: [getUserMedia], [blocking-gum+]
Comment 1•12 years ago
|
||
Need to get this fixed or make sure getUserMedia can't be enabled in FF18.
tracking-firefox18:
--- → ?
Updated•12 years ago
|
tracking-firefox19:
--- → ?
Comment 2•12 years ago
|
||
Randell, Tracking this for 18 as per comment 1 . Can you please help find an assignee for this ?
status-firefox17:
--- → unaffected
status-firefox18:
--- → affected
status-firefox19:
--- → affected
Comment 3•12 years ago
|
||
We aren't planning on enabling getUserMedia for FF 18 but this needs to be fixed nevertheless.
Assignee: nobody → anant
Reporter | ||
Comment 4•12 years ago
|
||
>> Need to get this fixed or make sure getUserMedia can't be enabled in FF18.
> We aren't planning on enabling getUserMedia for FF 18
Or did Smaug mean something stronger, like "ensure users can't even flip a pref to enable it"?
Comment 5•12 years ago
|
||
I did mean that. We should not advertize that one can enable media stuff on aurora if there is a
major bug like this. So better to either fix this or make sure the media stuff can't be used at all.
Comment 6•12 years ago
|
||
Also, this looks like something reasonable easy to fix.
Reporter | ||
Comment 7•12 years ago
|
||
I tried to repro this after writing bug 802982 comment 8, and got
Assertion failure: ((bool)(__builtin_expect(!!(!NS_FAILED_impl(rv)), 1))), at ../../../../toolkit/components/downloads/nsDownloadManager.cpp:299
I guess everything starts breaking when the process hits ~2048 threads (bug 798323).
Comment 8•12 years ago
|
||
http://mxr.mozilla.org/mozilla-central/source/dom/media/MediaManager.h#196 - MediaManager::Get is called in a runnable off the main thread, and it performs observer service manipulation.
Assignee | ||
Comment 9•12 years ago
|
||
This singleton needs to be inited earlier on (and shut down as well, perhaps from nsLayoutStatics.cpp) so we don't have to create the object in ::Get(), OR ::Get() needs to send a runnable to MainThread and get a result before returning (when creating MediaManager).
Also, the singleton will inherently leak at shutdown along with its thread as currently implemented; if it returns an already_addrefed pointer (such as in NS_GENERIC_FACTORY_SINGLETON_CONSTRUCTOR()) we can track references and we have a chance to cleaning it up (knowing there are no refs) on shutdown.
It doesn't need to be an XPCOM service obtainable by GetService, though I suppose it could be.
I'll throw together a patch that makes this safe for now until this work can get done. It still will leak MediaManager (and thread) at shutdown.
Assignee | ||
Comment 10•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #673768 -
Flags: review?(anant)
Assignee | ||
Updated•12 years ago
|
Assignee: anant → rjesup
OS: Mac OS X → All
Hardware: x86_64 → All
Comment 11•12 years ago
|
||
Comment on attachment 673768 [details] [diff] [review]
Force MediaManager to be created from MainThread
We could move this to Navigator.cpp:MozGetUserMedia as well (which will always be called on the main thread), but this is good for now as well.
Attachment #673768 -
Flags: review?(anant) → review+
Assignee | ||
Comment 12•12 years ago
|
||
Target Milestone: --- → mozilla19
Comment 13•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Updated•12 years ago
|
Keywords: verifyme
Whiteboard: [getUserMedia], [blocking-gum+] → [getUserMedia], [blocking-gum+], [qa verification failed]
Assignee | ||
Comment 14•12 years ago
|
||
If verification failed, does the assertion occur? I've tried this ~30 times, and do not see the assertion.
Comment 15•12 years ago
|
||
(In reply to Randell Jesup [:jesup] from comment #15)
> If verification failed, does the assertion occur? I've tried this ~30
> times, and do not see the assertion.
Ah, I think an issue unrelated to this bug. So verification actually didn't fail in that case.
Whiteboard: [getUserMedia], [blocking-gum+], [qa verification failed] → [getUserMedia], [blocking-gum+], [qa-]
Comment 16•12 years ago
|
||
(In reply to Randell Jesup [:jesup] from comment #12)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/6acf30e09a34
Please uplift the patch for aurora fixing this bug if we are happy with the bake time/testing. Else may be we can backout patches related to getUserMedia or disable it completely for FF18 as per comment 1 & 3
Assignee | ||
Comment 17•12 years ago
|
||
Comment on attachment 673768 [details] [diff] [review]
Force MediaManager to be created from MainThread
[Approval Request Comment]
Bug caused by (feature/regressing bug #): 752353 (or so)
User impact if declined: Crashes or worse *if* the feature is turned on by the user.
Testing completed (on m-c, etc.): Well-baked on m-c
Risk to taking this patch (and alternatives if risky): Almost no risk; well bakes and simple patch. Also turned off by default, so there's no risk to normal users if left alone.
String or UUID changes made by this patch: none
Attachment #673768 -
Flags: approval-mozilla-aurora?
Updated•12 years ago
|
Attachment #673768 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 18•12 years ago
|
||
trivial unbitrot for Aurora
Assignee | ||
Updated•12 years ago
|
Attachment #673768 -
Attachment is obsolete: true
Comment 19•12 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•