Merge pull request #241 from danw/posix_spawn

Switch from vfork to posix_spawn

8 files changed, 97 insertions, 50 deletions

diff --git a/Dockerfile b/Dockerfile
index 9cd4975..b4423be 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -1,6 +1,6 @@
-FROM ubuntu:18.04
+FROM ubuntu:20.04
 
-RUN apt-get update && apt-get install -y make git-core build-essential curl ninja-build python
+RUN apt-get update && apt-get install -y make git-core build-essential curl ninja-build python python3
 
 # Install Go
 RUN \
@@ -13,6 +13,7 @@ ENV GOPATH /gopath
 ENV PATH $GOROOT/bin:$GOPATH/bin:$PATH
 
 # Copy project code.
-COPY . /
+COPY . /src
+WORKDIR /src
 
-ENTRYPOINT make test
+CMD make test -j8
diff --git a/src/fileutil.cc b/src/fileutil.cc
index fef8d82..14ee68a 100644
--- a/src/fileutil.cc
+++ b/src/fileutil.cc
@@ -21,6 +21,7 @@
 #include <glob.h>
 #include <limits.h>
 #include <signal.h>
+#include <spawn.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/wait.h>
@@ -34,6 +35,8 @@
 #include "log.h"
 #include "strutil.h"
 
+extern char** environ;
+
 bool Exists(StringPiece filename) {
   CHECK(filename.size() < PATH_MAX);
   struct stat st;
@@ -84,52 +87,95 @@ int RunCommand(const string& shell,
   int pipefd[2];
   if (pipe(pipefd) != 0)
     PERROR("pipe failed");
-  int pid;
-  if ((pid = vfork())) {
-    int status;
-    close(pipefd[1]);
-    while (true) {
-      int result = waitpid(pid, &status, WNOHANG);
-      if (result < 0)
-        PERROR("waitpid failed");
-
-      while (true) {
-        char buf[4096];
-        ssize_t r = HANDLE_EINTR(read(pipefd[0], buf, 4096));
-        if (r < 0)
-          PERROR("read failed");
-        if (r == 0)
-          break;
-        s->append(buf, buf + r);
-      }
 
-      if (result != 0) {
-        break;
-      }
+  posix_spawn_file_actions_t action;
+  int err = posix_spawn_file_actions_init(&action);
+  if (err != 0) {
+    ERROR("posix_spawn_file_actions_init: %s", strerror(err));
+  }
+
+  err = posix_spawn_file_actions_addclose(&action, pipefd[0]);
+  if (err != 0) {
+    ERROR("posix_spawn_file_actions_addclose: %s", strerror(err));
+  }
+
+  if (redirect_stderr == RedirectStderr::STDOUT) {
+    err = posix_spawn_file_actions_adddup2(&action, pipefd[1], 2);
+    if (err != 0) {
+      ERROR("posix_spawn_file_actions_adddup2: %s", strerror(err));
+    }
+  } else if (redirect_stderr == RedirectStderr::DEV_NULL) {
+    err =
+        posix_spawn_file_actions_addopen(&action, 2, "/dev/null", O_WRONLY, 0);
+    if (err != 0) {
+      ERROR("posix_spawn_file_actions_addopen: %s", strerror(err));
     }
-    close(pipefd[0]);
+  }
+  err = posix_spawn_file_actions_adddup2(&action, pipefd[1], 1);
+  if (err != 0) {
+    ERROR("posix_spawn_file_actions_adddup2: %s", strerror(err));
+  }
+  err = posix_spawn_file_actions_addclose(&action, pipefd[1]);
+  if (err != 0) {
+    ERROR("posix_spawn_file_actions_addclose: %s", strerror(err));
+  }
 
-    return status;
-  } else {
-    close(pipefd[0]);
-    if (redirect_stderr == RedirectStderr::STDOUT) {
-      if (dup2(pipefd[1], 2) < 0)
-        PERROR("dup2 failed");
-    } else if (redirect_stderr == RedirectStderr::DEV_NULL) {
-      int fd = open("/dev/null", O_WRONLY);
-      if (dup2(fd, 2) < 0)
-        PERROR("dup2 failed");
-      close(fd);
+  posix_spawnattr_t attr;
+  err = posix_spawnattr_init(&attr);
+  if (err != 0) {
+    ERROR("posix_spawnattr_init: %s", strerror(err));
+  }
+
+  short flags = 0;
+#ifdef POSIX_SPAWN_USEVFORK
+  flags |= POSIX_SPAWN_USEVFORK;
+#endif
+
+  err = posix_spawnattr_setflags(&attr, flags);
+  if (err != 0) {
+    ERROR("posix_spawnattr_setflags: %s", strerror(err));
+  }
+
+  pid_t pid;
+  err = posix_spawn(&pid, argv[0], &action, &attr, const_cast<char**>(argv),
+                    environ);
+  if (err != 0) {
+    ERROR("posix_spawn: %s", strerror(err));
+  }
+
+  err = posix_spawnattr_destroy(&attr);
+  if (err != 0) {
+    ERROR("posix_spawnattr_destroy: %s", strerror(err));
+  }
+  err = posix_spawn_file_actions_destroy(&action);
+  if (err != 0) {
+    ERROR("posix_spawn_file_actions_destroy: %s", strerror(err));
+  }
+
+  int status;
+  close(pipefd[1]);
+  while (true) {
+    int result = waitpid(pid, &status, WNOHANG);
+    if (result < 0)
+      PERROR("waitpid failed");
+
+    while (true) {
+      char buf[4096];
+      ssize_t r = HANDLE_EINTR(read(pipefd[0], buf, 4096));
+      if (r < 0)
+        PERROR("read failed");
+      if (r == 0)
+        break;
+      s->append(buf, buf + r);
     }
-    if (dup2(pipefd[1], 1) < 0)
-      PERROR("dup2 failed");
-    close(pipefd[1]);
 
-    execvp(argv[0], const_cast<char**>(argv));
-    PLOG("execvp for %s failed", argv[0]);
-    kill(getppid(), SIGTERM);
-    _exit(1);
+    if (result != 0) {
+      break;
+    }
   }
+  close(pipefd[0]);
+
+  return status;
 }
 
 std::string GetExecutablePath() {
diff --git a/testcase/ninja_mkdir.sh b/testcase/ninja_mkdir.sh
index e54298d..de85733 100644
--- a/testcase/ninja_mkdir.sh
+++ b/testcase/ninja_mkdir.sh
@@ -16,7 +16,7 @@
 
 set -e
 
-log=/tmp/log
+log=stderr_log
 mk="$@"
 
 cat <<EOF > Makefile
diff --git a/testcase/ninja_pool.sh b/testcase/ninja_pool.sh
index 2ae3240..87f0053 100644
--- a/testcase/ninja_pool.sh
+++ b/testcase/ninja_pool.sh
@@ -16,7 +16,7 @@
 
 set -e
 
-log=/tmp/log
+log=stderr_log
 mk="$@"
 
 cat <<EOF > Makefile
diff --git a/testcase/ninja_regen.sh b/testcase/ninja_regen.sh
index 73ef34a..3c234ab 100755
--- a/testcase/ninja_regen.sh
+++ b/testcase/ninja_regen.sh
@@ -16,7 +16,7 @@
 
 set -e
 
-log=/tmp/log
+log=stderr_log
 mk="$@"
 
 export VAR=hoge
diff --git a/testcase/ninja_regen_filefunc_read.sh b/testcase/ninja_regen_filefunc_read.sh
index ce26614..52edf9f 100755
--- a/testcase/ninja_regen_filefunc_read.sh
+++ b/testcase/ninja_regen_filefunc_read.sh
@@ -16,7 +16,7 @@
 
 set -e
 
-log=/tmp/log
+log=stderr_log
 mk="$@"
 
 cat <<EOF > Makefile
diff --git a/testcase/ninja_regen_filefunc_write.sh b/testcase/ninja_regen_filefunc_write.sh
index cb438cc..bbf420e 100755
--- a/testcase/ninja_regen_filefunc_write.sh
+++ b/testcase/ninja_regen_filefunc_write.sh
@@ -16,7 +16,7 @@
 
 set -e
 
-log=/tmp/log
+log=stderr_log
 mk="$@"
 
 cat <<EOF > Makefile
diff --git a/testcase/ninja_regen_glob.sh b/testcase/ninja_regen_glob.sh
index ae73c9b..adb2905 100755
--- a/testcase/ninja_regen_glob.sh
+++ b/testcase/ninja_regen_glob.sh
@@ -16,7 +16,7 @@
 
 set -e
 
-log=/tmp/log
+log=stderr_log
 mk="$@"
 
 touch xe.mk yc.mk xa.mk yb.mk xd.mk