Bug 1282776 - Finalize crash reports for child process crashes happening too early r=froydnj a=RyanVM
authorGabriele Svelto <gsvelto@mozilla.com>
Fri, 09 Aug 2019 14:23:19 +0000
changeset 542013 522b14ff237200dc29a918ab477c083648df568d
parent 542012 6b5bb3e6a030541887b8955eaa68b6c1f3f274a9
child 542014 729d67dfacef969c9527b6492fbffbef181ce70d
push id11821
push userccoroiu@mozilla.com
push dateWed, 21 Aug 2019 09:22:34 +0000
treeherdermozilla-beta@0a5ba27e4163 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj, RyanVM
bugs1282776, 1547698
milestone69.0
Bug 1282776 - Finalize crash reports for child process crashes happening too early r=froydnj a=RyanVM This changes the way crash reports for child processes happening too early during the child process' startup. Before bug 1547698 we wrote a partial .extra file with those crashes that lacked the process type. The user would not be notified of those crashes until she restarted Firefox and even when submitted those crashes would be erroneously labeled as browser crashes. After bug 1547698 we stopped writing .extra files entirely for those crashes which left orphaned .dmp files among the pending crash reports. This patch does three things to improve the situation: * It writes a partial .extra file so that the crashes are detected at the next startup. So the user is still not notified directly of these crashes but she can report them later. * It adds the process type to the .extra file so that the crash reporters are labelled correctly. * It fixes a leak in the `pidToMinidump` hash-map. Since the crashes were not finalized the `ChildProcessData` strucutre associated with them would never be fred. Differential Revision: https://phabricator.services.mozilla.com/D40810
dom/ipc/ContentParent.cpp
dom/media/gmp/GMPParent.cpp
dom/media/gmp/GMPParent.h
dom/media/ipc/RDDChild.cpp
dom/plugins/ipc/PluginModuleParent.cpp
dom/plugins/ipc/PluginModuleParent.h
gfx/ipc/GPUChild.cpp
gfx/vr/ipc/VRChild.cpp
ipc/glue/CrashReporterHost.cpp
netwerk/ipc/SocketProcessParent.cpp
security/sandbox/win/src/remotesandboxbroker/RemoteSandboxBrokerParent.cpp
toolkit/crashreporter/nsDummyExceptionHandler.cpp
toolkit/crashreporter/nsExceptionHandler.cpp
toolkit/crashreporter/nsExceptionHandler.h
toolkit/xre/nsEmbedFunctions.cpp
xpcom/build/nsXULAppAPI.h
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -1628,16 +1628,19 @@ void ContentParent::ActorDestroy(ActorDe
           mCrashReporter->GenerateCrashReport(OtherPid());
         }
 
         nsAutoString dumpID;
         if (mCrashReporter->HasMinidump()) {
           dumpID = mCrashReporter->MinidumpID();
         }
         props->SetPropertyAsAString(NS_LITERAL_STRING("dumpID"), dumpID);
+      } else {
+        CrashReporter::FinalizeOrphanedMinidump(OtherPid(),
+                                                GeckoProcessType_Content);
       }
     }
     nsAutoString cpId;
     cpId.AppendInt(static_cast<uint64_t>(this->ChildID()));
     obs->NotifyObservers((nsIPropertyBag2*)props, "ipc:content-shutdown",
                          cpId.get());
   }
 
--- a/dom/media/gmp/GMPParent.cpp
+++ b/dom/media/gmp/GMPParent.cpp
@@ -418,32 +418,34 @@ bool GMPParent::EnsureProcessLoaded() {
     return false;
   }
 
   nsresult rv = LoadProcess();
 
   return NS_SUCCEEDED(rv);
 }
 
-void GMPParent::WriteExtraDataForMinidump() {
+void GMPParent::AddCrashAnnotations() {
   mCrashReporter->AddAnnotation(CrashReporter::Annotation::GMPPlugin, true);
   mCrashReporter->AddAnnotation(CrashReporter::Annotation::PluginFilename,
                                 NS_ConvertUTF16toUTF8(mName));
   mCrashReporter->AddAnnotation(CrashReporter::Annotation::PluginName,
                                 mDisplayName);
   mCrashReporter->AddAnnotation(CrashReporter::Annotation::PluginVersion,
                                 mVersion);
 }
 
 bool GMPParent::GetCrashID(nsString& aResult) {
   if (!mCrashReporter) {
+    CrashReporter::FinalizeOrphanedMinidump(OtherPid(),
+                                            GeckoProcessType_GMPlugin);
     return false;
   }
 
-  WriteExtraDataForMinidump();
+  AddCrashAnnotations();
   if (!mCrashReporter->GenerateCrashReport(OtherPid())) {
     return false;
   }
 
   aResult = mCrashReporter->MinidumpID();
   return true;
 }
 
--- a/dom/media/gmp/GMPParent.h
+++ b/dom/media/gmp/GMPParent.h
@@ -148,17 +148,17 @@ class GMPParent final : public PGMPParen
   RefPtr<GeckoMediaPluginServiceParent> mService;
   bool EnsureProcessLoaded();
   RefPtr<GenericPromise> ReadGMPMetaData();
   RefPtr<GenericPromise> ReadGMPInfoFile(nsIFile* aFile);
   RefPtr<GenericPromise> ParseChromiumManifest(
       const nsAString& aJSON);  // Main thread.
   RefPtr<GenericPromise> ReadChromiumManifestFile(
       nsIFile* aFile);  // GMP thread.
-  void WriteExtraDataForMinidump();
+  void AddCrashAnnotations();
   bool GetCrashID(nsString& aResult);
   void ActorDestroy(ActorDestroyReason aWhy) override;
 
   mozilla::ipc::IPCResult RecvInitCrashReporter(
       Shmem&& shmem, const NativeThreadId& aThreadId);
 
   mozilla::ipc::IPCResult RecvPGMPStorageConstructor(
       PGMPStorageParent* actor) override;
--- a/dom/media/ipc/RDDChild.cpp
+++ b/dom/media/ipc/RDDChild.cpp
@@ -117,16 +117,18 @@ mozilla::ipc::IPCResult RDDChild::RecvFi
   return IPC_OK();
 }
 
 void RDDChild::ActorDestroy(ActorDestroyReason aWhy) {
   if (aWhy == AbnormalShutdown) {
     if (mCrashReporter) {
       mCrashReporter->GenerateCrashReport(OtherPid());
       mCrashReporter = nullptr;
+    } else {
+      CrashReporter::FinalizeOrphanedMinidump(OtherPid(), GeckoProcessType_RDD);
     }
   }
 
   gfxVars::RemoveReceiver(this);
   mHost->OnChannelClosed();
 }
 
 class DeferredDeleteRDDChild : public Runnable {
--- a/dom/plugins/ipc/PluginModuleParent.cpp
+++ b/dom/plugins/ipc/PluginModuleParent.cpp
@@ -658,17 +658,17 @@ PluginModuleChromeParent::~PluginModuleC
     delete mHangUIParent;
     mHangUIParent = nullptr;
   }
 #endif
 
   mozilla::BackgroundHangMonitor::UnregisterAnnotator(*this);
 }
 
-void PluginModuleChromeParent::WriteExtraDataForMinidump() {
+void PluginModuleChromeParent::AddCrashAnnotations() {
   // mCrashReporterMutex is already held by the caller
   mCrashReporterMutex.AssertCurrentThreadOwns();
 
   typedef nsDependentCString cstring;
 
   // Get the plugin filename, try to get just the file leafname
   const std::string& pluginFile = mSubprocess->GetPluginFilePath();
   size_t filePos = pluginFile.rfind(FILE_PATH_SEPARATOR);
@@ -1274,19 +1274,23 @@ static void RemoveMinidump(nsIFile* mini
     extraFile->Remove(true);
   }
 }
 #endif  // MOZ_CRASHREPORTER_INJECTOR
 
 void PluginModuleChromeParent::ProcessFirstMinidump() {
   mozilla::MutexAutoLock lock(mCrashReporterMutex);
 
-  if (!mCrashReporter) return;
+  if (!mCrashReporter) {
+    CrashReporter::FinalizeOrphanedMinidump(OtherPid(),
+                                            GeckoProcessType_Plugin);
+    return;
+  }
 
-  WriteExtraDataForMinidump();
+  AddCrashAnnotations();
 
   if (mCrashReporter->HasMinidump()) {
     // A minidump may be set in TerminateChildProcess, which means the
     // process hang monitor has already collected a 3-way browser, plugin,
     // content crash report. If so, update the existing report with our
     // annotations and finalize it. If not, fall through for standard
     // plugin crash report handling.
     mCrashReporter->FinalizeCrashReport();
--- a/dom/plugins/ipc/PluginModuleParent.h
+++ b/dom/plugins/ipc/PluginModuleParent.h
@@ -415,17 +415,17 @@ class PluginModuleChromeParent : public 
   PluginInstanceParent* GetManagingInstance(mozilla::ipc::IProtocol* aProtocol);
 
   virtual void AnnotateHang(
       mozilla::BackgroundHangAnnotations& aAnnotations) override;
 
   virtual bool ShouldContinueFromReplyTimeout() override;
 
   void ProcessFirstMinidump();
-  void WriteExtraDataForMinidump();
+  void AddCrashAnnotations();
 
   PluginProcessParent* Process() const { return mSubprocess; }
   base::ProcessHandle ChildProcessHandle() {
     return mSubprocess->GetChildProcessHandle();
   }
 
 #if defined(XP_UNIX) && !defined(XP_MACOSX)
   virtual nsresult NP_Initialize(NPNetscapeFuncs* bFuncs, NPPluginFuncs* pFuncs,
--- a/gfx/ipc/GPUChild.cpp
+++ b/gfx/ipc/GPUChild.cpp
@@ -248,16 +248,18 @@ mozilla::ipc::IPCResult GPUChild::RecvFi
   return IPC_OK();
 }
 
 void GPUChild::ActorDestroy(ActorDestroyReason aWhy) {
   if (aWhy == AbnormalShutdown) {
     if (mCrashReporter) {
       mCrashReporter->GenerateCrashReport(OtherPid());
       mCrashReporter = nullptr;
+    } else {
+      CrashReporter::FinalizeOrphanedMinidump(OtherPid(), GeckoProcessType_GPU);
     }
 
     Telemetry::Accumulate(
         Telemetry::SUBPROCESS_ABNORMAL_ABORT,
         nsDependentCString(XRE_ChildProcessTypeToString(GeckoProcessType_GPU)),
         1);
 
     // Notify the Telemetry environment so that we can refresh and do a
--- a/gfx/vr/ipc/VRChild.cpp
+++ b/gfx/vr/ipc/VRChild.cpp
@@ -89,16 +89,18 @@ mozilla::ipc::IPCResult VRChild::RecvFin
   return IPC_OK();
 }
 
 void VRChild::ActorDestroy(ActorDestroyReason aWhy) {
   if (aWhy == AbnormalShutdown) {
     if (mCrashReporter) {
       mCrashReporter->GenerateCrashReport(OtherPid());
       mCrashReporter = nullptr;
+    } else {
+      CrashReporter::FinalizeOrphanedMinidump(OtherPid(), GeckoProcessType_VR);
     }
 
     Telemetry::Accumulate(
         Telemetry::SUBPROCESS_ABNORMAL_ABORT,
         nsDependentCString(XRE_ChildProcessTypeToString(GeckoProcessType_VR)),
         1);
   }
   gfxVars::RemoveReceiver(this);
@@ -205,9 +207,9 @@ class DeferredDeleteVRChild : public Run
 };
 
 /* static */
 void VRChild::Destroy(UniquePtr<VRChild>&& aChild) {
   NS_DispatchToMainThread(new DeferredDeleteVRChild(std::move(aChild)));
 }
 
 }  // namespace gfx
-}  // namespace mozilla
\ No newline at end of file
+}  // namespace mozilla
--- a/ipc/glue/CrashReporterHost.cpp
+++ b/ipc/glue/CrashReporterHost.cpp
@@ -113,39 +113,18 @@ int32_t CrashReporterHost::GetCrashType(
 }
 
 bool CrashReporterHost::FinalizeCrashReport() {
   MOZ_ASSERT(!mFinalized);
   MOZ_ASSERT(HasMinidump());
 
   CrashReporter::AnnotationTable annotations;
 
-  nsAutoCString type;
-  // The gecko media plugin and normal plugin processes are lumped together
-  // as a historical artifact.
-  if (mProcessType == GeckoProcessType_GMPlugin) {
-    type.AssignLiteral("plugin");
-  } else if (mProcessType == GeckoProcessType_Content) {
-    type.AssignLiteral("content");
-  } else {
-    // This check will pick up some cases that will never happen (e.g. IPDL
-    // unit tests), but that's OK.
-    switch (mProcessType) {
-#define GECKO_PROCESS_TYPE(enum_name, string_name, xre_name, bin_type) \
-  case GeckoProcessType_##enum_name:                                   \
-    type.AssignLiteral(string_name);                                   \
-    break;
-#include "mozilla/GeckoProcessTypes.h"
-#undef GECKO_PROCESS_TYPE
-      default:
-        NS_ERROR("unknown process type");
-        break;
-    }
-  }
-  annotations[CrashReporter::Annotation::ProcessType] = type;
+  annotations[CrashReporter::Annotation::ProcessType] =
+      XRE_ChildProcessTypeToAnnotation(mProcessType);
 
   char startTime[32];
   SprintfLiteral(startTime, "%lld", static_cast<long long>(mStartTime));
   annotations[CrashReporter::Annotation::StartupTime] =
       nsDependentCString(startTime);
 
   // We might not have shmem (for example, when running crashreporter tests).
   if (mShmem.IsReadable()) {
--- a/netwerk/ipc/SocketProcessParent.cpp
+++ b/netwerk/ipc/SocketProcessParent.cpp
@@ -51,16 +51,19 @@ mozilla::ipc::IPCResult SocketProcessPar
   return IPC_OK();
 }
 
 void SocketProcessParent::ActorDestroy(ActorDestroyReason aWhy) {
   if (aWhy == AbnormalShutdown) {
     if (mCrashReporter) {
       mCrashReporter->GenerateCrashReport(OtherPid());
       mCrashReporter = nullptr;
+    } else {
+      CrashReporter::FinalizeOrphanedMinidump(OtherPid(),
+                                              GeckoProcessType_Content);
     }
   }
 
   if (mHost) {
     mHost->OnChannelClosed();
   }
 }
 
--- a/security/sandbox/win/src/remotesandboxbroker/RemoteSandboxBrokerParent.cpp
+++ b/security/sandbox/win/src/remotesandboxbroker/RemoteSandboxBrokerParent.cpp
@@ -59,16 +59,19 @@ bool RemoteSandboxBrokerParent::Duplicat
 
 void RemoteSandboxBrokerParent::ActorDestroy(ActorDestroyReason aWhy) {
   if (AbnormalShutdown == aWhy) {
     Telemetry::Accumulate(Telemetry::SUBPROCESS_ABNORMAL_ABORT,
                           NS_LITERAL_CSTRING("sandboxbroker"), 1);
     if (mCrashReporter) {
       mCrashReporter->GenerateCrashReport(OtherPid());
       mCrashReporter = nullptr;
+    } else {
+      CrashReporter::FinalizeOrphanedMinidump(
+          OtherPid(), GeckoProcessType_RemoteSandboxBroker);
     }
   }
   Shutdown();
 }
 
 void RemoteSandboxBrokerParent::Shutdown() {
   if (mOpened) {
     mOpened = false;
--- a/toolkit/crashreporter/nsDummyExceptionHandler.cpp
+++ b/toolkit/crashreporter/nsDummyExceptionHandler.cpp
@@ -199,16 +199,20 @@ bool CreateNotificationPipeForChild(int*
 bool SetRemoteExceptionHandler() { return false; }
 #endif  // XP_WIN
 
 bool TakeMinidumpForChild(uint32_t childPid, nsIFile** dump,
                           AnnotationTable& aAnnotations, uint32_t* aSequence) {
   return false;
 }
 
+bool FinalizeOrphanedMinidump(uint32_t aChildPid, GeckoProcessType aType) {
+  return false;
+}
+
 ThreadId CurrentThreadId() { return -1; }
 
 bool TakeMinidump(nsIFile** aResult, bool aMoveToPending) { return false; }
 
 bool CreateMinidumpsAndPair(ProcessHandle aTargetPid,
                             ThreadId aTargetBlamedThread,
                             const nsACString& aIncomingPairName,
                             nsIFile* aIncomingDumpToPair,
--- a/toolkit/crashreporter/nsExceptionHandler.cpp
+++ b/toolkit/crashreporter/nsExceptionHandler.cpp
@@ -21,17 +21,16 @@
 #include "mozilla/Printf.h"
 #include "mozilla/Sprintf.h"
 #include "mozilla/StaticMutex.h"
 #include "mozilla/SyncRunnable.h"
 #include "mozilla/TimeStamp.h"
 #include "mozilla/ipc/CrashReporterClient.h"
 
 #include "nsThreadUtils.h"
-#include "nsXULAppAPI.h"
 #include "jsfriendapi.h"
 #include "ThreadAnnotation.h"
 #include "private/pprio.h"
 
 #if defined(XP_WIN)
 #  ifdef WIN32_LEAN_AND_MEAN
 #    undef WIN32_LEAN_AND_MEAN
 #  endif
@@ -3262,16 +3261,35 @@ bool TakeMinidumpForChild(uint32_t child
     *aSequence = pd->sequence;
   }
 
   pidToMinidump->RemoveEntry(pd);
 
   return !!*dump;
 }
 
+bool FinalizeOrphanedMinidump(uint32_t aChildPid, GeckoProcessType aType) {
+  AnnotationTable annotations;
+  nsCOMPtr<nsIFile> minidump;
+
+  if (!TakeMinidumpForChild(aChildPid, getter_AddRefs(minidump), annotations)) {
+    return false;
+  }
+
+  nsAutoString id;
+  if (!GetIDFromMinidump(minidump, id)) {
+    return false;
+  }
+
+  annotations[Annotation::ProcessType] =
+      XRE_ChildProcessTypeToAnnotation(aType);
+
+  return WriteExtraFile(id, annotations);
+}
+
 //-----------------------------------------------------------------------------
 // CreatePairedMinidumps() and helpers
 //
 
 /*
  * Renames the stand alone dump file aDumpFile to:
  *  |aOwnerDumpFile-aDumpFileProcessType.dmp|
  * and moves it into the same directory as aOwnerDumpFile. Does not
--- a/toolkit/crashreporter/nsExceptionHandler.h
+++ b/toolkit/crashreporter/nsExceptionHandler.h
@@ -16,16 +16,17 @@
 #include "mozilla/EnumeratedArray.h"
 
 #include "CrashAnnotations.h"
 
 #include <stddef.h>
 #include <stdint.h>
 #include "nsError.h"
 #include "nsString.h"
+#include "nsXULAppAPI.h"
 #include "prio.h"
 
 #if defined(XP_WIN)
 #  ifdef WIN32_LEAN_AND_MEAN
 #    undef WIN32_LEAN_AND_MEAN
 #  endif
 #  include <windows.h>
 #endif
@@ -167,16 +168,28 @@ bool TakeMinidump(nsIFile** aResult, boo
 // path in |dump|.  The caller owns the last reference to |dump| if it
 // is non-nullptr. The annotations for the crash will be stored in
 // |aAnnotations|. The sequence parameter will be filled with an ordinal
 // indicating which remote process crashed first.
 bool TakeMinidumpForChild(uint32_t childPid, nsIFile** dump,
                           AnnotationTable& aAnnotations,
                           uint32_t* aSequence = nullptr);
 
+/**
+ * If a dump was found for |childPid| then write a minimal .extra file to
+ * complete it and remove it from the list of pending crash dumps. It's
+ * required to call this method after a non-main process crash if the crash
+ * report could not be finalized via the CrashReporterHost (for example because
+ * it wasn't instanced yet).
+ *
+ * @param aChildPid The pid of the crashed child process
+ * @param aType The type of the crashed process
+ */
+bool FinalizeOrphanedMinidump(uint32_t aChildPid, GeckoProcessType aType);
+
 #if defined(XP_WIN)
 typedef HANDLE ProcessHandle;
 typedef DWORD ProcessId;
 typedef DWORD ThreadId;
 typedef HANDLE FileHandle;
 #elif defined(XP_MACOSX)
 typedef task_t ProcessHandle;
 typedef pid_t ProcessId;
--- a/toolkit/xre/nsEmbedFunctions.cpp
+++ b/toolkit/xre/nsEmbedFunctions.cpp
@@ -225,16 +225,31 @@ void XRE_TermEmbedding() {
 }
 
 const char* XRE_ChildProcessTypeToString(GeckoProcessType aProcessType) {
   return (aProcessType < GeckoProcessType_End)
              ? kGeckoProcessTypeString[aProcessType]
              : "invalid";
 }
 
+const char* XRE_ChildProcessTypeToAnnotation(GeckoProcessType aProcessType) {
+  switch (aProcessType) {
+    case GeckoProcessType_GMPlugin:
+      // The gecko media plugin and normal plugin processes are lumped together
+      // as a historical artifact.
+      return "plugin";
+    case GeckoProcessType_Default:
+      return "";
+    case GeckoProcessType_Content:
+      return "content";
+    default:
+      return XRE_ChildProcessTypeToString(aProcessType);
+  }
+}
+
 namespace mozilla {
 namespace startup {
 GeckoProcessType sChildProcessType = GeckoProcessType_Default;
 }  // namespace startup
 }  // namespace mozilla
 
 #if defined(MOZ_WIDGET_ANDROID)
 void XRE_SetAndroidChildFds(JNIEnv* env, const XRE_AndroidChildFds& fds) {
--- a/xpcom/build/nsXULAppAPI.h
+++ b/xpcom/build/nsXULAppAPI.h
@@ -380,16 +380,18 @@ static const char* const kGeckoProcessTy
 #undef GECKO_PROCESS_TYPE
 };
 
 static_assert(MOZ_ARRAY_LENGTH(kGeckoProcessTypeString) == GeckoProcessType_End,
               "Array length mismatch");
 
 XRE_API(const char*, XRE_ChildProcessTypeToString,
         (GeckoProcessType aProcessType))
+XRE_API(const char*, XRE_ChildProcessTypeToAnnotation,
+        (GeckoProcessType aProcessType))
 
 #if defined(MOZ_WIDGET_ANDROID)
 struct XRE_AndroidChildFds {
   int mPrefsFd;
   int mPrefMapFd;
   int mIpcFd;
   int mCrashFd;
   int mCrashAnnotationFd;