From 645f80c048f8272ef3e7da92b16d175522bf377a Mon Sep 17 00:00:00 2001 From: Brian Hackett Date: Fri, 2 Nov 2018 14:30:39 -1000 Subject: [PATCH] Bug 1504372 - Always generate minidumps for replaying child crashes, r=mccr8. --HG-- extra : rebase_source : f68f98127fc914339db47f57bf365a950ca2f2b4 --- toolkit/recordreplay/ipc/Channel.cpp | 1 + toolkit/recordreplay/ipc/Channel.h | 7 ++++++ toolkit/recordreplay/ipc/ChildIPC.cpp | 7 +++++- toolkit/recordreplay/ipc/ChildProcess.cpp | 29 ++++++++++++++++++----- toolkit/recordreplay/ipc/ParentInternal.h | 5 ++++ 5 files changed, 42 insertions(+), 7 deletions(-) diff --git a/toolkit/recordreplay/ipc/Channel.cpp b/toolkit/recordreplay/ipc/Channel.cpp index b258a74cce9d..881a3cc32e58 100644 --- a/toolkit/recordreplay/ipc/Channel.cpp +++ b/toolkit/recordreplay/ipc/Channel.cpp @@ -151,6 +151,7 @@ void Channel::SendMessage(const Message& aMsg) { MOZ_RELEASE_ASSERT(NS_IsMainThread() || + aMsg.mType == MessageType::BeginFatalError || aMsg.mType == MessageType::FatalError || aMsg.mType == MessageType::MiddlemanCallRequest); diff --git a/toolkit/recordreplay/ipc/Channel.h b/toolkit/recordreplay/ipc/Channel.h index 08c6079cd0bd..117f6da8d085 100644 --- a/toolkit/recordreplay/ipc/Channel.h +++ b/toolkit/recordreplay/ipc/Channel.h @@ -109,8 +109,13 @@ namespace recordreplay { \ /* A critical error occurred and execution cannot continue. The child will */ \ /* stop executing after sending this message and will wait to be terminated. */ \ + /* A minidump for the child has been generated. */ \ _Macro(FatalError) \ \ + /* Sent when a fatal error has occurred, but before the minidump has been */ \ + /* generated. */ \ + _Macro(BeginFatalError) \ + \ /* The child's graphics were repainted. */ \ _Macro(Paint) \ \ @@ -371,6 +376,8 @@ struct FatalErrorMessage : public Message const char* Error() const { return Data(); } }; +typedef EmptyMessage BeginFatalErrorMessage; + // The format for graphics data which will be sent to the middleman process. // This needs to match the format expected for canvas image data, to avoid // transforming the data before rendering it in the middleman process. diff --git a/toolkit/recordreplay/ipc/ChildIPC.cpp b/toolkit/recordreplay/ipc/ChildIPC.cpp index 0efa27599731..ca2c35bdb63b 100644 --- a/toolkit/recordreplay/ipc/ChildIPC.cpp +++ b/toolkit/recordreplay/ipc/ChildIPC.cpp @@ -108,8 +108,9 @@ ChannelMessageHandler(Message* aMsg) PrintSpew("Terminate message received, exiting...\n"); _exit(0); } else { - MOZ_CRASH("Hanged replaying process"); + ReportFatalError(Nothing(), "Hung replaying process"); } + break; } case MessageType::SetIsActive: { const SetIsActiveMessage& nmsg = (const SetIsActiveMessage&) *aMsg; @@ -350,6 +351,10 @@ CreateCheckpoint() void ReportFatalError(const Maybe& aMinidump, const char* aFormat, ...) { + // Notify the middleman that we are crashing and are going to try to write a + // minidump. + gChannel->SendMessage(BeginFatalErrorMessage()); + // Unprotect any memory which might be written while producing the minidump. UnrecoverableSnapshotFailure(); diff --git a/toolkit/recordreplay/ipc/ChildProcess.cpp b/toolkit/recordreplay/ipc/ChildProcess.cpp index c8e72f5f48c9..8ca7fa76a718 100644 --- a/toolkit/recordreplay/ipc/ChildProcess.cpp +++ b/toolkit/recordreplay/ipc/ChildProcess.cpp @@ -40,6 +40,8 @@ ChildProcessInfo::ChildProcessInfo(UniquePtr aRole, , mNumRecoveredMessages(0) , mRole(std::move(aRole)) , mPauseNeeded(false) + , mHasBegunFatalError(false) + , mHasFatalError(false) { MOZ_RELEASE_ASSERT(NS_IsMainThread()); @@ -199,7 +201,11 @@ ChildProcessInfo::OnIncomingMessage(size_t aChannelId, const Message& aMsg) } // Always handle fatal errors in the same way. - if (aMsg.mType == MessageType::FatalError) { + if (aMsg.mType == MessageType::BeginFatalError) { + mHasBegunFatalError = true; + return; + } else if (aMsg.mType == MessageType::FatalError) { + mHasFatalError = true; const FatalErrorMessage& nmsg = static_cast(aMsg); OnCrash(nmsg.Error()); return; @@ -527,12 +533,23 @@ ChildProcessInfo::OnCrash(const char* aWhy) { MOZ_RELEASE_ASSERT(NS_IsMainThread()); - // If a child process crashes or hangs then annotate the crash report and - // shut down cleanly so that we don't mask the report with our own crash. - // We want the crash to happen quickly so the user doesn't get a hanged tab. + // If a child process crashes or hangs then annotate the crash report. CrashReporter::AnnotateCrashReport(CrashReporter::Annotation::RecordReplayError, nsAutoCString(aWhy)); - Shutdown(); + + // If we received a FatalError message then the child generated a minidump. + // Shut down cleanly so that we don't mask the report with our own crash. + if (mHasFatalError) { + Shutdown(); + } + + // Indicate when we crash if the child tried to send us a fatal error message + // but had a problem either unprotecting system memory or generating the + // minidump. + MOZ_RELEASE_ASSERT(!mHasBegunFatalError); + + // The child crashed without producing a minidump, produce one ourselves. + MOZ_CRASH("Unexpected child crash"); } /////////////////////////////////////////////////////////////////////////////// @@ -613,7 +630,7 @@ ChildProcessInfo::WaitUntil(const std::function& aCallback) sentTerminateMessage = true; } else { // The child is still non-responsive after sending the terminate - // message, fail without producing a minidump. + // message. OnCrash("Child process non-responsive"); } } diff --git a/toolkit/recordreplay/ipc/ParentInternal.h b/toolkit/recordreplay/ipc/ParentInternal.h index 51789c46a52e..f51231c7ec7e 100644 --- a/toolkit/recordreplay/ipc/ParentInternal.h +++ b/toolkit/recordreplay/ipc/ParentInternal.h @@ -274,6 +274,11 @@ class ChildProcessInfo // Whether we need this child to pause while the recording is updated. bool mPauseNeeded; + // Flags for whether we have received messages from the child indicating it + // is crashing. + bool mHasBegunFatalError; + bool mHasFatalError; + void OnIncomingMessage(size_t aChannelId, const Message& aMsg); void OnIncomingRecoveryMessage(const Message& aMsg); void SendNextRecoveryMessage();