Use posix_spawn() instead of fork()/exec().
authorNico Weber <nicolasweber@gmx.de>
Mon, 21 Mar 2016 01:41:15 +0000 (21:41 -0400)
committerNico Weber <nicolasweber@gmx.de>
Mon, 21 Mar 2016 01:42:17 +0000 (21:42 -0400)
posix_spawn() is a syscall on OS X and Solaris and a bit faster.  It's
also easier emulate for cygwin, and the code is a bit simpler.

src/subprocess-posix.cc
src/subprocess_test.cc

index ae7ae6f..5ffe85b 100644 (file)
@@ -22,6 +22,9 @@
 #include <stdio.h>
 #include <string.h>
 #include <sys/wait.h>
+#include <spawn.h>
+
+extern char** environ;
 
 #include "util.h"
 
@@ -50,62 +53,60 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) {
 #endif  // !USE_PPOLL
   SetCloseOnExec(fd_);
 
-  pid_ = fork();
-  if (pid_ < 0)
-    Fatal("fork: %s", strerror(errno));
-
-  if (pid_ == 0) {
-    close(output_pipe[0]);
-
-    // Track which fd we use to report errors on.
-    int error_pipe = output_pipe[1];
-    do {
-      if (sigaction(SIGINT, &set->old_int_act_, 0) < 0)
-        break;
-      if (sigaction(SIGTERM, &set->old_term_act_, 0) < 0)
-        break;
-      if (sigaction(SIGHUP, &set->old_hup_act_, 0) < 0)
-        break;
-      if (sigprocmask(SIG_SETMASK, &set->old_mask_, 0) < 0)
-        break;
-
-      if (!use_console_) {
-        // Put the child in its own process group, so ctrl-c won't reach it.
-        if (setpgid(0, 0) < 0)
-          break;
-
-        // Open /dev/null over stdin.
-        int devnull = open("/dev/null", O_RDONLY);
-        if (devnull < 0)
-          break;
-        if (dup2(devnull, 0) < 0)
-          break;
-        close(devnull);
-
-        if (dup2(output_pipe[1], 1) < 0 ||
-            dup2(output_pipe[1], 2) < 0)
-          break;
-
-        // Now can use stderr for errors.
-        error_pipe = 2;
-        close(output_pipe[1]);
-      }
-      // In the console case, output_pipe is still inherited by the child and
-      // closed when the subprocess finishes, which then notifies ninja.
-
-      execl("/bin/sh", "/bin/sh", "-c", command.c_str(), (char *) NULL);
-    } while (false);
-
-    // If we get here, something went wrong; the execl should have
-    // replaced us.
-    char* err = strerror(errno);
-    if (write(error_pipe, err, strlen(err)) < 0) {
-      // If the write fails, there's nothing we can do.
-      // But this block seems necessary to silence the warning.
+  posix_spawn_file_actions_t action;
+  if (posix_spawn_file_actions_init(&action) != 0)
+    Fatal("posix_spawn_file_actions_init: %s", strerror(errno));
+
+  if (posix_spawn_file_actions_addclose(&action, output_pipe[0]) != 0)
+    Fatal("posix_spawn_file_actions_addclose: %s", strerror(errno));
+
+  posix_spawnattr_t attr;
+  if (posix_spawnattr_init(&attr) != 0)
+    Fatal("posix_spawnattr_init: %s", strerror(errno));
+
+  short flags = 0;
+
+  flags |= POSIX_SPAWN_SETSIGMASK;
+  if (posix_spawnattr_setsigmask(&attr, &set->old_mask_) != 0)
+    Fatal("posix_spawnattr_setsigmask: %s", strerror(errno));
+  // Signals which are set to be caught in the calling process image are set to
+  // default action in the new process image, so no explicit
+  // POSIX_SPAWN_SETSIGDEF parameter is needed.
+
+  // TODO: Consider using POSIX_SPAWN_USEVFORK on Linux with glibc?
+
+  if (!use_console_) {
+    // Put the child in its own process group, so ctrl-c won't reach it.
+    flags |= POSIX_SPAWN_SETPGROUP;
+    // No need to posix_spawnattr_setpgroup(&attr, 0), it's the default.
+
+    // Open /dev/null over stdin.
+    if (posix_spawn_file_actions_addopen(&action, 0, "/dev/null", O_RDONLY,
+                                         0) != 0) {
+      Fatal("posix_spawn_file_actions_addopen: %s", strerror(errno));
     }
-    _exit(1);
+
+    if (posix_spawn_file_actions_adddup2(&action, output_pipe[1], 1) != 0)
+      Fatal("posix_spawn_file_actions_adddup2: %s", strerror(errno));
+    if (posix_spawn_file_actions_adddup2(&action, output_pipe[1], 2) != 0)
+      Fatal("posix_spawn_file_actions_adddup2: %s", strerror(errno));
+    // In the console case, output_pipe is still inherited by the child and
+    // closed when the subprocess finishes, which then notifies ninja.
   }
 
+  if (posix_spawnattr_setflags(&attr, flags) != 0)
+    Fatal("posix_spawnattr_setflags: %s", strerror(errno));
+
+  const char* spawned_args[] = { "/bin/sh", "-c", command.c_str(), NULL };
+  if (posix_spawn(&pid_, "/bin/sh", &action, &attr,
+                  const_cast<char**>(spawned_args), environ) != 0)
+    Fatal("posix_spawn: %s", strerror(errno));
+
+  if (posix_spawnattr_destroy(&attr) != 0)
+    Fatal("posix_spawnattr_destroy: %s", strerror(errno));
+  if (posix_spawn_file_actions_destroy(&action) != 0)
+    Fatal("posix_spawn_file_actions_destroy: %s", strerror(errno));
+
   close(output_pipe[1]);
   return true;
 }
index c8f2fb8..ee16190 100644 (file)
@@ -226,7 +226,8 @@ TEST_F(SubprocessTest, SetWithLots) {
   rlimit rlim;
   ASSERT_EQ(0, getrlimit(RLIMIT_NOFILE, &rlim));
   if (rlim.rlim_cur < kNumProcs) {
-    printf("Raise [ulimit -n] well above %u (currently %lu) to make this test go\n", kNumProcs, rlim.rlim_cur);
+    printf("Raise [ulimit -n] above %u (currently %lu) to make this test go\n",
+           kNumProcs, rlim.rlim_cur);
     return;
   }