776558 - Add special test mode to complete crash processing before shutdown

Status: NEW

(Toolkit :: Crash Reporting, enhancement)

Description

[Bob Clary [:bc] (inactive)]

It appears that the new Flash sandbox/multiprocess release can cause crashes during shutdown which produce incomplete or corrupt minidumps which can not be parsed by the current minidump_stackwalk. When the patch from bug 772432 is applied to breakpad, most of these minidumps can be parsed but do not have a thread or have a single non-crashing thread.

It appears that we are shutting down the browser before the minidump is completed. If we had a special test mode where we can make the browser wait for the crash reporting to be completed before shutting down we might be able to get more useful information regarding these crashes.

Comment 1

[(not currently active) Ted Mielczarek]

That makes a lot of sense as the root cause for busted minidumps. I think we should probably add an environment variable, like MOZ_WAIT_FOR_CHILD_PROCESSES that causes us to wait for all child processes to terminate on shutdown (whether they exit normally or crash, and we handle the minidump).

Comment 2

[Benjamin Smedberg]

What I think needs to happen here is:

We're running http://hg.mozilla.org/mozilla-central/annotate/98c2a42a3aef/toolkit/crashreporter/google-breakpad/src/client/windows/crash_generation/crash_generation_server.cc#l128

At the same time as http://hg.mozilla.org/mozilla-central/annotate/98c2a42a3aef/toolkit/crashreporter/google-breakpad/src/client/windows/crash_generation/crash_generation_server.cc#l813

I believe that the client_info->UnregisterWaits() ends up signalling/closing the dump generation event early, but I'm not actually sure of the sequence of events here and how it's safe for http://hg.mozilla.org/mozilla-central/annotate/98c2a42a3aef/toolkit/crashreporter/google-breakpad/src/client/windows/crash_generation/crash_generation_server.cc#l823 to run, since in my theory, that client_info could be dead already.

Maybe printf debugging could validate my timing theory, at least.

Comment 3

[Benjamin Smedberg]

The other option here is that we could (in this debug mode) just wait for the plugin-container process to fully exit. Something like:

At http://mxr.mozilla.org/mozilla-central/source/dom/plugins/ipc/PluginModuleParent.cpp#133 insert

WaitForSingleObject(mSubprocess->GetChildProcessHandle(), INFINITE)

bc you could try inserting that and see whether it helps.

Comment 4

[Bob Clary [:bc] (inactive)]

I can do that. Thanks.

Comment 5

[Bob Clary [:bc] (inactive)]

Attachment: test patch for comment 3

I tried this patch (updated for today's nullptr change) on Nightly and the corresponding patches for Aurora, Beta and retested a large number of Flash crash urls. I did not see an appreciable improvement unfortunately. It is difficult to determine if it had any effect at all.

Comment 6

[Bob Clary [:bc] (inactive)]

FYI, these crashes are still the most reproducible for crash automation. I would say that in excess of 90% of the flash related crashes found in automation are of this type.

Comment 7

[(not currently active) Ted Mielczarek]

Related to bug 776558?