diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc index 8e785406c9..3c1de2defc 100644 --- a/src/subprocess-posix.cc +++ b/src/subprocess-posix.cc @@ -36,6 +36,8 @@ extern char** environ; using namespace std; +static void CloseFileWhenPidExits(pid_t pid, int fd); + Subprocess::Subprocess(bool use_console) : fd_(-1), pid_(-1), use_console_(use_console) { } @@ -103,12 +105,13 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { err = posix_spawn_file_actions_adddup2(&action, output_pipe[1], 2); if (err != 0) Fatal("posix_spawn_file_actions_adddup2: %s", strerror(err)); - err = posix_spawn_file_actions_addclose(&action, output_pipe[1]); - if (err != 0) - Fatal("posix_spawn_file_actions_addclose: %s", strerror(err)); // In the console case, output_pipe is still inherited by the child and // closed when the subprocess finishes, which then notifies ninja. } + err = posix_spawn_file_actions_addclose(&action, output_pipe[1]); + if (err != 0) + Fatal("posix_spawn_file_actions_addclose: %s", strerror(err)); + #ifdef POSIX_SPAWN_USEVFORK flags |= POSIX_SPAWN_USEVFORK; #endif @@ -117,20 +120,42 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { if (err != 0) Fatal("posix_spawnattr_setflags: %s", strerror(err)); + if (use_console_) { + // Block SIGCHLD to prevent a race where the new subprocess exits before we + // register its PID. + sigset_t signals; + sigemptyset(&signals); + sigaddset(&signals, SIGCHLD); + if (sigprocmask(SIG_BLOCK, &signals, NULL) < 0) + Fatal("sigprocmask: %s", strerror(errno)); + } + const char* spawned_args[] = { "/bin/sh", "-c", command.c_str(), NULL }; err = posix_spawn(&pid_, "/bin/sh", &action, &attr, const_cast(spawned_args), environ); if (err != 0) Fatal("posix_spawn: %s", strerror(err)); + if (use_console_) { + // Shared console case: We keep the write-side of the pipe just so that we + // can close it on SIGCHLD reception. + CloseFileWhenPidExits(pid_, output_pipe[1]); + sigset_t signals; + sigemptyset(&signals); + sigaddset(&signals, SIGCHLD); + if (sigprocmask(SIG_UNBLOCK, &signals, NULL) < 0) + Fatal("sigprocmask: %s", strerror(errno)); + } else { + // Not shared console: Only the subprocess needs the write-end of the pipe. + close(output_pipe[1]); + } + err = posix_spawnattr_destroy(&attr); if (err != 0) Fatal("posix_spawnattr_destroy: %s", strerror(err)); err = posix_spawn_file_actions_destroy(&action); if (err != 0) Fatal("posix_spawn_file_actions_destroy: %s", strerror(err)); - - close(output_pipe[1]); return true; } @@ -150,8 +175,10 @@ void Subprocess::OnPipeReady() { ExitStatus Subprocess::Finish() { assert(pid_ != -1); int status; - if (waitpid(pid_, &status, 0) < 0) - Fatal("waitpid(%d): %s", pid_, strerror(errno)); + while (waitpid(pid_, &status, 0) < 0) { + if (errno != EINTR) + Fatal("waitpid(%d): %s", pid_, strerror(errno)); + } pid_ = -1; #ifdef _AIX @@ -205,6 +232,93 @@ void SubprocessSet::HandlePendingInterruption() { interrupted_ = SIGHUP; } +// SIGCHLD handling: + +struct PidFdEntry; +typedef unsigned int IxEntry; +// PidFdList is used to store the PID and file descriptor pairs of the +// subprocesses being run by ninja that share a terminal. +// On SIGCHLD the PID of the signal is matched with one of the entries and the +// associated file descriptor closed. Then the entry is freed for reuse. +// Normally we would use something like std::unordered_map<>, but that involves +// memory allocations which are not reentrant. +struct PidFdList { + PidFdEntry* entries; // Entry with index 0 is reserved as NULL and not used. + size_t capacity; // Number of PidFdEntry allocated for pid_fds. + IxEntry ix_head_used; + IxEntry ix_head_free; +}; + +// A PidFdEntry may be used or free. +struct PidFdEntry { + pid_t pid; // -1 if this is a free entry + int fd; + // ix_next of a used entry points to another used entry, ix_next of free entry + // points to another free entry. + IxEntry ix_next; +}; + +static PidFdList PID_FD_LIST = { nullptr, 0, 0, 1 }; + +static void initializePidFdEntries(PidFdEntry* entries, IxEntry ix_start, size_t capacity) { + for (unsigned int i = ix_start; i < capacity; i++) { + entries[i].pid = -1; + entries[i].fd = -1; + entries[i].ix_next = i + 1; + } + entries[capacity - 1].ix_next = 0; // end of the free list. +} + +// Must only be called with SIGCHLD blocked. +static void CloseFileWhenPidExits(pid_t pid, int fd) { + PidFdList* list = &PID_FD_LIST; + if (!list->entries) { + // First use, allocate the list. + list->capacity = 16; + list->entries = new PidFdEntry[list->capacity]; + initializePidFdEntries(list->entries, 1, list->capacity); + } else if (!list->ix_head_free) { + // Expand the list to get more free entries. + size_t old_capacity = list->capacity; + list->capacity *= 2; + list->entries = static_cast(realloc(list->entries, + sizeof(PidFdEntry) * list->capacity)); + initializePidFdEntries(list->entries, old_capacity, list->capacity); + list->ix_head_free = old_capacity; + } + // Replace the head free entry and make it the new used head. + IxEntry ix_entry = list->ix_head_free; + PidFdEntry* entry = &list->entries[ix_entry]; + IxEntry ix_next_free = entry->ix_next; + entry->pid = pid; + entry->fd = fd; + entry->ix_next = list->ix_head_used; + list->ix_head_used = ix_entry; + list->ix_head_free = ix_next_free; +} + +static void SigChldHandler(int signo, siginfo_t* info, void* context) { + int pid = info->si_pid; + + PidFdList* list = &PID_FD_LIST; + if (!list->entries) + return; + for (IxEntry* ix = &list->ix_head_used; *ix; ix = &list->entries[*ix].ix_next) { + PidFdEntry* entry = &list->entries[*ix]; + if (entry->pid == pid) { + // Found a match. Close the pipe and remove the entry. + close(entry->fd); + IxEntry ix_next_used = entry->ix_next; + entry->fd = -1; + entry->pid = -1; + entry->ix_next = list->ix_head_free; + list->ix_head_free = *ix; + *ix = ix_next_used; + break; + } + } +} + SubprocessSet::SubprocessSet() { sigset_t set; sigemptyset(&set); @@ -223,6 +337,12 @@ SubprocessSet::SubprocessSet() { Fatal("sigaction: %s", strerror(errno)); if (sigaction(SIGHUP, &act, &old_hup_act_) < 0) Fatal("sigaction: %s", strerror(errno)); + + memset(&act, 0, sizeof(act)); + act.sa_flags = SA_SIGINFO | SA_NOCLDSTOP; + act.sa_sigaction = SigChldHandler; + if (sigaction(SIGCHLD, &act, &old_chld_act_) < 0) + Fatal("sigaction: %s", strerror(errno)); } SubprocessSet::~SubprocessSet() { @@ -234,6 +354,8 @@ SubprocessSet::~SubprocessSet() { Fatal("sigaction: %s", strerror(errno)); if (sigaction(SIGHUP, &old_hup_act_, 0) < 0) Fatal("sigaction: %s", strerror(errno)); + if (sigaction(SIGCHLD, &old_chld_act_, 0) < 0) + Fatal("sigaction: %s", strerror(errno)); if (sigprocmask(SIG_SETMASK, &old_mask_, 0) < 0) Fatal("sigprocmask: %s", strerror(errno)); } diff --git a/src/subprocess.h b/src/subprocess.h index 9e3d2ee98f..77edb122fe 100644 --- a/src/subprocess.h +++ b/src/subprocess.h @@ -106,6 +106,7 @@ struct SubprocessSet { struct sigaction old_int_act_; struct sigaction old_term_act_; struct sigaction old_hup_act_; + struct sigaction old_chld_act_; sigset_t old_mask_; #endif };