This bug is for crash report bp-0e96c027-4b06-4c13-8f51-507450190519.

Top 10 frames of crashing thread:

0 mozglue.dll mozalloc_abort memory/mozalloc/mozalloc_abort.cpp:33
1 xul.dll static void Abort xpcom/base/nsDebugImpl.cpp:439
2 xul.dll NS_DebugBreak xpcom/base/nsDebugImpl.cpp
3 xul.dll nsresult nsDebugImpl::Abort xpcom/base/nsDebugImpl.cpp:133
4 xul.dll XPTC__InvokebyIndex 
5  @0x257149b837f 
6 xul.dll round 
7 xul.dll round 
8 xul.dll static bool XPCWrappedNative::CallMethod js/xpconnect/src/XPCWrappedNative.cpp:1157
9 xul.dll static bool XPC_WN_CallMethod js/xpconnect/src/XPCWrappedNativeJSOps.cpp:943

Dan, can you take a look at this? Seems like a classic mutex deadlock in the DirectShow backend. Thread 50 is locking mutexes on its way down into webrtc::videocapturemodule::CaptureSinkFilter ([1], then [2]) and thread 56 is locking them in reverse order on its way up from webrtc::videocapturemodule::CaptureSinkFilter ([3], then [4]).

[1] https://hg.mozilla.org/mozilla-central/annotate/9b2f851979cb8d0dd0cd2618656eddee32e4f143/media/webrtc/trunk/webrtc/modules/video_capture/windows/video_capture_ds.cc#l154
[2] https://hg.mozilla.org/mozilla-central/annotate/9b2f851979cb8d0dd0cd2618656eddee32e4f143/media/webrtc/trunk/webrtc/modules/video_capture/windows/sink_filter_ds.cc#l393
[3] https://hg.mozilla.org/mozilla-central/annotate/9b2f851979cb8d0dd0cd2618656eddee32e4f143/media/webrtc/trunk/webrtc/modules/video_capture/windows/sink_filter_ds.cc#l319
[4] https://hg.mozilla.org/mozilla-central/annotate/9b2f851979cb8d0dd0cd2618656eddee32e4f143/media/webrtc/trunk/webrtc/modules/video_capture/video_capture_impl.cc#l140

This updates just video_capture/windows to the tip of webrtc.org, from commit
8581877121f58435d683c2a39a73e72e5dcd558a.

This removes the locking which was leading to the deadlock in Bug 1552755. It
also removes the reliance upon the DirectShow example code which lead to our
local implementations of BaseFilter, BaseInputPin, BasePin, and MediaType.
These can now be removed.

This was added in commit 74395345e8d2fecd504cb687eb79deee4ef394c3, but this
only cherry picks the ToHex part of that commit.

Depends on D33336

A few small changes are required to backport the updated windows capture code
to our version of webrtc.org. These are largely path changes, but
DetachFromThread has been renamed to Detach, and they seem to have a working
overload for ostream operator<< for VideoType that I have not been able to
track down. It is also necessary to add our local modification for pid_t back
to the GetDeviceName method, but it is not used for video capture anyways.

Depends on D33337

Depends on D33338

Depends on D33339

Try job here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f7ecd883183209900763ef7e8341e72c4ef0cc8b
I've done the usual local testing.

https://hg.mozilla.org/mozilla-central/rev/a1967aabb055
https://hg.mozilla.org/mozilla-central/rev/0f0fdb21b674
https://hg.mozilla.org/mozilla-central/rev/c2c056771ee9
https://hg.mozilla.org/mozilla-central/rev/d28872da6016
https://hg.mozilla.org/mozilla-central/rev/e9685aeaa59a

Please nominate this for Beta approval when you get a chance.

Comment on attachment 9069073 [details]
Bug 1552755 - Update moz.build files; r=ng!

Beta/Release Uplift Approval Request

• User impact if declined: This should prevent a shutdown hang due to a deadlock that results when shutting down video capture at the same time a new frame is being delivered. This removes one of the locks that was causing problems.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a low risk import code from upstream webrtc.org. It initially landed there in early April so there is a good chance that any problems would have subsequently been detected and fixed upstream prior to us importing it.
• String changes made/needed: None

I see there are still some shutdown hangs with this signature. They do not seem to be occurring in the same place as before, so I think they are a separate problem.

I don't suppose there's a more targeted fix we could do for beta?

(In reply to Julien Cristau [:jcristau] from comment #13)

I don't suppose there's a more targeted fix we could do for beta?

Because it is an import of upstream code it is all or nothing I'm afraid. Trying to break out just the part of their changes that removed the locking would be more risky than taking the whole thing in my opinion.

OK I guess I'd rather live with this bug then, it doesn't seem worse than bug 1407415 that we had previously... :/