832413 - Use ReportHooks to replace MS CRT assert dialogs in xpcshell

Status: NEW

(Testing :: General, enhancement, P3)

Description

[Georg Fritzsche [:gfritzsche]]

Bug 587982 got rid of CRT assert dialogs in the browser, but that change was never carried over to xpcshell so that simple failures like failed memory allocation leads to test timeouts in debug builds.

Comment 1

[Georg Fritzsche [:gfritzsche]]

My straight-forward attempt of using the same approach as bug 587982 successfully added the hooks, but hung afterwards below crashreporter code:

(from bug 813245 comment #16)
> Doing a straight-forward adoption i get a hang which is apparently waiting
> on the CrashReporter thread, which hangs in allocation related code (hanging
> on heap unlock in the CRT), stack below.
> 
> We receive allocation failure crash reports for Fx, so this must be another
> setup issue? Ted, any ideas?
> 
> ntdll.dll!_NtWaitForSingleObject@12()  + 0x15 bytes	
> ntdll.dll!_NtWaitForSingleObject@12()  + 0x15 bytes	
> browsercomps.dll!_heap_alloc_dbg_impl(unsigned int nSize, int nBlockUse,
> const char * szFileName, int nLine, int * errno_tmp)  Line 507 + 0x7 bytes
> C++
> ntdll.dll!_RtlInitUnicodeStringEx@8() 	
> msvcr100d.dll!__lock()  + 0x3d bytes	
> msvcr100d.dll!__free_dbg()  + 0x39 bytes	
> msvcr100d.dll!_free()  + 0x10 bytes	
> mozalloc.dll!moz_free(void * ptr)  Line 48 + 0xa bytes	C++
> xul.dll!operator delete[](void * ptr)  Line 236 + 0xa bytes	C++
> xul.dll!nsAutoArrayPtr<wchar_t>::~nsAutoArrayPtr<wchar_t>()  Line 475 + 0x11
> bytes	C++
> xul.dll!`anonymous namespace'::patched_LdrLoadDll(wchar_t * filePath,
> unsigned long * flags, _UNICODE_STRING * moduleFileName, void * * handle) 
> Line 453 + 0x27 bytes	C++
> KernelBase.dll!74d92c95() 	
> [Frames below may be incorrect and/or missing, no symbols loaded for
> KernelBase.dll]	
> Nvd3d9wrap.dll!74642654() 	
> dbghelp.dll!Win32LiveSystemProvider::Initialize()  + 0x10 bytes	
> dbghelp.dll!NtWin32LiveSystemProvider::Initialize()  + 0xa bytes	
> dbghelp.dll!_MiniDumpCreateLiveSystemProvider@8()  + 0xb3 bytes	
> dbghelp.dll!_MiniDumpWriteDump@28()  + 0x38 bytes	
> xul.dll!google_breakpad::ExceptionHandler::
> WriteMinidumpWithExceptionForProcess(unsigned long requesting_thread_id,
> _EXCEPTION_POINTERS * exinfo, MDRawAssertionInfo * assertion, void *
> process, bool write_requester_stream)  Line 990 + 0x43 bytes	C++
> xul.dll!google_breakpad::ExceptionHandler::
> WriteMinidumpWithException(unsigned long requesting_thread_id,
> _EXCEPTION_POINTERS * exinfo, MDRawAssertionInfo * assertion)  Line 819 +
> 0x1d bytes	C++
> xul.dll!google_breakpad::ExceptionHandler::ExceptionHandlerThreadMain(void *
> lpParameter)  Line 374 + 0x26 bytes	C++

Comment 2

[Georg Fritzsche [:gfritzsche]]

Moving the need-info request over here :)

Comment 3

[(not currently active) Ted Mielczarek]

I have no idea what the hell is going on in this stack. The only thing I note is that it's going through our patched LdrLoadDll, so maybe this is related to bug 830001?

Comment 4

[:ehsan akhgari]

Can I please see a patch plus the stack trace from all threads when the hang happens?

Thanks!

Comment 5

[Georg Fritzsche [:gfritzsche]]

(In reply to :Ehsan Akhgari from comment #4)
> Can I please see a patch plus the stack trace from all threads when the hang
> happens?

Sorry, coming up now - i was in a hurry when breaking this out.

Comment 6

[Georg Fritzsche [:gfritzsche]]

Attachment: test-case

Test-case, add an xpcshell-test that allocates |0x7fffffff * 3| bytes (will be fixed by bug 813245).

Comment 7

[Georg Fritzsche [:gfritzsche]]

Attachment: Add report hooks

Straight-forward adoption of the report-hooks.

Comment 8

[Georg Fritzsche [:gfritzsche]]

Attachment: Stacks

~*k

Comment 9

[:ehsan akhgari]

Hmm, I would just call OutputDebugString and not bother with libc in MSCRTReportHook.  It seems like there's some allocator locks being held on the stack, so avoiding allocations in the hook might in fact solve this.