2050963 - Windows in-process minidump can deadlock when the MiniDumpWithHandleData stream makes dbgcore LoadLibrary while threads are suspended

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Florian Quèze [:florian]]

Symptom
On Windows, an intentionally-crashed process can hang forever in MiniDumpWriteDump instead of exiting. The crashreporter do_crash() test children hold the install-dir update lock, so a wedged one keeps the lock until it is reaped, which later makes test_backgroundupdate_exitcodes.js fail with OTHER_INSTANCE (bug 1760099).

Root cause
Our in-process minidump type (GetMinidumpType in nsExceptionHandler.cpp) includes MiniDumpWithHandleData. While MiniDumpWriteDump writes the handle-data stream it has already suspended every other thread; dbgcore (Win32LiveSystemProvider::StartHandleOperationsEnum) then calls LoadLibrary, and on Win10+ that blocks in LdrpDrainWorkQueue waiting on the loader worker threads MiniDumpWriteDump just suspended. The load never returns, the dump never finishes, the process never exits.

Five wedged holders captured across four CI jobs show the identical breakpad-handler-thread stack (bug 1760099 comment 202): LdrpDrainWorkQueue <- LdrLoadDll <- LoadLibraryExW <- dbgcore!Win32LiveSystemProvider::StartHandleOperationsEnum <- dbgcore!GenWriteHandleOperations <- MiniDumpWriteDump. The ntdll loader-trace strings in the dump name the DLL being loaded directly: verifier.dll (the Application Verifier provider), which pulls in vrfcore.dll.

Windows-only (in-process self-dump path). The out-of-process WER path (mozwer-rust) does not request MiniDumpWithHandleData, so it is unaffected.

Frequency
The test was skipped on Windows x86_64 opt (D308902) as enabling the profiler by default for xpcshell tests (bug 2032957) made it perma-fail. With the skip removed and no fix it fails ~10 of 11 runs on try.

Two candidate fixes (try, skip removed):

• no fix: 10 failures / 11 runs
• drop MiniDumpWithHandleData: 2 / 40
• pre-load verifier.dll: 2 / 39

1. Drop MiniDumpWithHandleData from GetMinidumpType, removing the mid-dump LoadLibrary. Tradeoff: Windows in-process minidumps lose the handle-data stream (WER dumps already lack it).
2. Pre-load verifier.dll at startup, next to the existing psapi.dll pre-load in SetExceptionHandler, so it is already resident when dbgcore loads it mid-dump and the load no longer blocks. Tradeoff: eagerly loads the Application Verifier provider DLL (and vrfcore.dll) into every process that initializes the crash reporter.

Both fixes leave a residual ~2/40 from a separate, still-unidentified failure mode this bug does not address; that is why it only partially fixes bug 1760099. I'll submit a patch for preloading verifier.dll, but both fixes are one liner, to happy to submit the MiniDumpWithHandleData flag removal if the handle data hasn't been useful in your experience.

Comment 1

[Florian Quèze [:florian]]

Attachment: Bug 2050963 - Pre-load verifier.dll before installing the exception handler, r=gsvelto.

When MiniDumpWithHandleData is set, MiniDumpWriteDump suspends every other thread and then, while writing the handle-data stream, dbgcore (Win32LiveSystemProvider::StartHandleOperationsEnum) loads verifier.dll. On Win10+ that LoadLibrary blocks in LdrpDrainWorkQueue waiting on the loader worker threads MiniDumpWriteDump just suspended, so the dump never finishes and the crashing process never exits, holding the update lock long enough to intermittently fail test_backgroundupdate_exitcodes.js with OTHER_INSTANCE.

Pre-loading verifier.dll up front, next to the existing psapi.dll pre-load, makes it already resident when dbgcore loads it mid-dump so the load no longer blocks, while keeping the handle-data stream in our minidumps. This also backs out the skip-if added in D308902, re-enabling the test on Windows x86_64 opt.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/de1c87065461
https://hg.mozilla.org/integration/autoland/rev/1582611a1357
Pre-load verifier.dll before installing the exception handler, r=gsvelto.

Comment 3

[Silaghi Andreea]

https://hg.mozilla.org/mozilla-central/rev/1582611a1357

Comment 4

[Florian Quèze [:florian]]

Attachment: Bug 2050963 - Pre-load verifier.dll before installing the exception handler, .

When MiniDumpWithHandleData is set, MiniDumpWriteDump suspends every other thread and then, while writing the handle-data stream, dbgcore (Win32LiveSystemProvider::StartHandleOperationsEnum) loads verifier.dll. On Win10+ that LoadLibrary blocks in LdrpDrainWorkQueue waiting on the loader worker threads MiniDumpWriteDump just suspended, so the dump never finishes and the crashing process never exits, holding the update lock long enough to intermittently fail test_backgroundupdate_exitcodes.js with OTHER_INSTANCE.

Pre-loading verifier.dll up front, next to the existing psapi.dll pre-load, makes it already resident when dbgcore loads it mid-dump so the load no longer blocks, while keeping the handle-data stream in our minidumps. This also backs out the skip-if added in D308902, re-enabling the test on Windows x86_64 opt.

Original Revision: https://phabricator.services.mozilla.com/D309183

Comment 5

[Phabricator Automation]

firefox-esr153 Uplift Approval Request

• User impact if declined/Reason for urgency: Potential deadlock when capturing a minidump on Windows. esr153 is a large part of the remaining failures in https://bugzilla.mozilla.org/show_bug.cgi?id=2050963
• Code covered by automated testing?: no
• Fix verified in Nightly?: yes
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: This only preloads one more library when initializing the crash reporter.
• String changes made/needed?: None
• Is Android affected?: no

Comment 6

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr153/rev/4aa2bea4f42b