Bug 1406971 - Change nsProcess to use LaunchApp from IPC, instead of NSPR, on Unix. r=froydnj draft
authorJed Davis <jld@mozilla.com>
Fri, 06 Oct 2017 19:58:33 -0600
changeset 679799 4c9ee289a8acc9dcb239cec1d4f4b35262b6fba6
parent 679629 25aad10380b10b6efa50c2b4d97245f078d870a0
child 735676 388ff7972c8c2fac802a76e99a7d04aa05ca41d4
push id84306
push userbmo:jld@mozilla.com
push dateFri, 13 Oct 2017 01:40:01 +0000
reviewersfroydnj
bugs1406971, 678369, 147659, 227246
milestone58.0a1
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