1419488 - Crash in CDeviceEnumerator::DestroyHWndNotificationThread. Shutdownhang/crash.

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect, P1)

Description

[Marcia Knous [:marcia]]

This bug was filed from the Socorro interface and is
report bp-4275d32a-77d0-49b6-884a-627590171121.
=============================================================

Seen while looking at beta crash stats: http://bit.ly/2hRQAK5. All Windows 7 crashes - 232 crashes so far.

Top 10 frames of crashing thread:

0 ntdll.dll NtWaitForSingleObject 
1 kernelbase.dll WaitForSingleObjectEx 
2 kernel32.dll WaitForSingleObjectExImplementation 
3 kernel32.dll WaitForSingleObject 
4 mmdevapi.dll CDeviceEnumerator::DestroyHWndNotificationThread 
5 mmdevapi.dll CDeviceEnumerator::SubEnumeratorUnregisterEndpointNotificationCallback 
6 mmdevapi.dll CDeviceEnumerator::UnregisterEndpointNotificationCallback 
7 audioses.dll CAudioSessionControl::FinalRelease 
8 audioses.dll ATL::CComObject<CAudioSessionControl>::~CComObject<CAudioSessionControl> 
9 audioses.dll ATL::CComObject<CAudioSessionControl>::`vector deleting destructor' 

=============================================================

Comment 1

[Firefox Bug Husbandry Bot]

https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Move_fix-optionals

Comment 2

[Trevor Skywalker]

STR: Shut down Firefox. Results in Shutdownhang/crash.

FF51
hang | CDeviceEnumerator::DestroyHWndNotificationThread

FF41-60
shutdownhang | CDeviceEnumerator::DestroyHWndNotificationThread

FF60-61
CDeviceEnumerator::DestroyHWndNotificationThread

Comment 3

[Trevor Skywalker]

Signature replaced with an IPC Channel Error, was: "CDeviceEnumerator::DestroyHWndNotificationThread"


Firefox 60.0b12 Crash Report [@ IPCError-browser | ShutDownKill ]
ID: 8afdcaf5-779b-44e3-85be-344b30180415

Uptime 	5,597 seconds (1 hour, 33 minutes and 17 seconds)
Release Channel 	beta
Version 	60.0b12
Build ID 	20180412172954
OS 	Windows 7
Build Architecture 	amd64
Startup Crash: False
Process Type 	content (extension)
Crash Reason 	EXCEPTION_BREAKPOINT
Crash Address 	0x771f98ea

Signature replaced with an IPC Channel Error, was: "CDeviceEnumerator::DestroyHWndNotificationThread"
 
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	ntdll.dll 	ZwWaitForSingleObject 	
1 	kernelbase.dll 	WaitForSingleObjectEx 	
2 	mmdevapi.dll 	CDeviceEnumerator::DestroyHWndNotificationThread() 	
3 	mmdevapi.dll 	CDeviceEnumerator::ReleaseHWndNotification() 	
4 	mmdevapi.dll 	CDeviceEnumerator::UnregisterEndpointNotificationCallback(IMMNotificationClient*) 	
5 	audioses.dll 	CAudioSessionControl::FinalRelease() 	
6 	audioses.dll 	ATL::CComObject<CAudioSessionControl>::~CComObject<CAudioSessionControl>() 	
7 	audioses.dll 	ATL::CComObject<CAudioSessionControl>::`vector deleting destructor'(unsigned int) 	
8 	audioses.dll 	ATL::CComObject<CAudioSessionControl>::Release() 	
9 	xul.dll 	mozilla::widget::StopAudioSession() 	widget/windows/AudioSession.cpp:110
10 	xul.dll 	mozilla::dom::ContentChild::ShutdownInternal() 	dom/ipc/ContentChild.cpp:3036
11 	xul.dll 	mozilla::dom::ContentChild::RecvShutdown() 	dom/ipc/ContentChild.cpp:2986
12 	xul.dll 	mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) 	ipc/ipdl/PContentChild.cpp:7288
13 	xul.dll 	mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) 	ipc/glue/MessageChannel.cpp:2133
14 	xul.dll 	mozilla::ipc::MessageChannel::DispatchMessageW(IPC::Message&&) 	ipc/glue/MessageChannel.cpp:2063
15 	xul.dll 	mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) 	ipc/glue/MessageChannel.cpp:1909
16 	xul.dll 	mozilla::ipc::MessageChannel::MessageTask::Run() 	ipc/glue/MessageChannel.cpp:1942
17 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1040
18 	xul.dll 	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:97
19 	xul.dll 	mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:301
20 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc:319
21 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc:299
22 	xul.dll 	nsBaseAppShell::Run() 	widget/nsBaseAppShell.cpp:157
23 	xul.dll 	nsAppShell::Run() 	widget/windows/nsAppShell.cpp:401
24 	xul.dll 	XRE_RunAppShell() 	toolkit/xre/nsEmbedFunctions.cpp:892
25 	xul.dll 	mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:269
26 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc:319
27 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc:299
28 	xul.dll 	XRE_InitChildProcess(int, char** const, XREChildData const*) 	toolkit/xre/nsEmbedFunctions.cpp:718
29 	firefox.exe 	content_process_main(mozilla::Bootstrap*, int, char** const) 	ipc/contentproc/plugin-container.cpp:50
30 	firefox.exe 	NS_internal_main(int, char**, char**) 	browser/app/nsBrowserApp.cpp:280
31 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:129
32 	firefox.exe 	__scrt_common_main_seh 	f:/dd/vctools/crt/vcstartup/src/startup/exe_common.inl:283
33 	kernel32.dll 	BaseThreadInitThunk 	
34 	ntdll.dll 	RtlUserThreadStart
 
 
---

Thunderbird 	38.7.1 	2 	0.1% 	1
Firefox 	41.0.2 	1 	0.1% 	1
Firefox 	44.0b1 	1 	0.1% 	1
Thunderbird 	45.0 	2 	0.1% 	2
Firefox 	45.0.1esr 	1 	0.1% 	1
Thunderbird 	45.1.0 	1 	0.1% 	1
Thunderbird 	45.1.1 	1 	0.1% 	1
Firefox 	45.1.1esr 	7 	0.5% 	8

Comment 4

[Ryan VanderMeulen [:RyanVM]]

David, any thoughts on what might be going on here?

Comment 5

[David Parks [:handyman]]

Maybe there is an issue here with the COM (un)initialization?  The COM init routines are pretty mixed up with extraneous state machine states.  I'll see what I can do.

Comment 6

[David Parks [:handyman]]

Attachment: Release IAudioSessionControl on background thread on Win7

This hang is usually related to background audio interrupt handlers taking too long/hanging.  Sometimes, it has found a race condition in Windows that has it waiting on the handle for a thread that has already finished.  Either way, we end up killing the (main) process.

None of the Windows functions involved in the crash even exist beyond Win7 (the core impl is totally different) so I limit this behavior to Win7.

This patch releases the IAudioSessionControl on a new worker thread.  That should be safe since the IAudioSessionControl is no longer used -- because we called `UnregisterAudioSessionNotification(this)`.  From there, shutdown can take as long as it likes.

Unfortunately, I'm using a Win7 VM to work on this so my testing for failures is not thorough.  It would benefit from someone with Win7 trying it and pushing it with e.g. inserting+removing headphones.  We could ask QA but I'd bet its safe enough for nightly.

----

Tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=01aa019a2202dec7bb9063b2818f8a385dc86a39

Comment 7

[Jim Mathies [:jimm]]

Comment on attachment 8973370 [details] [diff] [review]
Release IAudioSessionControl on background thread on Win7

Review of attachment 8973370 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/AudioSession.cpp
@@ +296,5 @@
> +  // nsCOM here.)
> +  IAudioSessionControl* aRawPtr = nullptr;
> +  aASC.forget(&aRawPtr);
> +  MOZ_ASSERT(aRawPtr);
> +  std::thread([aRawPtr]() { aRawPtr->Release(); });

I'm not sure if we allow the use of std::thread in generic mozilla code. I see it's used in servo and rust code, and a few 3rd party libs but that's about it. Please confirm this is acceptable in practice.

Comment 8

[Jim Mathies [:jimm]]

Comment on attachment 8973370 [details] [diff] [review]
Release IAudioSessionControl on background thread on Win7

minusing for confirmation of use of std::thread.

Comment 9

[David Parks [:handyman]]

Attachment: Release IAudioSessionControl on background thread on Win7

Switched from std::thread to PR_CreateThread.

Any failure of this code would look like a crash in shutdown.  I've been testing it with a Win7 VM but this isn't a good stress test (and I have had repeated, unexplained bouts of 'no-audio' with the VM -- in all versions of Firefox -- that randomly go away).  Since you volunteered, you could run this a couple of times with some page that makes sound (that part shouldnt matter tho) and then see if the browser cleanly shuts down.  But it works for me.

(Whether or not it actually fixes the crash still remains to be seen.)

Tests (and builds you can use): https://treeherder.mozilla.org/#/jobs?repo=try&revision=82f46d6f927e67136f393b65299908a8a28182fa

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a0d9b9a8e22
Release IAudioSessionControl on background thread on Win7 r=jimm

Comment 11

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/4a0d9b9a8e22

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider for Beta uplift or let it ride the trains?

Comment 13

[David Parks [:handyman]]

This is totally an experiment that needs to simmer.  It might not solve the problem and, although unlikely, it could create new problems.  So, let it ride....

Comment 14

[Worcester12345]

Just recently having this crash in Thunderbird 60.3.0.

Also:

Thunderbird 60.3.0 Crash Report [@ shutdownhang | ZwReadFile ]
ID: 89f62b15-661a-46e7-b25a-efdd20181114


Other than these, have not had many Thunderbird crashes in the past year or so.