From 89587196705f54afb904c8f4572e65de7274dd81 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sun, 20 Mar 2016 21:41:15 -0400 Subject: Use posix_spawn() instead of fork()/exec(). 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 | 107 ++++++++++++++++++++++++------------------------ src/subprocess_test.cc | 3 +- 2 files changed, 56 insertions(+), 54 deletions(-) diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc index ae7ae6f..5ffe85b 100644 --- a/src/subprocess-posix.cc +++ b/src/subprocess-posix.cc @@ -22,6 +22,9 @@ #include #include #include +#include + +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(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; } diff --git a/src/subprocess_test.cc b/src/subprocess_test.cc index c8f2fb8..ee16190 100644 --- a/src/subprocess_test.cc +++ b/src/subprocess_test.cc @@ -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; } -- cgit v0.12