Bug 1386760 - Remove the ExecutableName annotation r=froydnj
authorGabriele Svelto <gsvelto@mozilla.com>
Wed, 17 Apr 2019 14:05:42 +0000
changeset 469962 f0b7027aa979309df34458f56ac4f4039905e5f9
parent 469961 a55e5c83e32eb40abe53f427a74a45b1f461ecc1
child 469963 f5273f8e51966ff5d45588bfcd1826a5642ba8b4
push id83456
push usergsvelto@mozilla.com
push dateWed, 17 Apr 2019 23:22:46 +0000
treeherderautoland@f0b7027aa979 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1386760
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1386760 - Remove the ExecutableName annotation r=froydnj Differential Revision: https://phabricator.services.mozilla.com/D27885
toolkit/crashreporter/CrashAnnotations.yaml
xpcom/threads/nsProcess.h
xpcom/threads/nsProcessCommon.cpp
--- a/toolkit/crashreporter/CrashAnnotations.yaml
+++ b/toolkit/crashreporter/CrashAnnotations.yaml
@@ -268,24 +268,16 @@ EMCheckCompatibility:
 
 EventLoopNestingLevel:
   description: >
     Present only if higher than 0, indicates that we're running in a nested
     event loop and indicates the nesting level.
   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.
   type: integer
 
 FlashProcessDump:
   description: >
     Type of process the flash plugin is running in, can be either "Broker" or
--- a/xpcom/threads/nsProcess.h
+++ b/xpcom/threads/nsProcess.h
@@ -41,28 +41,26 @@ class nsProcess final : public nsIProces
   NS_DECL_NSIPROCESS
   NS_DECL_NSIOBSERVER
 
   nsProcess();
 
  private:
   ~nsProcess();
   static void Monitor(void* aArg);
-  static void RemoveExecutableCrashAnnotation();
   void ProcessComplete();
   nsresult CopyArgsAndRunProcess(bool aBlocking, const char** aArgs,
                                  uint32_t aCount, nsIObserver* aObserver,
                                  bool aHoldWeak);
   nsresult CopyArgsAndRunProcessw(bool aBlocking, const char16_t** aArgs,
                                   uint32_t aCount, nsIObserver* aObserver,
                                   bool aHoldWeak);
   // 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;
   bool mShutdown;
   bool mBlocking;
   bool mStartHidden;
   bool mNoShell;
 
--- a/xpcom/threads/nsProcessCommon.cpp
+++ b/xpcom/threads/nsProcessCommon.cpp
@@ -10,17 +10,16 @@
  * wait (blocking) or continue (non-blocking).
  *
  *****************************************************************************
  */
 
 #include "mozilla/ArrayUtils.h"
 
 #include "nsCOMPtr.h"
-#include "nsExceptionHandler.h"
 #include "nsAutoPtr.h"
 #include "nsMemory.h"
 #include "nsProcess.h"
 #include "prio.h"
 #include "prenv.h"
 #include "nsCRT.h"
 #include "nsThreadUtils.h"
 #include "nsIObserverService.h"
@@ -241,54 +240,61 @@ void nsProcess::Monitor(void* aArg) {
   unsigned long exitCode = -1;
 
   dwRetVal = WaitForSingleObject(process->mProcess, INFINITE);
   if (dwRetVal != WAIT_FAILED) {
     if (GetExitCodeProcess(process->mProcess, &exitCode) == FALSE) {
       exitCode = -1;
     }
   }
-#elif defined(XP_UNIX)
+
+  // 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
   int exitCode = -1;
   int status = 0;
   pid_t result;
   do {
     result = waitpid(process->mPid, &status, 0);
   } while (result == -1 && errno == EINTR);
   if (result == process->mPid) {
     if (WIFEXITED(status)) {
       exitCode = WEXITSTATUS(status);
     } else if (WIFSIGNALED(status)) {
       exitCode = 256;  // match NSPR's signal exit status
     }
   }
-#else
+#  else
   int32_t exitCode = -1;
   if (PR_WaitProcess(process->mProcess, &exitCode) != PR_SUCCESS) {
     exitCode = -1;
   }
-#endif
-
-  // The application has finished executing once we reach this point
-  RemoveExecutableCrashAnnotation();
+#  endif
 
   // 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)
+#  if !defined(XP_UNIX)
     process->mProcess = nullptr;
-#endif
+#  endif
     process->mExitValue = exitCode;
     if (process->mShutdown) {
       return;
     }
   }
+#endif
 
   // If we ran a background thread for the monitor then notify on the main
   // thread
   if (NS_IsMainThread()) {
     process->ProcessComplete();
   } else {
     NS_DispatchToMainThread(NewRunnableMethod(
         "nsProcess::ProcessComplete", process, &nsProcess::ProcessComplete));
@@ -536,18 +542,16 @@ nsresult nsProcess::RunProcess(bool aBlo
   }
   struct MYProcess {
     uint32_t pid;
   };
   MYProcess* ptrProc = (MYProcess*)mProcess;
   mPid = ptrProc->pid;
 #endif
 
-  AddExecutableCrashAnnotation();
-
   NS_ADDREF_THIS();
   mBlocking = aBlocking;
   if (aBlocking) {
     Monitor(this);
     if (mExitValue < 0) {
       return NS_ERROR_FILE_EXECUTION_FAILED;
     }
   } else {
@@ -575,43 +579,16 @@ nsProcess::GetIsRunning(bool* aIsRunning
     *aIsRunning = true;
   } else {
     *aIsRunning = false;
   }
 
   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;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsProcess::SetStartHidden(bool aStartHidden) {