Merge pull request #743 from nicolasdespres/sigterm

Handle SIGTERM properly.

3 files changed, 72 insertions, 35 deletions

diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc
index cc8bff6..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) {
 }
@@ -74,7 +60,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 +136,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;
@@ -162,31 +150,48 @@ 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() {
   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));
@@ -217,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;
@@ -248,7 +254,7 @@ bool SubprocessSet::DoWork() {
     ++i;
   }
 
-  return interrupted_;
+  return IsInterrupted();
 }
 
 #else  // !defined(USE_PPOLL)
@@ -267,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<Subprocess*>::iterator i = running_.begin();
@@ -294,7 +301,7 @@ bool SubprocessSet::DoWork() {
     ++i;
   }
 
-  return interrupted_;
+  return IsInterrupted();
 }
 #endif  // !defined(USE_PPOLL)
 
@@ -309,10 +316,10 @@ Subprocess* SubprocessSet::NextFinished() {
 void SubprocessSet::Clear() {
   for (vector<Subprocess*>::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<Subprocess*>::iterator i = running_.begin();
        i != running_.end(); ++i)
     delete *i;
diff --git a/src/subprocess.h b/src/subprocess.h
index b7a1a4c..a001fc9 100644
--- a/src/subprocess.h
+++ b/src/subprocess.h
@@ -89,9 +89,15 @@ 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_;
 
-  struct sigaction old_act_;
+  static bool IsInterrupted() { return interrupted_ != 0; }
+
+  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.