Closed Bug 1189058 Opened 9 years ago Closed 9 years ago

unresponsive gUM requiring restart on Android after http://webrtc.github.io/samples/src/content/devices/input-output

Categories

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

39 Branch
Unspecified
Android
defect

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox39 --- wontfix
firefox40 --- verified
firefox41 --- fixed
firefox42 --- fixed
firefox-esr31 --- wontfix
firefox-esr38 --- wontfix

People

(Reporter: jib, Assigned: gcp)

References

Details

(Keywords: sec-high)

Attachments

(1 file)

I'm filing this separate from Bug 1188816 since the STRs are slightly different, though it may end up being the same bug.

STR:
1. Open http://jsfiddle.net/jib1/Lqo4paed , hit [gUM!] button + accept
   and verify that camera works, then refresh page (or hit [Run] button).

2. Visit http://webrtc.github.io/samples/src/content/devices/input-output
   in another tab.

3. Go back to first tab and repeat step 1.

Expected result:

    gUM works.

Actual result:

    gUM never asks for permission and nothing happens, and worse, gUM now appears hosed in the system at this point, and wont work in any page or tab!

Even closing all apps and restarting Firefox seems to not solve this, and I've had to restart my phone, which fixes it.

Although if I visited Firefox Beta at this point then it worked there, which was weird.

I suspect this may have something to do with the sample site calling enumerateDevices, and that site is not working correctly, since labels should appear on buttons but aren't.

That said, there's an [Enumerate] button next to [gUM!] and that seems to work most of the time, HOWEVER I was able to make gUM hang once or twice by hitting [enumerate] about 10 times before hitting [gUM!], although this is less consistent than the STRs which happen every time. It also didn't seem to require a restart that time. Odd.
Given that this seems to be tougher to hit, I'm making this a lower priority than bug 1188816, but it's obviously pretty bad when it happens.  Not sure that this is the same bug 1188816.  There are some similarities.  

Jib -- You're working on more important stuff at the moment.  When that's done, would you feel comfortable looking into this (as a bug owner), or are you not sure where to start?  (Cc'ing gcp since this also appears to be Android-only, this may be related to 1188816, and he may have ideas on how to attack this.)
backlog: --- → webRTC+
Rank: 15
Flags: needinfo?(jib)
Priority: -- → P1
This seems 100% reproducible for me with the STRs, and I'll definitely take a look after I clear my other higher priority items, but others please feel free to look as well if you can reproduce it, in case it helps with Bug 1188816.
Flags: needinfo?(jib)
I found this in the log. Reproduces on Nexus 4.

I/Gecko   ( 4145): [4145] ###!!! ASSERTION: Potential deadlock detected:
I/Gecko   ( 4145): Cyclical dependency starts at
I/Gecko   ( 4145): Mutex : mozilla::MediaEngineWebRTC (currently acquired)
I/Gecko   ( 4145): Cycle completed at
I/Gecko   ( 4145): Mutex : mozilla::MediaEngineWebRTC (currently acquired)
I/Gecko   ( 4145): 
I/Gecko   ( 4145): ###!!! Deadlock may happen NOW!
I/Gecko   ( 4145):
+jesup because the deadlock is in generic code.
(In reply to Jan-Ivar Bruaroey [:jib] from comment #0)
> Even closing all apps and restarting Firefox seems to not solve this, and
> I've had to restart my phone, which fixes it.
> 
> Although if I visited Firefox Beta at this point then it worked there, which
> was weird.

The first paragraph strongly hints at a hardware/driver bug, but the above and the second paragraph make me think you were not successful in actually killing off the running Firefox instance.
I got a backtrace for the deadlock:

Thread 3 (Thread 9936):
#0  0x400675d0 in __futex_syscall3 () from /home/morbo/git/jimdb-arm/lib/00658c727e962a43/system/lib/libc.so
#1  0x40058e90 in ?? () from /home/morbo/git/jimdb-arm/lib/00658c727e962a43/system/lib/libc.so
#2  0x7746e50a in PR_Lock (lock=0x904faba0) at /home/morbo/hg/mozilla-central/nsprpub/pr/src/pthreads/ptsynch.c:177
#3  0x7b90aa0a in mozilla::OffTheBooksMutex::Lock (this=0x8b23221c)
    at /home/morbo/hg/mozilla-central/xpcom/glue/BlockingResourceBase.cpp:382
#4  0x7d0804fe in BaseAutoLock (_notifier=<optimized out>, aLock=..., this=<synthetic pointer>)
    at ../../../dist/include/mozilla/Mutex.h:164
#5  mozilla::MediaEngineWebRTC::EnumerateVideoDevices (this=0x8b232210, aMediaSource=mozilla::dom::Camera, 
    aVSources=0x93c5f7b8) at /home/morbo/hg/mozilla-central/dom/media/webrtc/MediaEngineWebRTC.cpp:81
#6  0x7cf3a06a in GetSources<mozilla::VideoDevice> (media_device_name=0x0, aResult=..., 
    aEnumerate=&virtual mozilla::MediaEngine::EnumerateVideoDevices(mozilla::dom::MediaSourceEnum, nsTArray<nsRefPtr<moz
illa::MediaEngineVideoSource> >*), aSrcType=mozilla::dom::Camera, engine=0x8b232210)
    at /home/morbo/hg/mozilla-central/dom/media/MediaManager.cpp:922
#7  operator() (__closure=0x8bb165c0) at /home/morbo/hg/mozilla-central/dom/media/MediaManager.cpp:1309
#8  mozilla::media::LambdaTask<mozilla::MediaManager::EnumerateRawDevices(uint64_t, mozilla::dom::MediaSourceEnum, mozil
la::dom::MediaSourceEnum, bool, bool)::<lambda()> >::Run(void) (this=0x8bb165b0)
    at ../../dist/include/mozilla/media/MediaUtils.h:306
#9  0x7bb9fa5a in MessageLoop::RunTask (this=this@entry=0x93c5fdd0, task=0x8bb165b0)
    at /home/morbo/hg/mozilla-central/ipc/chromium/src/base/message_loop.cc:364
#10 0x7bba4606 in MessageLoop::DeferOrRunPendingTask (this=this@entry=0x93c5fdd0, pending_task=...)
    at /home/morbo/hg/mozilla-central/ipc/chromium/src/base/message_loop.cc:372
#11 0x7bba65f8 in MessageLoop::DoWork (this=0x93c5fdd0)
    at /home/morbo/hg/mozilla-central/ipc/chromium/src/base/message_loop.cc:459
#12 0x7bbcb3c8 in mozilla::ipc::DoWorkRunnable::Run (this=<optimized out>)
    at /home/morbo/hg/mozilla-central/ipc/glue/MessagePump.cpp:220
#13 0x7b8e868e in nsThread::ProcessNextEvent (this=0x8f99e4c0, aMayWait=<optimized out>, aResult=0x93c5f8d7)
    at /home/morbo/hg/mozilla-central/xpcom/threads/nsThread.cpp:867
#14 0x7b913484 in NS_ProcessNextEvent (aThread=<optimized out>, aMayWait=<optimized out>)
    at /home/morbo/hg/mozilla-central/xpcom/glue/nsThreadUtils.cpp:277
#15 0x7b8ed134 in nsThread::DispatchInternal(already_AddRefed<nsIRunnable>&&, unsigned int, nsThread::nsNestedEventTarge
t*) (this=this@entry=0x73d43a10, 
    aEvent=aEvent@entry=<unknown type in /home/morbo/hg/mozilla-central/objdir-android/dist/bin/libxul.so, CU 0x4d9ef9, 
DIE 0x55ec33>, aFlags=aFlags@entry=1, aTarget=aTarget@entry=0x0)
    at /home/morbo/hg/mozilla-central/xpcom/threads/nsThread.cpp:579
#16 0x7b8ed27a in nsThread::Dispatch(already_AddRefed<nsIRunnable>&&, unsigned int) (this=0x73d43a10, 
    aEvent=<unknown type in /home/morbo/hg/mozilla-central/objdir-android/dist/bin/libxul.so, CU 0x4d9ef9, DIE 0x55ecfc>
, aFlags=1) at /home/morbo/hg/mozilla-central/xpcom/threads/nsThread.cpp:603
#17 0x7d4f7536 in Dispatch (aFlags=1, aEvent=0x87916260, this=0x73d43a10) at ../../dist/include/nsIEventTarget.h:37
#18 jsjni_GetGlobalClassRef (className=<optimized out>)
    at /home/morbo/hg/mozilla-central/widget/android/AndroidJNIWrapper.cpp:69
#19 0x7d9d66c8 in webrtc::AudioManagerJni::SetAndroidAudioDeviceObjects (jvm=0x414eacb0, env=0x8ccd7d98, 
    context=0x1f900406)
    at /home/morbo/hg/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_device/android/audio_manager_jni.cc:58
#20 0x7d9d2f76 in webrtc::OpenSlesInput::SetAndroidAudioDeviceObjects (javaVM=<optimized out>, env=<optimized out>, 
    context=<optimized out>)
    at /home/morbo/hg/mozilla-central/media/webrtc/trunk/webrtc/modules/audio_device/opensl/../android/opensles_input.cc:90
#21 0x7d9f181a in SetAndroidAudioDeviceObjects (context=0x1f900406, env=0x8ccd7d98, javaVM=0x414eacb0)
    at /home/morbo/hg/mozilla-central/media/webrtc/trunk/webrtc/voice_engine/../../webrtc/modules/audio_device/android/audio_device_template.h:31
#22 webrtc::VoiceEngine::SetAndroidObjects (javaVM=0x414eacb0, env=0x8ccd7d98, context=0x1f900406)
    at /home/morbo/hg/mozilla-central/media/webrtc/trunk/webrtc/voice_engine/voice_engine_impl.cc:172
#23 0x7d07fe42 in mozilla::MediaEngineWebRTC::EnumerateAudioDevices (this=0x8b232210, aMediaSource=<optimized out>, 
    aASources=0x93c5fc30) at /home/morbo/hg/mozilla-central/dom/media/webrtc/MediaEngineWebRTC.cpp:308
#24 0x7cf3a052 in GetSources<mozilla::AudioDevice> (media_device_name=0x0, aResult=..., 
    aEnumerate=&virtual mozilla::MediaEngine::EnumerateAudioDevices(mozilla::dom::MediaSourceEnum, nsTArray<nsRefPtr<mozilla::MediaEngineAudioSource> >*), aSrcType=mozilla::dom::Microphone, engine=0x8b232210)
    at /home/morbo/hg/mozilla-central/dom/media/MediaManager.cpp:922
#25 operator() (__closure=0x8b0e34d0) at /home/morbo/hg/mozilla-central/dom/media/MediaManager.cpp:1316
#26 mozilla::media::LambdaTask<mozilla::MediaManager::EnumerateRawDevices(uint64_t, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::<lambda()> >::Run(void) (this=0x8b0e34c0)
    at ../../dist/include/mozilla/media/MediaUtils.h:306
#27 0x7bb9fa5a in MessageLoop::RunTask (this=this@entry=0x93c5fdd0, task=0x8b0e34c0)
    at /home/morbo/hg/mozilla-central/ipc/chromium/src/base/message_loop.cc:364
#28 0x7bba4606 in MessageLoop::DeferOrRunPendingTask (this=this@entry=0x93c5fdd0, pending_task=...)
    at /home/morbo/hg/mozilla-central/ipc/chromium/src/base/message_loop.cc:372
#29 0x7bba65f8 in MessageLoop::DoWork (this=0x93c5fdd0)
    at /home/morbo/hg/mozilla-central/ipc/chromium/src/base/message_loop.cc:459
#30 0x7bbcb3c8 in mozilla::ipc::DoWorkRunnable::Run (this=<optimized out>)
    at /home/morbo/hg/mozilla-central/ipc/glue/MessagePump.cpp:220
#31 0x7b8e868e in nsThread::ProcessNextEvent (this=0x8f99e4c0, aMayWait=<optimized out>, aResult=0x93c5fd4f)
    at /home/morbo/hg/mozilla-central/xpcom/threads/nsThread.cpp:867
#32 0x7b913484 in NS_ProcessNextEvent (aThread=<optimized out>, aMayWait=<optimized out>)
    at /home/morbo/hg/mozilla-central/xpcom/glue/nsThreadUtils.cpp:277
#33 0x7bbd431c in mozilla::ipc::MessagePumpForNonMainThreads::Run (this=0x8c6ea970, aDelegate=0x93c5fdd0)
    at /home/morbo/hg/mozilla-central/ipc/glue/MessagePump.cpp:355
#34 0x7bb9f92a in MessageLoop::RunInternal (this=this@entry=0x93c5fdd0)
    at /home/morbo/hg/mozilla-central/ipc/chromium/src/base/message_loop.cc:234
#35 0x7bb9faa8 in RunHandler (this=0x93c5fdd0)
    at /home/morbo/hg/mozilla-central/ipc/chromium/src/base/message_loop.cc:227
#36 MessageLoop::Run (this=0x93c5fdd0) at /home/morbo/hg/mozilla-central/ipc/chromium/src/base/message_loop.cc:201
#37 0x7bbaef26 in base::Thread::ThreadMain (this=0x8c6ea910)
    at /home/morbo/hg/mozilla-central/ipc/chromium/src/base/thread.cc:170
#38 0x7bbaa0a8 in ThreadFunc (closure=<optimized out>)
    at /home/morbo/hg/mozilla-central/ipc/chromium/src/base/platform_thread_posix.cc:39
#39 0x40057a5c in __thread_entry () from /home/morbo/git/jimdb-arm/lib/00658c727e962a43/system/lib/libc.so
#40 0x40057bd8 in pthread_create () from /home/morbo/git/jimdb-arm/lib/00658c727e962a43/system/lib/libc.so
#41 0x00000000 in ?? ()
So, what happens:

WebRTC tries to initialize, calls the functions to set up the Android end. For this, the Android ends needs to initialize Java stuff on the main thread. It does this with a DISPATCH_SYNC. However, when the main thread event loop spins, it already contains another message requesting another WebRTC call. That one tries to take the same lock and we have a deadlock.
>However, when the main thread event loop spins

Actually, it's the WebRTC event loop. Maybe this just needs a SyncRunnable. We definitely don't want to process WebRTC events while we're trying to set it up. Now, please tell me the main thread isn't SYNC dispatching into the WebRTC thread too.
Assignee: nobody → gpascutto
Attachment #8641188 - Flags: review?(snorp) → review+
Gian-Carlo -- Does this affect all branches?  (How far do we need to uplift this?)
Flags: needinfo?(gpascutto)
Group: core-security
(In reply to Gian-Carlo Pascutto [:gcp] from comment #8)
> Now, please tell me the main thread isn't SYNC dispatching into the
> WebRTC thread too.

We already assume this elsewhere FWIW:
http://mxr.mozilla.org/mozilla-central/source/dom/media/MediaManager.cpp?rev=4c62be998cb4&mark=1372-1374#1371
I'd guess at least sec-high, though rather hard to exploit if possible.
Comment on attachment 8641188 [details] [diff] [review]
Don't spin the Event loop while setting up WebRTC + Java

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Probably very hard

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
The current checkin comment likely would at least advertise the problem; without that comment it won't be at all obvious what the security issue would be.

Which older supported branches are affected by this flaw?
Probably all of them.

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Backport should be trivial or apply directly


How likely is this patch to cause regressions; how much testing does it need?
Low chance of regressions, and only real possible regression would be a deadlock.  Per comments here, we already assume there are no sync dispatches from Main to this thread.
Attachment #8641188 - Flags: sec-approval?
Attachment #8641188 - Flags: approval-mozilla-esr38?
Attachment #8641188 - Flags: approval-mozilla-beta?
Attachment #8641188 - Flags: approval-mozilla-aurora?
Attachment #8641188 - Flags: sec-approval? → sec-approval+
Android isn't supported on esr38, so I think we can wontfix this bug there.
Flags: needinfo?(rjesup)
Attachment #8641188 - Flags: approval-mozilla-esr38?
Comment on attachment 8641188 [details] [diff] [review]
Don't spin the Event loop while setting up WebRTC + Java

Taking for 40 (before it lands in m-c) as the RC go to build today.

Adding to approval-mozilla-release to make sure it lands in the 40 branch as we are going to do the m-b => m-r merge today.
Attachment #8641188 - Flags: approval-mozilla-release+
Attachment #8641188 - Flags: approval-mozilla-beta?
Attachment #8641188 - Flags: approval-mozilla-beta+
Attachment #8641188 - Flags: approval-mozilla-aurora?
Attachment #8641188 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/de87db481f88
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
QA can verify this with the STR in the description, but given that this is going into RC it's also very important basic gUM/WebRTC functionality still works.
Flags: needinfo?(gpascutto)
Using the STR from comment#0 I was able to reproduce the bug on Firefox 40.0b8.
On Firefox 40.0b10 the bug is no longer reproducible. 
We also did some exploratory testing on GUM/WebRTC functionality and it's working as expected. The only issue that showed up was a crash and happened one time only.
See Bug #1190841
Based on that I'm verifying the bug as fixed on Firefox 40
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.