Bug 1406971 - Change nsProcess to use LaunchApp from IPC, instead of NSPR, on Unix. r=froydnj
authorJed Davis <jld@mozilla.com>
Fri, 06 Oct 2017 19:58:33 -0600
changeset 385990 e0e042fe8d40bdf4a382b16f5cbe758c6228fdb4
parent 385989 e006f8a22781efbe46486a3072f620de9ce428e1
child 385991 f290517125dc8b2e64ebd0dc0a76fe60bd33c966
push id53228
push userjedavis@mozilla.com
push dateFri, 13 Oct 2017 01:45:22 +0000
treeherderautoland@e0e042fe8d40 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1406971, 678369, 147659, 227246
milestone58.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 1406971 - Change nsProcess to use LaunchApp from IPC, instead of NSPR, on Unix. r=froydnj This also fixes the Unix part of bug 678369, and fixes bug 147659 as a convenient side-effect of using LaunchApp (which has the desired fd-closing behavior already) rather than directly using fork/exec. The main goal is to work around bug 227246, where PR_CreateProcess on Unix interferes with anything else that tries to use child processes. This does not fix that bug -- NSPR will still cause problems if used in that way -- but it's an adequate workaround for that bug in Gecko in almost all cases (the one known exception is nsAuthSambaNTLM, which uses NSPR directly and needs to be fixed separately). Waiting for the child process uses waitpid() directly, sharing the existing code used for OS X. MozReview-Commit-ID: 6zfZ1Zgk2i9
xpcom/threads/nsProcess.h
xpcom/threads/nsProcessCommon.cpp
--- a/xpcom/threads/nsProcess.h
+++ b/xpcom/threads/nsProcess.h
@@ -15,17 +15,17 @@
 #include "mozilla/Mutex.h"
 #include "nsIProcess.h"
 #include "nsIFile.h"
 #include "nsIThread.h"
 #include "nsIObserver.h"
 #include "nsIWeakReferenceUtils.h"
 #include "nsIObserver.h"
 #include "nsString.h"
-#ifndef XP_MACOSX
+#ifndef XP_UNIX
 #include "prproces.h"
 #endif
 #if defined(PROCESSMODEL_WINAPI)
 #include <windows.h>
 #include <shellapi.h>
 #endif
 
 #define NS_PROCESS_CID \
@@ -71,14 +71,14 @@ private:
   nsCOMPtr<nsIObserver> mObserver;
   nsWeakPtr mWeakObserver;
 
   // These members are modified by multiple threads, any accesses should be
   // protected with mLock.
   int32_t mExitValue;
 #if defined(PROCESSMODEL_WINAPI)
   HANDLE mProcess;
-#elif !defined(XP_MACOSX)
+#elif !defined(XP_UNIX)
   PRProcess* mProcess;
 #endif
 };
 
 #endif
--- a/xpcom/threads/nsProcessCommon.cpp
+++ b/xpcom/threads/nsProcessCommon.cpp
@@ -33,16 +33,21 @@
 #include "nsString.h"
 #include "nsLiteralString.h"
 #include "nsReadableUtils.h"
 #include "mozilla/UniquePtrExtensions.h"
 #else
 #ifdef XP_MACOSX
 #include <crt_externs.h>
 #include <spawn.h>
+#endif
+#ifdef XP_UNIX
+#ifndef XP_MACOSX
+#include "base/process_util.h"
+#endif
 #include <sys/wait.h>
 #include <sys/errno.h>
 #endif
 #include <sys/types.h>
 #include <signal.h>
 #endif
 
 using namespace mozilla;
@@ -73,17 +78,17 @@ nsProcess::nsProcess()
   , mShutdown(false)
   , mBlocking(false)
   , mStartHidden(false)
   , mNoShell(false)
   , mPid(-1)
   , mObserver(nullptr)
   , mWeakObserver(nullptr)
   , mExitValue(-1)
-#if !defined(XP_MACOSX)
+#if !defined(XP_UNIX)
   , mProcess(nullptr)
 #endif
 {
 }
 
 //Destructor
 nsProcess::~nsProcess()
 {
@@ -267,17 +272,17 @@ nsProcess::Monitor(void* aArg)
     CloseHandle(process->mProcess);
     process->mProcess = nullptr;
     process->mExitValue = exitCode;
     if (process->mShutdown) {
       return;
     }
   }
 #else
-#ifdef XP_MACOSX
+#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)) {
@@ -291,17 +296,17 @@ nsProcess::Monitor(void* aArg)
   if (PR_WaitProcess(process->mProcess, &exitCode) != PR_SUCCESS) {
     exitCode = -1;
   }
 #endif
 
   // Lock in case Kill or GetExitCode are called during this
   {
     MutexAutoLock lock(process->mLock);
-#if !defined(XP_MACOSX)
+#if !defined(XP_UNIX)
     process->mProcess = nullptr;
 #endif
     process->mExitValue = exitCode;
     if (process->mShutdown) {
       return;
     }
   }
 #endif
@@ -565,16 +570,30 @@ nsProcess::RunProcess(bool aBlocking, ch
                             *_NSGetEnviron());
   mPid = static_cast<int32_t>(newPid);
 
   posix_spawnattr_destroy(&spawnattr);
 
   if (result != 0) {
     return NS_ERROR_FAILURE;
   }
+#elif defined(XP_UNIX)
+  base::file_handle_mapping_vector fdMap;
+  std::vector<std::string> argvVec;
+  for (char** arg = aMyArgv; *arg != nullptr; ++arg) {
+    argvVec.push_back(*arg);
+  }
+  pid_t newPid;
+  if (base::LaunchApp(argvVec, fdMap, false, &newPid)) {
+    static_assert(sizeof(pid_t) <= sizeof(int32_t),
+                  "mPid is large enough to hold a pid");
+    mPid = static_cast<int32_t>(newPid);
+  } else {
+    return NS_ERROR_FAILURE;
+  }
 #else
   mProcess = PR_CreateProcess(aMyArgv[0], aMyArgv, nullptr, nullptr);
   if (!mProcess) {
     return NS_ERROR_FAILURE;
   }
   struct MYProcess
   {
     uint32_t pid;
@@ -671,17 +690,17 @@ nsProcess::Kill()
   }
 
   {
     MutexAutoLock lock(mLock);
 #if defined(PROCESSMODEL_WINAPI)
     if (TerminateProcess(mProcess, 0) == 0) {
       return NS_ERROR_FAILURE;
     }
-#elif defined(XP_MACOSX)
+#elif defined(XP_UNIX)
     if (kill(mPid, SIGKILL) != 0) {
       return NS_ERROR_FAILURE;
     }
 #else
     if (!mProcess || (PR_KillProcess(mProcess) != PR_SUCCESS)) {
       return NS_ERROR_FAILURE;
     }
 #endif