From b4bc5cf7c924be251a5c8abdfca58f47c3f5c185 Mon Sep 17 00:00:00 2001 From: Nicolas Despres Date: Wed, 16 Apr 2014 10:05:25 +0200 Subject: Allow SIGTERM for interruption. Default signal sent by many other programs (mainly kill(1)) to gently terminates another one is SIGTERM. --- src/subprocess-posix.cc | 15 +++++++++++---- src/subprocess.h | 3 ++- src/subprocess_test.cc | 26 +++++++++++++++++++++++++- 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc index cc8bff6..c784a39 100644 --- a/src/subprocess-posix.cc +++ b/src/subprocess-posix.cc @@ -74,7 +74,9 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { // Track which fd we use to report errors on. int error_pipe = output_pipe[1]; do { - if (sigaction(SIGINT, &set->old_act_, 0) < 0) + if (sigaction(SIGINT, &set->old_int_act_, 0) < 0) + break; + if (sigaction(SIGTERM, &set->old_term_act_, 0) < 0) break; if (sigprocmask(SIG_SETMASK, &set->old_mask_, 0) < 0) break; @@ -148,7 +150,7 @@ ExitStatus Subprocess::Finish() { if (exit == 0) return ExitSuccess; } else if (WIFSIGNALED(status)) { - if (WTERMSIG(status) == SIGINT) + if (WTERMSIG(status) == SIGINT || WTERMSIG(status) == SIGTERM) return ExitInterrupted; } return ExitFailure; @@ -173,20 +175,25 @@ SubprocessSet::SubprocessSet() { sigset_t set; sigemptyset(&set); sigaddset(&set, SIGINT); + sigaddset(&set, SIGTERM); if (sigprocmask(SIG_BLOCK, &set, &old_mask_) < 0) Fatal("sigprocmask: %s", strerror(errno)); struct sigaction act; memset(&act, 0, sizeof(act)); act.sa_handler = SetInterruptedFlag; - if (sigaction(SIGINT, &act, &old_act_) < 0) + if (sigaction(SIGINT, &act, &old_int_act_) < 0) + Fatal("sigaction: %s", strerror(errno)); + if (sigaction(SIGTERM, &act, &old_term_act_) < 0) Fatal("sigaction: %s", strerror(errno)); } SubprocessSet::~SubprocessSet() { Clear(); - if (sigaction(SIGINT, &old_act_, 0) < 0) + if (sigaction(SIGINT, &old_int_act_, 0) < 0) + Fatal("sigaction: %s", strerror(errno)); + if (sigaction(SIGTERM, &old_term_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 b7a1a4c..daeeef6 100644 --- a/src/subprocess.h +++ b/src/subprocess.h @@ -91,7 +91,8 @@ struct SubprocessSet { static void SetInterruptedFlag(int signum); static bool interrupted_; - struct sigaction old_act_; + struct sigaction old_int_act_; + struct sigaction old_term_act_; sigset_t old_mask_; #endif }; diff --git a/src/subprocess_test.cc b/src/subprocess_test.cc index 1838c43..07cc52f 100644 --- a/src/subprocess_test.cc +++ b/src/subprocess_test.cc @@ -98,6 +98,30 @@ TEST_F(SubprocessTest, InterruptParent) { ASSERT_FALSE("We should have been interrupted"); } +TEST_F(SubprocessTest, InterruptChildWithSigTerm) { + Subprocess* subproc = subprocs_.Add("kill -TERM $$"); + ASSERT_NE((Subprocess *) 0, subproc); + + while (!subproc->Done()) { + subprocs_.DoWork(); + } + + EXPECT_EQ(ExitInterrupted, subproc->Finish()); +} + +TEST_F(SubprocessTest, InterruptParentWithSigTerm) { + Subprocess* subproc = subprocs_.Add("kill -TERM $PPID ; sleep 1"); + ASSERT_NE((Subprocess *) 0, subproc); + + while (!subproc->Done()) { + bool interrupted = subprocs_.DoWork(); + if (interrupted) + return; + } + + ASSERT_FALSE("We should have been interrupted"); +} + // A shell command to check if the current process is connected to a terminal. // This is different from having stdin/stdout/stderr be a terminal. (For // instance consider the command "yes < /dev/null > /dev/null 2>&1". @@ -221,7 +245,7 @@ TEST_F(SubprocessTest, SetWithLots) { } ASSERT_EQ(kNumProcs, subprocs_.finished_.size()); } -#endif // !__APPLE__ && !_WIN32 +#endif // !__APPLE__ && !_WIN32 // TODO: this test could work on Windows, just not sure how to simply // read stdin. -- cgit v1.2.1 From 9fff9551b2f3d3e5c994512b06832b40560d510b Mon Sep 17 00:00:00 2001 From: Nicolas Despres Date: Wed, 16 Apr 2014 10:08:42 +0200 Subject: Forward interruption signal to child processes. --- src/subprocess-posix.cc | 56 ++++++++++++++++++++++++------------------------- src/subprocess.h | 7 ++++++- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc index c784a39..f3baec2 100644 --- a/src/subprocess-posix.cc +++ b/src/subprocess-posix.cc @@ -25,20 +25,6 @@ #include "util.h" -namespace { - -bool HasPendingInterruption() { - sigset_t pending; - sigemptyset(&pending); - if (sigpending(&pending) == -1) { - perror("ninja: sigpending"); - return false; - } - return sigismember(&pending, SIGINT); -} - -} // anonymous namespace - Subprocess::Subprocess(bool use_console) : fd_(-1), pid_(-1), use_console_(use_console) { } @@ -164,11 +150,23 @@ const string& Subprocess::GetOutput() const { return buf_; } -bool SubprocessSet::interrupted_; +int SubprocessSet::interrupted_; void SubprocessSet::SetInterruptedFlag(int signum) { - (void) signum; - interrupted_ = true; + interrupted_ = signum; +} + +void SubprocessSet::HandlePendingInterruption() { + sigset_t pending; + sigemptyset(&pending); + if (sigpending(&pending) == -1) { + perror("ninja: sigpending"); + return; + } + if (sigismember(&pending, SIGINT)) + interrupted_ = SIGINT; + else if (sigismember(&pending, SIGTERM)) + interrupted_ = SIGTERM; } SubprocessSet::SubprocessSet() { @@ -224,17 +222,18 @@ bool SubprocessSet::DoWork() { ++nfds; } - interrupted_ = false; + interrupted_ = 0; int ret = ppoll(&fds.front(), nfds, NULL, &old_mask_); if (ret == -1) { if (errno != EINTR) { perror("ninja: ppoll"); return false; } - return interrupted_; + return IsInterrupted(); } - if (HasPendingInterruption()) + HandlePendingInterruption(); + if (IsInterrupted()) return true; nfds_t cur_nfd = 0; @@ -255,7 +254,7 @@ bool SubprocessSet::DoWork() { ++i; } - return interrupted_; + return IsInterrupted(); } #else // !defined(USE_PPOLL) @@ -274,17 +273,18 @@ bool SubprocessSet::DoWork() { } } - interrupted_ = false; + interrupted_ = 0; int ret = pselect(nfds, &set, 0, 0, 0, &old_mask_); if (ret == -1) { if (errno != EINTR) { perror("ninja: pselect"); return false; } - return interrupted_; + return IsInterrupted(); } - if (HasPendingInterruption()) + HandlePendingInterruption(); + if (IsInterrupted()) return true; for (vector::iterator i = running_.begin(); @@ -301,7 +301,7 @@ bool SubprocessSet::DoWork() { ++i; } - return interrupted_; + return IsInterrupted(); } #endif // !defined(USE_PPOLL) @@ -316,10 +316,10 @@ Subprocess* SubprocessSet::NextFinished() { void SubprocessSet::Clear() { for (vector::iterator i = running_.begin(); i != running_.end(); ++i) - // Since the foreground process is in our process group, it will receive a - // SIGINT at the same time as us. + // Since the foreground process is in our process group, it will receive + // the interruption signal (i.e. SIGINT or SIGTERM) at the same time as us. if (!(*i)->use_console_) - kill(-(*i)->pid_, SIGINT); + kill(-(*i)->pid_, interrupted_); for (vector::iterator i = running_.begin(); i != running_.end(); ++i) delete *i; diff --git a/src/subprocess.h b/src/subprocess.h index daeeef6..a001fc9 100644 --- a/src/subprocess.h +++ b/src/subprocess.h @@ -89,7 +89,12 @@ struct SubprocessSet { static HANDLE ioport_; #else static void SetInterruptedFlag(int signum); - static bool interrupted_; + static void HandlePendingInterruption(); + /// Store the signal number that causes the interruption. + /// 0 if not interruption. + static int interrupted_; + + static bool IsInterrupted() { return interrupted_ != 0; } struct sigaction old_int_act_; struct sigaction old_term_act_; -- cgit v1.2.1