Bug 1386760 - Add a crash annotation containing the last executable we launched r=froydnj

Differential Revision: https://phabricator.services.mozilla.com/D12643

--HG--
extra : moz-landing-system : lando

Author: gabrielesvelto

toolkit/crashreporter/CrashAnnotations.yaml

@@ -268,6 +268,14 @@ EventLoopNestingLevel:
   type: integer
   ping: true
 
+ExecutableName:
+  description: >
+    The name of the last executable that was launched via the nsIProcess class.
+    Multiple executables can be launched at the same time from one or more
+    threads so this annotation might not always contain the correct value. To
+    be removed once bug 1386760 is fixed.
+  type: string
+
 ExpectedStreamLen:
   description: >
     Expected length of an IPC proxy stream.

xpcom/threads/nsProcess.h

@@ -46,6 +46,7 @@ class nsProcess final : public nsIProcess, public nsIObserver {
  private:
   ~nsProcess();
   static void Monitor(void* aArg);
+  static void RemoveExecutableCrashAnnotation();
   void ProcessComplete();
   nsresult CopyArgsAndRunProcess(bool aBlocking, const char** aArgs,
                                  uint32_t aCount, nsIObserver* aObserver,
@@ -56,6 +57,7 @@ class nsProcess final : public nsIProcess, public nsIObserver {
   // The 'args' array is null-terminated.
   nsresult RunProcess(bool aBlocking, char** aArgs, nsIObserver* aObserver,
                       bool aHoldWeak, bool aArgsUTF8);
+  void AddExecutableCrashAnnotation();
 
   PRThread* mThread;
   mozilla::Mutex mLock;

xpcom/threads/nsProcessCommon.cpp

@@ -15,6 +15,7 @@
 #include "mozilla/ArrayUtils.h"
 
 #include "nsCOMPtr.h"
+#include "nsExceptionHandler.h"
 #include "nsAutoPtr.h"
 #include "nsMemory.h"
 #include "nsProcess.h"
@@ -245,19 +246,7 @@ void nsProcess::Monitor(void* aArg) {
       exitCode = -1;
     }
   }
-
-  // Lock in case Kill or GetExitCode are called during this
-  {
-    MutexAutoLock lock(process->mLock);
-    CloseHandle(process->mProcess);
-    process->mProcess = nullptr;
-    process->mExitValue = exitCode;
-    if (process->mShutdown) {
-      return;
-    }
-  }
-#else
-#ifdef XP_UNIX
+#elif defined(XP_UNIX)
   int exitCode = -1;
   int status = 0;
   pid_t result;
@@ -278,9 +267,15 @@ void nsProcess::Monitor(void* aArg) {
   }
 #endif
 
+  // The application has finished executing once we reach this point
+  RemoveExecutableCrashAnnotation();
+
   // Lock in case Kill or GetExitCode are called during this
   {
     MutexAutoLock lock(process->mLock);
+#if defined(PROCESSMODEL_WINAPI)
+    CloseHandle(process->mProcess);
+#endif
 #if !defined(XP_UNIX)
     process->mProcess = nullptr;
 #endif
@@ -289,7 +284,6 @@ void nsProcess::Monitor(void* aArg) {
       return;
     }
   }
-#endif
 
   // If we ran a background thread for the monitor then notify on the main
   // thread
@@ -547,6 +541,8 @@ nsresult nsProcess::RunProcess(bool aBlocking, char** aMyArgv,
   mPid = ptrProc->pid;
 #endif
 
+  AddExecutableCrashAnnotation();
+
   NS_ADDREF_THIS();
   mBlocking = aBlocking;
   if (aBlocking) {
@@ -584,6 +580,32 @@ nsProcess::GetIsRunning(bool* aIsRunning) {
   return NS_OK;
 }
 
+void nsProcess::AddExecutableCrashAnnotation() {
+#if defined(XP_WIN)
+  nsAutoCString executableName;
+  if (NS_FAILED(mExecutable->GetNativeLeafName(executableName))) {
+    return;
+  }
+
+  // The following executables might be launched during shutdown and lead to a
+  // shutdown hang. We're adding this annotation to try and detect which is the
+  // culprit of bug 1386760
+  if (executableName.EqualsLiteral("minidump-analyzer.exe") ||
+      executableName.EqualsLiteral("pingsender.exe") ||
+      executableName.EqualsLiteral("updater.exe")) {
+    CrashReporter::AnnotateCrashReport(
+        CrashReporter::Annotation::ExecutableName, executableName.get());
+  }
+#endif  // defined(XP_WIN)
+}
+
+/* static */ void nsProcess::RemoveExecutableCrashAnnotation() {
+#if defined(XP_WIN)
+  CrashReporter::RemoveCrashReportAnnotation(
+      CrashReporter::Annotation::ExecutableName);
+#endif  // defined(XP_WIN)
+}
+
 NS_IMETHODIMP
 nsProcess::GetStartHidden(bool* aStartHidden) {
   *aStartHidden = mStartHidden;