Bug 592951: Use 'posix_spawnp' to launch child processes on Mac OS X. r=ted a=blocking-b6+

ipc/app/MozillaRuntimeMain.cpp

@@ -54,37 +54,17 @@
 int
 main(int argc, char* argv[])
 {
-#if defined(MOZ_CRASHREPORTER)
-    if (argc < 2)
-        return 1;
-    const char* const crashReporterArg = argv[--argc];
-
-#  if defined(XP_WIN) || defined(XP_MACOSX)
-    // on windows and mac, |crashReporterArg| is the named pipe on which the
-    // server is listening for requests, or "-" if crash reporting is
-    // disabled.
-    if (0 != strcmp("-", crashReporterArg)
-        && !XRE_SetRemoteExceptionHandler(crashReporterArg))
-        return 1;
-#  elif defined(OS_LINUX)
-    // on POSIX, |crashReporterArg| is "true" if crash reporting is
-    // enabled, false otherwise
-    if (0 != strcmp("false", crashReporterArg)
-        && !XRE_SetRemoteExceptionHandler(NULL))
-        return 1;
-#  else
-#    error "OOP crash reporting unsupported on this platform"
-#  endif   
-#endif // if defined(MOZ_CRASHREPORTER)
-
 #if defined(XP_WIN) && defined(DEBUG_bent)
     MessageBox(NULL, L"Hi", L"Hi", MB_OK);
 #endif
 
-    GeckoProcessType proctype =
-        XRE_StringToChildProcessType(argv[argc - 1]);
+    // Check for the absolute minimum number of args we need to move
+    // forward here. We expect the last arg to be the child process type.
+    if (argc < 1)
+      return 1;
+    GeckoProcessType proctype = XRE_StringToChildProcessType(argv[--argc]);
 
-    nsresult rv = XRE_InitChildProcess(argc - 1, argv, proctype);
+    nsresult rv = XRE_InitChildProcess(argc, argv, proctype);
     NS_ENSURE_SUCCESS(rv, 1);
 
     return 0;

ipc/chromium/src/base/process_util.h

@@ -131,36 +131,17 @@ bool LaunchApp(const std::wstring& cmdline,
 //
 // Note that the first argument in argv must point to the filename,
 // and must be fully specified.
-#ifdef OS_MACOSX
-typedef std::vector<std::pair<int, int> > file_handle_mapping_vector;
-bool LaunchApp(const std::vector<std::string>& argv,
-               const file_handle_mapping_vector& fds_to_remap,
-               bool wait, ProcessHandle* process_handle,
-               task_t* task_handle);
-
-#if defined(OS_LINUX) || defined(OS_MACOSX)
-typedef std::map<std::string, std::string> environment_map;
-bool LaunchApp(const std::vector<std::string>& argv,
-               const file_handle_mapping_vector& fds_to_remap,
-               const environment_map& env_vars_to_set,
-               bool wait, ProcessHandle* process_handle,
-               task_t* task_handle);
-#endif
-#else // !OS_MACOSX
 typedef std::vector<std::pair<int, int> > file_handle_mapping_vector;
 bool LaunchApp(const std::vector<std::string>& argv,
                const file_handle_mapping_vector& fds_to_remap,
                bool wait, ProcessHandle* process_handle);
 
-#if defined(OS_LINUX) || defined(OS_MACOSX)
 typedef std::map<std::string, std::string> environment_map;
 bool LaunchApp(const std::vector<std::string>& argv,
                const file_handle_mapping_vector& fds_to_remap,
                const environment_map& env_vars_to_set,
                bool wait, ProcessHandle* process_handle);
 #endif
-#endif
-#endif
 
 // Executes the application specified by cl. This function delegates to one
 // of the above two platform-specific functions.

ipc/chromium/src/base/process_util_mac.mm

@@ -17,192 +17,114 @@
 #include "base/eintr_wrapper.h"
 #include "base/logging.h"
 #include "base/rand_util.h"
-#include "base/scoped_ptr.h"
 #include "base/string_util.h"
 #include "base/time.h"
-#include "chrome/common/mach_ipc_mac.h"
 
 namespace base {
 
-static std::string MachErrorCode(kern_return_t err) {
-  return StringPrintf("0x%x %s", err, mach_error_string(err));
-}
-
-// Forks the current process and returns the child's |task_t| in the parent
-// process.
-static pid_t fork_and_get_task(task_t* child_task) {
-  const int kTimeoutMs = 100;
-  kern_return_t err;
-
-  // Put a random number into the channel name, so that a compromised renderer
-  // can't pretend being the child that's forked off.
-  std::string mach_connection_name = StringPrintf(
-      "org.mozilla.samplingfork.%p.%d",
-      child_task, base::RandInt(0, std::numeric_limits<int>::max()));
-  ReceivePort parent_recv_port(mach_connection_name.c_str());
-
-  // Error handling philosophy: If Mach IPC fails, don't touch |child_task| but
-  // return a valid pid. If IPC fails in the child, the parent will have to wait
-  // until kTimeoutMs is over. This is not optimal, but I've never seen it
-  // happen, and stuff should still mostly work.
-  pid_t pid = fork();
-  switch (pid) {
-    case -1:
-      return pid;
-    case 0: {  // child
-      ReceivePort child_recv_port;
-
-      MachSendMessage child_message(/* id= */0);
-      if (!child_message.AddDescriptor(mach_task_self())) {
-        LOG(ERROR) << "child AddDescriptor(mach_task_self()) failed.";
-        return pid;
-      }
-      mach_port_t raw_child_recv_port = child_recv_port.GetPort();
-      if (!child_message.AddDescriptor(raw_child_recv_port)) {
-        LOG(ERROR) << "child AddDescriptor(" << raw_child_recv_port
-                   << ") failed.";
-        return pid;
-      }
-
-      MachPortSender child_sender(mach_connection_name.c_str());
-      err = child_sender.SendMessage(child_message, kTimeoutMs);
-      if (err != KERN_SUCCESS) {
-        LOG(ERROR) << "child SendMessage() failed: " << MachErrorCode(err);
-        return pid;
-      }
-
-      MachReceiveMessage parent_message;
-      err = child_recv_port.WaitForMessage(&parent_message, kTimeoutMs);
-      if (err != KERN_SUCCESS) {
-        LOG(ERROR) << "child WaitForMessage() failed: " << MachErrorCode(err);
-        return pid;
-      }
-
-      if (parent_message.GetTranslatedPort(0) == MACH_PORT_NULL) {
-        LOG(ERROR) << "child GetTranslatedPort(0) failed.";
-        return pid;
-      }
-      err = task_set_bootstrap_port(mach_task_self(),
-                                    parent_message.GetTranslatedPort(0));
-      if (err != KERN_SUCCESS) {
-        LOG(ERROR) << "child task_set_bootstrap_port() failed: "
-                   << MachErrorCode(err);
-        return pid;
-      }
-      break;
-    }
-    default: {  // parent
-      MachReceiveMessage child_message;
-      err = parent_recv_port.WaitForMessage(&child_message, kTimeoutMs);
-      if (err != KERN_SUCCESS) {
-        LOG(ERROR) << "parent WaitForMessage() failed: " << MachErrorCode(err);
-        return pid;
-      }
-
-      if (child_message.GetTranslatedPort(0) == MACH_PORT_NULL) {
-        LOG(ERROR) << "parent GetTranslatedPort(0) failed.";
-        return pid;
-      }
-      *child_task = child_message.GetTranslatedPort(0);
-
-      if (child_message.GetTranslatedPort(1) == MACH_PORT_NULL) {
-        LOG(ERROR) << "parent GetTranslatedPort(1) failed.";
-        return pid;
-      }
-      MachPortSender parent_sender(child_message.GetTranslatedPort(1));
-
-      MachSendMessage parent_message(/* id= */0);
-      if (!parent_message.AddDescriptor(bootstrap_port)) {
-        LOG(ERROR) << "parent AddDescriptor(" << bootstrap_port << ") failed.";
-        return pid;
-      }
-
-      err = parent_sender.SendMessage(parent_message, kTimeoutMs);
-      if (err != KERN_SUCCESS) {
-        LOG(ERROR) << "parent SendMessage() failed: " << MachErrorCode(err);
-        return pid;
-      }
-      break;
-    }
-  }
-  return pid;
+bool LaunchApp(const std::vector<std::string>& argv,
+               const file_handle_mapping_vector& fds_to_remap,
+               bool wait, ProcessHandle* process_handle) {
+  return LaunchApp(argv, fds_to_remap, environment_map(),
+                   wait, process_handle);
 }
 
 bool LaunchApp(const std::vector<std::string>& argv,
                const file_handle_mapping_vector& fds_to_remap,
-               bool wait, ProcessHandle* process_handle,
-               task_t* process_task) {
-  return LaunchApp(argv, fds_to_remap, environment_map(),
-                   wait, process_handle, process_task);
-}
+               const environment_map& env_vars_to_set,
+               bool wait, ProcessHandle* process_handle) {
+  bool retval = true;
 
-bool LaunchApp(
-    const std::vector<std::string>& argv,
-    const file_handle_mapping_vector& fds_to_remap,
-    const environment_map& environ,
-    bool wait,
-    ProcessHandle* process_handle,
-    task_t* task_handle) {
-  pid_t pid;
+  char* argv_copy[argv.size() + 1];
+  for (size_t i = 0; i < argv.size(); i++) {
+    argv_copy[i] = const_cast<char*>(argv[i].c_str());
+  }
+  argv_copy[argv.size()] = NULL;
 
-  if (task_handle == NULL) {
-    pid = fork();
-  } else {
-    // On OS X, the task_t for a process is needed for several reasons. Sadly,
-    // the function task_for_pid() requires privileges a normal user doesn't
-    // have. Instead, a short-lived Mach IPC connection is opened between parent
-    // and child, and the child sends its task_t to the parent at fork time.
-    *task_handle = MACH_PORT_NULL;
-    pid = fork_and_get_task(task_handle);
+  // Make sure we don't leak any FDs to the child process by marking all FDs
+  // as close-on-exec.
+  SetAllFDsToCloseOnExec();
+
+  // Copy _NSGetEnviron() to a new char array and add the variables
+  // in env_vars_to_set.
+  // Existing variables are overwritten by env_vars_to_set.
+  int pos = 0;
+  environment_map combined_env_vars = env_vars_to_set;
+  while((*_NSGetEnviron())[pos] != NULL) {
+    std::string varString = (*_NSGetEnviron())[pos];
+    std::string varName = varString.substr(0, varString.find_first_of('='));
+    std::string varValue = varString.substr(varString.find_first_of('=') + 1);
+    if (combined_env_vars.find(varName) == combined_env_vars.end()) {
+      combined_env_vars[varName] = varValue;
+    }
+    pos++;
+  }
+  int varsLen = combined_env_vars.size() + 1;
+
+  char** vars = new char*[varsLen];
+  int i = 0;
+  for (environment_map::const_iterator it = combined_env_vars.begin();
+       it != combined_env_vars.end(); ++it) {
+    std::string entry(it->first);
+    entry += "=";
+    entry += it->second;
+    vars[i] = strdup(entry.c_str());
+    i++;
+  }
+  vars[i] = NULL;
+
+  posix_spawn_file_actions_t file_actions;
+  if (posix_spawn_file_actions_init(&file_actions) != 0) {
+    for(int j = 0; j < varsLen; j++) {
+      free(vars[j]);
+    }  
+    delete[] vars;
+    return false;
   }
 
-  if (pid < 0)
-    return false;
+  // Turn fds_to_remap array into a set of dup2 calls.
+  for (file_handle_mapping_vector::const_iterator it = fds_to_remap.begin();
+       it != fds_to_remap.end();
+       ++it) {
+    int src_fd = it->first;
+    int dest_fd = it->second;
 
-  if (pid == 0) {
-    // Child process
-
-    InjectiveMultimap fd_shuffle;
-    for (file_handle_mapping_vector::const_iterator
-        it = fds_to_remap.begin(); it != fds_to_remap.end(); ++it) {
-      fd_shuffle.push_back(InjectionArc(it->first, it->second, false));
-    }
-
-    for (environment_map::const_iterator it = environ.begin();
-         it != environ.end(); ++it) {
-      if (it->first.empty())
-        continue;
-
-      if (it->second.empty()) {
-        unsetenv(it->first.c_str());
-      } else {
-        setenv(it->first.c_str(), it->second.c_str(), 1);
+    if (src_fd == dest_fd) {
+      int flags = fcntl(src_fd, F_GETFD);
+      if (flags != -1) {
+        fcntl(src_fd, F_SETFD, flags & ~FD_CLOEXEC);
+      }
+    } else {
+      if (posix_spawn_file_actions_adddup2(&file_actions, src_fd, dest_fd) != 0) {
+        posix_spawn_file_actions_destroy(&file_actions);
+        for(int j = 0; j < varsLen; j++) {
+          free(vars[j]);
+        }  
+        delete[] vars;
+        return false;
       }
     }
+  }
 
-    // Obscure fork() rule: in the child, if you don't end up doing exec*(),
-    // you call _exit() instead of exit(). This is because _exit() does not
-    // call any previously-registered (in the parent) exit handlers, which
-    // might do things like block waiting for threads that don't even exist
-    // in the child.
-    if (!ShuffleFileDescriptors(fd_shuffle))
-      _exit(127);
+  int pid = 0;
+  int spawn_succeeded = (posix_spawnp(&pid,
+                                      argv_copy[0],
+                                      &file_actions,
+                                      NULL,
+                                      argv_copy,
+                                      vars) == 0);
 
-    // If we are using the SUID sandbox, it sets a magic environment variable
-    // ("SBX_D"), so we remove that variable from the environment here on the
-    // off chance that it's already set.
-    unsetenv("SBX_D");
+  for(int j = 0; j < varsLen; j++) {
+    free(vars[j]);
+  }  
+  delete[] vars;
 
-    CloseSuperfluousFds(fd_shuffle);
+  posix_spawn_file_actions_destroy(&file_actions);
 
-    scoped_array<char*> argv_cstr(new char*[argv.size() + 1]);
-    for (size_t i = 0; i < argv.size(); i++)
-      argv_cstr[i] = const_cast<char*>(argv[i].c_str());
-    argv_cstr[argv.size()] = NULL;
-    execvp(argv_cstr[0], argv_cstr.get());
-    _exit(127);
+  bool process_handle_valid = pid > 0;
+  if (!spawn_succeeded || !process_handle_valid) {
+    retval = false;
   } else {
-    // Parent process
     if (wait)
       HANDLE_EINTR(waitpid(pid, 0, 0));
 
@@ -210,14 +132,14 @@ bool LaunchApp(
       *process_handle = pid;
   }
 
-  return true;
+  return retval;
 }
 
 bool LaunchApp(const CommandLine& cl,
                bool wait, bool start_hidden, ProcessHandle* process_handle) {
   // TODO(playmobil): Do we need to respect the start_hidden flag?
   file_handle_mapping_vector no_files;
-  return LaunchApp(cl.argv(), no_files, wait, process_handle, NULL);
+  return LaunchApp(cl.argv(), no_files, wait, process_handle);
 }
 
 NamedProcessIterator::NamedProcessIterator(const std::wstring& executable_name,

ipc/glue/GeckoChildProcessHost.cpp

@@ -43,6 +43,10 @@
 #include "base/string_util.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/process_watcher.h"
+#ifdef XP_MACOSX
+#include "chrome/common/mach_ipc_mac.h"
+#include "base/rand_util.h"
+#endif
 
 #include "prprf.h"
 
@@ -224,9 +228,6 @@ GeckoChildProcessHost::PerformAsyncLaunch(std::vector<std::string> aExtraOpts)
   }
 
   base::ProcessHandle process;
-#if defined(XP_MACOSX)
-  task_t child_task;
-#endif
 
   // send the child the PID so that it can open a ProcessHandle back to us.
   // probably don't want to do this in the long run
@@ -311,7 +312,6 @@ GeckoChildProcessHost::PerformAsyncLaunch(std::vector<std::string> aExtraOpts)
 #endif
 
   childArgv.push_back(pidstring);
-  childArgv.push_back(childProcessType);
 
 #if defined(MOZ_CRASHREPORTER)
 #  if defined(OS_LINUX)
@@ -333,15 +333,63 @@ GeckoChildProcessHost::PerformAsyncLaunch(std::vector<std::string> aExtraOpts)
 #  endif  // OS_LINUX
 #endif
 
+#ifdef XP_MACOSX
+  // Add a mach port to the command line so the child can communicate its
+  // 'task_t' back to the parent.
+  //
+  // Put a random number into the channel name, so that a compromised renderer
+  // can't pretend being the child that's forked off.
+  std::string mach_connection_name = StringPrintf("org.mozilla.machname.%d",
+                                                  base::RandInt(0, std::numeric_limits<int>::max()));
+  childArgv.push_back(mach_connection_name.c_str());
+#endif
+
+  childArgv.push_back(childProcessType);
+
   base::LaunchApp(childArgv, mFileMap,
 #if defined(OS_LINUX) || defined(OS_MACOSX)
                   newEnvVars,
 #endif
-                  false, &process
-#if defined(XP_MACOSX)
-                  , &child_task
+                  false, &process);
+
+#ifdef XP_MACOSX
+  // Wait for the child process to send us its 'task_t' data.
+  const int kTimeoutMs = 1000;
+
+  MachReceiveMessage child_message;
+  ReceivePort parent_recv_port(mach_connection_name.c_str());
+  kern_return_t err = parent_recv_port.WaitForMessage(&child_message, kTimeoutMs);
+  if (err != KERN_SUCCESS) {
+    std::string errString = StringPrintf("0x%x %s", err, mach_error_string(err));
+    LOG(ERROR) << "parent WaitForMessage() failed: " << errString;
+    return false;
+  }
+
+  task_t child_task = child_message.GetTranslatedPort(0);
+  if (child_task == MACH_PORT_NULL) {
+    LOG(ERROR) << "parent GetTranslatedPort(0) failed.";
+    return false;
+  }
+
+  if (child_message.GetTranslatedPort(1) == MACH_PORT_NULL) {
+    LOG(ERROR) << "parent GetTranslatedPort(1) failed.";
+    return false;
+  }
+  MachPortSender parent_sender(child_message.GetTranslatedPort(1));
+
+  MachSendMessage parent_message(/* id= */0);
+  if (!parent_message.AddDescriptor(bootstrap_port)) {
+    LOG(ERROR) << "parent AddDescriptor(" << bootstrap_port << ") failed.";
+    return false;
+  }
+
+  err = parent_sender.SendMessage(parent_message, kTimeoutMs);
+  if (err != KERN_SUCCESS) {
+    std::string errString = StringPrintf("0x%x %s", err, mach_error_string(err));
+    LOG(ERROR) << "parent SendMessage() failed: " << errString;
+    return false;
+  }
 #endif
-                  );
 
 //--------------------------------------------------
 #elif defined(OS_WIN)
@@ -375,12 +423,14 @@ GeckoChildProcessHost::PerformAsyncLaunch(std::vector<std::string> aExtraOpts)
 #endif
 
   cmdLine.AppendLooseValue(UTF8ToWide(pidstring));
-  cmdLine.AppendLooseValue(UTF8ToWide(childProcessType));
+
 #if defined(MOZ_CRASHREPORTER)
   cmdLine.AppendLooseValue(
     UTF8ToWide(CrashReporter::GetChildNotificationPipe()));
 #endif
 
+  cmdLine.AppendLooseValue(UTF8ToWide(childProcessType));
+
   base::LaunchApp(cmdLine, false, false, &process);
 
 #else

toolkit/xre/nsEmbedFunctions.cpp

@@ -75,6 +75,9 @@
 
 #include "mozilla/Omnijar.h"
 #ifdef MOZ_IPC
+#if defined(XP_MACOSX)
+#include "chrome/common/mach_ipc_mac.h"
+#endif
 #include "nsX11ErrorHandler.h"
 #include "base/at_exit.h"
 #include "base/command_line.h"
@@ -309,6 +312,77 @@ XRE_InitChildProcess(int aArgc,
 
   sChildProcessType = aProcess;
 
+  // Complete 'task_t' exchange for Mac OS X. This structure has the same size
+  // regardless of architecture so we don't have any cross-arch issues here.
+#ifdef XP_MACOSX
+  if (aArgc < 1)
+    return 1;
+  const char* const mach_port_name = aArgv[--aArgc];
+
+  const int kTimeoutMs = 1000;
+
+  MachSendMessage child_message(0);
+  if (!child_message.AddDescriptor(mach_task_self())) {
+    NS_WARNING("child AddDescriptor(mach_task_self()) failed.");
+    return 1;
+  }
+
+  ReceivePort child_recv_port;
+  mach_port_t raw_child_recv_port = child_recv_port.GetPort();
+  if (!child_message.AddDescriptor(raw_child_recv_port)) {
+    NS_WARNING("Adding descriptor to message failed");
+    return 1;
+  }
+
+  MachPortSender child_sender(mach_port_name);
+  kern_return_t err = child_sender.SendMessage(child_message, kTimeoutMs);
+  if (err != KERN_SUCCESS) {
+    NS_WARNING("child SendMessage() failed");
+    return 1;
+  }
+
+  MachReceiveMessage parent_message;
+  err = child_recv_port.WaitForMessage(&parent_message, kTimeoutMs);
+  if (err != KERN_SUCCESS) {
+    NS_WARNING("child WaitForMessage() failed");
+    return 1;
+  }
+
+  if (parent_message.GetTranslatedPort(0) == MACH_PORT_NULL) {
+    NS_WARNING("child GetTranslatedPort(0) failed");
+    return 1;
+  }
+  err = task_set_bootstrap_port(mach_task_self(),
+                                parent_message.GetTranslatedPort(0));
+  if (err != KERN_SUCCESS) {
+    NS_WARNING("child task_set_bootstrap_port() failed");
+    return 1;
+  }
+#endif
+  
+#if defined(MOZ_CRASHREPORTER)
+  if (aArgc < 1)
+    return 1;
+  const char* const crashReporterArg = aArgv[--aArgc];
+  
+#  if defined(XP_WIN) || defined(XP_MACOSX)
+  // on windows and mac, |crashReporterArg| is the named pipe on which the
+  // server is listening for requests, or "-" if crash reporting is
+  // disabled.
+  if (0 != strcmp("-", crashReporterArg)
+      && !XRE_SetRemoteExceptionHandler(crashReporterArg))
+    return 1;
+#  elif defined(OS_LINUX)
+  // on POSIX, |crashReporterArg| is "true" if crash reporting is
+  // enabled, false otherwise
+  if (0 != strcmp("false", crashReporterArg)
+      && !XRE_SetRemoteExceptionHandler(NULL))
+    return 1;
+#  else
+#    error "OOP crash reporting unsupported on this platform"
+#  endif   
+#endif // if defined(MOZ_CRASHREPORTER)
+
   gArgv = aArgv;
   gArgc = aArgc;