1504372 - Always generate minidumps for replaying child crashes

Status: RESOLVED FIXED

(Core Graveyard :: Web Replay, enhancement)

Description

[Brian Hackett [Laid off!]]

Attachment: patch

There have been cases lately (observed by several people) of minidumps not being generated on record/replay tab crashes.  While it's hard to know exactly what's going on here, there are several ways in which tabs can crash without producing minidumps.  The attached patch improves this situation by keeping track of whether child processes have generated a minidump; if a replaying process crashes or hangs without generating a minidump, the middleman will crash and produce its own minidump.  We don't do anything comparable for recording processes, as they use the normal crash reporter mechanisms for generating their minidumps.  This could be improved if missing crash reports remain a problem, by making some modifications to the crash reporter itself.

There are also a couple related improvements in this bug to (A) when hanging, generate minidumps directly instead of crashing and going through the dirty memory handler (which might be having problems of its own), and (B) watch out for child processes that had a fatal error but were unable to generate a minidump; the middleman will then crash and indicate the problem in the resulting crash report.

Comment 1

[Andrew McCreight [:mccr8]]

Comment on attachment 9022344 [details] [diff] [review]
patch

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

Looks straightforward enough. If you go too much further down the road of splitting out these crashing failure conditions, please talk to Ted or somebody in case they have ideas.

::: toolkit/recordreplay/ipc/ChildIPC.cpp
@@ +108,5 @@
>      if (IsRecording()) {
>        PrintSpew("Terminate message received, exiting...\n");
>        _exit(0);
>      } else {
> +      ReportFatalError(Nothing(), "Hanged replaying process");

nit while you are here: "Hung" instead of "Hanged".

::: toolkit/recordreplay/ipc/ParentInternal.h
@@ +275,5 @@
>    bool mPauseNeeded;
>  
> +  // Flags for whether we have received messages from the child indicating it
> +  // is crashing.
> +  bool mHasBeginFatalError;

Should this be "Begun" instead of "Begin"? (I know the message is "begin" but it makes more sense in isolation as "begun".)

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b29555ff89f8
Always generate minidumps for replaying child crashes, r=mccr8.

Comment 3

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/b29555ff89f8