Bug 600711: Use 'posix_spawnp' for nsIProcess on Mac OS X. Prefer the current architecture when creating a new process. r=bsmedberg a=blocking-beta7
authorJosh Aas <joshmoz@gmail.com>
Fri, 01 Oct 2010 09:01:04 -0400
changeset 54831 e089470f19df60e58dc80fc718a3b122748cbae4
parent 54830 dabe4204f1c110a61aaf6d0f55d752ecdf600f8c
child 54869 e3dcde25c792558ef0e17fbcd35aa9f1187fbc5a
push idunknown
push userunknown
push dateunknown
reviewersbsmedberg, blocking-beta7
bugs600711
milestone2.0b7pre
Bug 600711: Use 'posix_spawnp' for nsIProcess on Mac OS X. Prefer the current architecture when creating a new process. r=bsmedberg a=blocking-beta7
xpcom/threads/nsProcess.h
xpcom/threads/nsProcessCommon.cpp
--- a/xpcom/threads/nsProcess.h
+++ b/xpcom/threads/nsProcess.h
@@ -47,18 +47,20 @@
 #include "nsIProcess.h"
 #include "nsIFile.h"
 #include "nsIThread.h"
 #include "nsIObserver.h"
 #include "nsIWeakReference.h"
 #include "nsIWeakReferenceUtils.h"
 #include "nsIObserver.h"
 #include "nsString.h"
+#ifndef XP_MACOSX
 #include "prproces.h"
-#if defined(PROCESSMODEL_WINAPI) 
+#endif
+#if defined(PROCESSMODEL_WINAPI)
 #include <windows.h>
 #include <shellapi.h>
 #endif
 
 #define NS_PROCESS_CID \
 {0x7b4eeb20, 0xd781, 0x11d4, \
    {0x8A, 0x83, 0x00, 0x10, 0xa4, 0xe0, 0xc9, 0xca}}
 
@@ -72,39 +74,40 @@ public:
   NS_DECL_NSIOBSERVER
 
   nsProcess();
 
 private:
   ~nsProcess();
   static void PR_CALLBACK Monitor(void *arg);
   void ProcessComplete();
-  NS_IMETHOD CopyArgsAndRunProcess(PRBool blocking, const char** args,
-                                   PRUint32 count, nsIObserver* observer,
-                                   PRBool holdWeak);
-  NS_IMETHOD CopyArgsAndRunProcessw(PRBool blocking, const PRUnichar** args,
-                                    PRUint32 count, nsIObserver* observer,
-                                    PRBool holdWeak);
-  NS_IMETHOD RunProcess(PRBool blocking, char **args, PRUint32 count,
-                        nsIObserver* observer, PRBool holdWeak, PRBool argsUTF8);
+  nsresult CopyArgsAndRunProcess(PRBool blocking, const char** args,
+                                 PRUint32 count, nsIObserver* observer,
+                                 PRBool holdWeak);
+  nsresult CopyArgsAndRunProcessw(PRBool blocking, const PRUnichar** args,
+                                  PRUint32 count, nsIObserver* observer,
+                                  PRBool holdWeak);
+  // The 'args' array is null-terminated.
+  nsresult RunProcess(PRBool blocking, char **args, nsIObserver* observer,
+                      PRBool holdWeak, PRBool argsUTF8);
 
   PRThread* mThread;
   PRLock* mLock;
   PRBool mShutdown;
 
   nsCOMPtr<nsIFile> mExecutable;
   nsString mTargetPath;
   PRInt32 mPid;
   nsCOMPtr<nsIObserver> mObserver;
   nsWeakPtr mWeakObserver;
 
   // These members are modified by multiple threads, any accesses should be
   // protected with mLock.
   PRInt32 mExitValue;
-#if defined(PROCESSMODEL_WINAPI) 
+#if defined(PROCESSMODEL_WINAPI)
   typedef DWORD (WINAPI*GetProcessIdPtr)(HANDLE process);
   HANDLE mProcess;
-#else
+#elif !defined(XP_MACOSX)
   PRProcess *mProcess;
 #endif
 };
 
 #endif
--- a/xpcom/threads/nsProcessCommon.cpp
+++ b/xpcom/threads/nsProcessCommon.cpp
@@ -61,45 +61,63 @@
 #include <stdlib.h>
 
 #if defined(PROCESSMODEL_WINAPI)
 #include "prmem.h"
 #include "nsString.h"
 #include "nsLiteralString.h"
 #include "nsReadableUtils.h"
 #else
+#ifdef XP_MACOSX
+#include <spawn.h>
+#include <sys/wait.h>
+#endif
 #include <sys/types.h>
 #include <signal.h>
 #endif
 
 #ifdef WINCE
 #include <windows.h> // for MultiByteToWideChar
 #include "prmem.h"
 #define SHELLEXECUTEINFOW SHELLEXECUTEINFO
 #define SEE_MASK_FLAG_DDEWAIT 0
 #define SEE_MASK_NO_CONSOLE 0
 #define ShellExecuteExW ShellExecuteEx
 #endif
 
+#ifdef XP_MACOSX
+cpu_type_t pref_cpu_types[2] = {
+#if defined(__i386__)
+                                 CPU_TYPE_X86,
+#elif defined(__x86_64__)
+                                 CPU_TYPE_X86_64,
+#elif defined(__ppc__)
+                                 CPU_TYPE_POWERPC,
+#endif
+                                 CPU_TYPE_ANY };
+#endif
+
 //-------------------------------------------------------------------//
 // nsIProcess implementation
 //-------------------------------------------------------------------//
 NS_IMPL_THREADSAFE_ISUPPORTS2(nsProcess, nsIProcess,
                                          nsIObserver)
 
 //Constructor
 nsProcess::nsProcess()
-    : mThread(nsnull),
-      mLock(PR_NewLock()),
-      mShutdown(PR_FALSE),
-      mPid(-1),
-      mObserver(nsnull),
-      mWeakObserver(nsnull),
-      mExitValue(-1),
-      mProcess(nsnull)
+    : mThread(nsnull)
+    , mLock(PR_NewLock())
+    , mShutdown(PR_FALSE)
+    , mPid(-1)
+    , mObserver(nsnull)
+    , mWeakObserver(nsnull)
+    , mExitValue(-1)
+#if !defined(XP_MACOSX)
+    , mProcess(nsnull)
+#endif
 {
 }
 
 //Destructor
 nsProcess::~nsProcess()
 {
     PR_DestroyLock(mLock);
 }
@@ -260,24 +278,33 @@ void PR_CALLBACK nsProcess::Monitor(void
         nsAutoLock lock(process->mLock);
         CloseHandle(process->mProcess);
         process->mProcess = NULL;
         process->mExitValue = exitCode;
         if (process->mShutdown)
             return;
     }
 #else
+#ifdef XP_MACOSX
+    int exitCode = -1;
+    int status = 0;
+    if (waitpid(process->mPid, &status, 0) == process->mPid && WIFEXITED(status))
+        exitCode = WEXITSTATUS(status);
+#else
     PRInt32 exitCode = -1;
     if (PR_WaitProcess(process->mProcess, &exitCode) != PR_SUCCESS)
         exitCode = -1;
+#endif
 
     // Lock in case Kill or GetExitCode are called during this
     {
         nsAutoLock lock(process->mLock);
+#if !defined(XP_MACOSX)
         process->mProcess = nsnull;
+#endif
         process->mExitValue = exitCode;
         if (process->mShutdown)
             return;
     }
 #endif
 
     // If we ran a background thread for the monitor then notify on the main
     // thread
@@ -331,43 +358,37 @@ nsProcess::Run(PRBool blocking, const ch
 // XXXldb |args| has the wrong const-ness
 NS_IMETHODIMP  
 nsProcess::RunAsync(const char **args, PRUint32 count,
                     nsIObserver* observer, PRBool holdWeak)
 {
     return CopyArgsAndRunProcess(PR_FALSE, args, count, observer, holdWeak);
 }
 
-NS_IMETHODIMP
+nsresult
 nsProcess::CopyArgsAndRunProcess(PRBool blocking, const char** args,
                                  PRUint32 count, nsIObserver* observer,
                                  PRBool holdWeak)
 {
-    // make sure that when we allocate we have 1 greater than the
-    // count since we need to null terminate the list for the argv to
-    // pass into PR_CreateProcess
+    // Add one to the count for the program name and one for NULL termination.
     char **my_argv = NULL;
-    my_argv = (char **)NS_Alloc(sizeof(char *) * (count + 2) );
+    my_argv = (char**)NS_Alloc(sizeof(char*) * (count + 2));
     if (!my_argv) {
         return NS_ERROR_OUT_OF_MEMORY;
     }
 
-    // copy the args
-    PRUint32 i;
-    for (i=0; i < count; i++) {
-        my_argv[i+1] = const_cast<char*>(args[i]);
-        printf("arg[%d] = %s\n", i, my_argv[i+1]);
+    my_argv[0] = ToNewUTF8String(mTargetPath);
+
+    for (PRUint32 i = 0; i < count; i++) {
+        my_argv[i + 1] = const_cast<char*>(args[i]);
     }
-    // we need to set argv[0] to the program name.
-    my_argv[0] = ToNewUTF8String(mTargetPath);
-    // null terminate the array
-    my_argv[count+1] = NULL;
 
-    nsresult rv =
-        RunProcess(blocking, my_argv, count, observer, holdWeak, PR_FALSE);
+    my_argv[count + 1] = NULL;
+
+    nsresult rv = RunProcess(blocking, my_argv, observer, holdWeak, PR_FALSE);
 
     NS_Free(my_argv[0]);
     NS_Free(my_argv);
     return rv;
 }
 
 // XXXldb |args| has the wrong const-ness
 NS_IMETHODIMP  
@@ -379,53 +400,48 @@ nsProcess::Runw(PRBool blocking, const P
 // XXXldb |args| has the wrong const-ness
 NS_IMETHODIMP  
 nsProcess::RunwAsync(const PRUnichar **args, PRUint32 count,
                     nsIObserver* observer, PRBool holdWeak)
 {
     return CopyArgsAndRunProcessw(PR_FALSE, args, count, observer, holdWeak);
 }
 
-NS_IMETHODIMP
+nsresult
 nsProcess::CopyArgsAndRunProcessw(PRBool blocking, const PRUnichar** args,
                                   PRUint32 count, nsIObserver* observer,
                                   PRBool holdWeak)
 {
-    // make sure that when we allocate we have 1 greater than the
-    // count since we need to null terminate the list for the argv to
-    // pass into PR_CreateProcess
+    // Add one to the count for the program name and one for NULL termination.
     char **my_argv = NULL;
-    my_argv = (char **)NS_Alloc(sizeof(char *) * (count + 2) );
+    my_argv = (char**)NS_Alloc(sizeof(char*) * (count + 2));
     if (!my_argv) {
         return NS_ERROR_OUT_OF_MEMORY;
     }
 
-    // copy the args
-    PRUint32 i;
-    for (i=0; i < count; i++) {
-        my_argv[i+1] = ToNewUTF8String(nsDependentString(args[i]));
+    my_argv[0] = ToNewUTF8String(mTargetPath);
+
+    for (PRUint32 i = 0; i < count; i++) {
+        my_argv[i + 1] = ToNewUTF8String(nsDependentString(args[i]));
     }
-    // we need to set argv[0] to the program name.
-    my_argv[0] = ToNewUTF8String(mTargetPath);
-    // null terminate the array
-    my_argv[count+1] = NULL;
+
+    my_argv[count + 1] = NULL;
 
-    nsresult rv =
-        RunProcess(blocking, my_argv, count, observer, holdWeak, PR_TRUE);
+    nsresult rv = RunProcess(blocking, my_argv, observer, holdWeak, PR_TRUE);
 
-    for (i=0; i <= count; ++i) {
+    for (PRUint32 i = 0; i <= count; i++) {
         NS_Free(my_argv[i]);
     }
     NS_Free(my_argv);
     return rv;
 }
 
-NS_IMETHODIMP  
-nsProcess::RunProcess(PRBool blocking, char **my_argv, PRUint32 count,
-                      nsIObserver* observer, PRBool holdWeak, PRBool argsUTF8)
+nsresult  
+nsProcess::RunProcess(PRBool blocking, char **my_argv, nsIObserver* observer,
+                      PRBool holdWeak, PRBool argsUTF8)
 {
     NS_ENSURE_TRUE(mExecutable, NS_ERROR_NOT_INITIALIZED);
     NS_ENSURE_FALSE(mThread, NS_ERROR_ALREADY_INITIALIZED);
 
     if (observer) {
         if (holdWeak) {
             mWeakObserver = do_GetWeakReference(observer);
             if (!mWeakObserver)
@@ -436,20 +452,22 @@ nsProcess::RunProcess(PRBool blocking, c
         }
     }
 
     mExitValue = -1;
     mPid = -1;
 
 #if defined(PROCESSMODEL_WINAPI)
     BOOL retVal;
-    PRUnichar *cmdLine;
+    PRUnichar *cmdLine = NULL;
 
-    if (count > 0 && assembleCmdLine(my_argv + 1, &cmdLine, 
-                                     argsUTF8 ? CP_UTF8 : CP_ACP) == -1) {
+    // The 'argv' array is null-terminated and always starts with the program path.
+    // If the second slot is non-null then arguments are being passed.
+    if (my_argv[1] != NULL &&
+        assembleCmdLine(my_argv + 1, &cmdLine, argsUTF8 ? CP_UTF8 : CP_ACP) == -1) {
         return NS_ERROR_FILE_EXECUTION_FAILED;    
     }
 
     /* The SEE_MASK_NO_CONSOLE flag is important to prevent console windows
      * from appearing. This makes behavior the same on all platforms. The flag
      * will not have any effect on non-console applications.
      */
 
@@ -463,44 +481,67 @@ nsProcess::RunProcess(PRBool blocking, c
     sinfo.cbSize = sizeof(SHELLEXECUTEINFOW);
     sinfo.hwnd   = NULL;
     sinfo.lpFile = wideFile;
     sinfo.nShow  = SW_SHOWNORMAL;
     sinfo.fMask  = SEE_MASK_FLAG_DDEWAIT |
                    SEE_MASK_NO_CONSOLE |
                    SEE_MASK_NOCLOSEPROCESS;
 
-    if (count > 0)
+    if (cmdLine)
         sinfo.lpParameters = cmdLine;
 
     retVal = ShellExecuteExW(&sinfo);
     if (!retVal) {
         return NS_ERROR_FILE_EXECUTION_FAILED;
     }
 
     mProcess = sinfo.hProcess;
 
     PR_Free(wideFile);
-    if (count > 0)
-        PR_Free( cmdLine );
+    if (cmdLine)
+        PR_Free(cmdLine);
 
     HMODULE kernelDLL = ::LoadLibraryW(L"kernel32.dll");
     if (kernelDLL) {
         GetProcessIdPtr getProcessId = (GetProcessIdPtr)GetProcAddress(kernelDLL, "GetProcessId");
         if (getProcessId)
             mPid = getProcessId(mProcess);
         FreeLibrary(kernelDLL);
     }
+#elif defined(XP_MACOSX)
+    // Initialize spawn attributes.
+    posix_spawnattr_t spawnattr;
+    if (posix_spawnattr_init(&spawnattr) != 0) {
+        return NS_ERROR_FAILURE;
+    }
 
-#else // Note, this must not be an #elif ...!
-    
+    // Set spawn attributes.
+    size_t attr_count = NS_ARRAY_LENGTH(pref_cpu_types);
+    size_t attr_ocount = 0;
+    if (posix_spawnattr_setbinpref_np(&spawnattr, attr_count, pref_cpu_types, &attr_ocount) != 0 ||
+        attr_ocount != attr_count) {
+        posix_spawnattr_destroy(&spawnattr);
+        return NS_ERROR_FAILURE;
+    }
+
+    // Note that the 'argv' array is already null-terminated, which 'posix_spawnp' requires.
+    pid_t newPid = 0;
+    int result = posix_spawnp(&newPid, my_argv[0], NULL, &spawnattr, my_argv, NULL);
+    mPid = static_cast<PRInt32>(newPid);
+
+    posix_spawnattr_destroy(&spawnattr);
+
+    if (result != 0) {
+        return NS_ERROR_FAILURE;
+    }
+#else
     mProcess = PR_CreateProcess(my_argv[0], my_argv, NULL, NULL);
     if (!mProcess)
         return NS_ERROR_FAILURE;
-
 #if !defined WINCE
     struct MYProcess {
         PRUint32 pid;
     };
     MYProcess* ptrProc = (MYProcess *) mProcess;
     mPid = ptrProc->pid;
 #endif
 #endif
@@ -557,16 +598,19 @@ nsProcess::Kill()
     if (!mThread)
         return NS_ERROR_FAILURE;
 
     {
         nsAutoLock lock(mLock);
 #if defined(PROCESSMODEL_WINAPI)
         if (TerminateProcess(mProcess, NULL) == 0)
             return NS_ERROR_FAILURE;
+#elif defined(XP_MACOSX)
+        if (kill(mPid, SIGKILL) != 0)
+            return NS_ERROR_FAILURE;
 #else
         if (!mProcess || (PR_KillProcess(mProcess) != PR_SUCCESS))
             return NS_ERROR_FAILURE;
 #endif
     }
 
     // We must null out mThread if we want IsRunning to return false immediately
     // after this call.