Implement cleanup-on-interrupt
authorPeter Collingbourne <peter@pcc.me.uk>
Sun, 13 Nov 2011 05:49:16 +0000 (05:49 +0000)
committerPeter Collingbourne <peter@pcc.me.uk>
Sat, 4 Feb 2012 21:46:12 +0000 (21:46 +0000)
This causes us to clean up by deleting any output files belonging
to currently-running commands before we quit if we are interrupted
(either by Ctrl-C or by a command failing).

Fixes issue #110.

src/build.cc
src/build.h
src/build_test.cc
src/exit_status.h [new file with mode: 0644]
src/subprocess-win32.cc
src/subprocess.cc
src/subprocess.h
src/subprocess_test.cc

index 8c9981c..4c15de9 100644 (file)
@@ -396,35 +396,52 @@ struct RealCommandRunner : public CommandRunner {
   virtual ~RealCommandRunner() {}
   virtual bool CanRunMore();
   virtual bool StartCommand(Edge* edge);
-  virtual Edge* WaitForCommand(bool* success, string* output);
+  virtual Edge* WaitForCommand(ExitStatus* status, string* output);
+  virtual vector<Edge*> GetActiveEdges();
+  virtual void Abort();
 
   const BuildConfig& config_;
   SubprocessSet subprocs_;
   map<Subprocess*, Edge*> subproc_to_edge_;
 };
 
+vector<Edge*> RealCommandRunner::GetActiveEdges() {
+  vector<Edge*> edges;
+  for (map<Subprocess*, Edge*>::iterator i = subproc_to_edge_.begin();
+       i != subproc_to_edge_.end(); ++i)
+    edges.push_back(i->second);
+  return edges;
+}
+
+void RealCommandRunner::Abort() {
+  subprocs_.Clear();
+}
+
 bool RealCommandRunner::CanRunMore() {
   return ((int)subprocs_.running_.size()) < config_.parallelism;
 }
 
 bool RealCommandRunner::StartCommand(Edge* edge) {
   string command = edge->EvaluateCommand();
-  Subprocess* subproc = new Subprocess;
-  subproc_to_edge_.insert(make_pair(subproc, edge));
-  if (!subproc->Start(&subprocs_, command))
+  Subprocess* subproc = subprocs_.Add(command);
+  if (!subproc)
     return false;
-
-  subprocs_.Add(subproc);
+  subproc_to_edge_.insert(make_pair(subproc, edge));
+  
   return true;
 }
 
-Edge* RealCommandRunner::WaitForCommand(bool* success, string* output) {
+Edge* RealCommandRunner::WaitForCommand(ExitStatus* status, string* output) {
   Subprocess* subproc;
   while ((subproc = subprocs_.NextFinished()) == NULL) {
-    subprocs_.DoWork();
+    bool interrupted = subprocs_.DoWork();
+    if (interrupted) {
+      *status = ExitInterrupted;
+      return 0;
+    }
   }
 
-  *success = subproc->Finish();
+  *status = subproc->Finish();
   *output = subproc->GetOutput();
 
   map<Subprocess*, Edge*>::iterator i = subproc_to_edge_.find(subproc);
@@ -445,10 +462,12 @@ struct DryRunCommandRunner : public CommandRunner {
     finished_.push(edge);
     return true;
   }
-  virtual Edge* WaitForCommand(bool* success, string* output) {
-    if (finished_.empty())
+  virtual Edge* WaitForCommand(ExitStatus* status, string* /* output */) {
+    if (finished_.empty()) {
+      *status = ExitFailure;
       return NULL;
-    *success = true;
+    }
+    *status = ExitSuccess;
     Edge* edge = finished_.front();
     finished_.pop();
     return edge;
@@ -461,13 +480,29 @@ Builder::Builder(State* state, const BuildConfig& config)
     : state_(state), config_(config) {
   disk_interface_ = new RealDiskInterface;
   if (config.dry_run)
-    command_runner_ = new DryRunCommandRunner;
+    command_runner_.reset(new DryRunCommandRunner);
   else
-    command_runner_ = new RealCommandRunner(config);
+    command_runner_.reset(new RealCommandRunner(config));
   status_ = new BuildStatus(config);
   log_ = state->build_log_;
 }
 
+Builder::~Builder() {
+  if (command_runner_.get()) {
+    vector<Edge*> active_edges = command_runner_->GetActiveEdges();
+    command_runner_->Abort();
+
+    for (vector<Edge*>::iterator i = active_edges.begin();
+         i != active_edges.end(); ++i) {
+      for (vector<Node*>::iterator ni = (*i)->outputs_.begin();
+           ni != (*i)->outputs_.end(); ++ni)
+        disk_interface_->RemoveFile((*ni)->path());
+      if (!(*i)->rule_->depfile_.empty())
+        disk_interface_->RemoveFile((*i)->EvaluateDepFile());
+    }
+  }
+}
+
 Node* Builder::AddTarget(const string& name, string* err) {
   Node* node = state_->LookupNode(name);
   if (!node) {
@@ -533,10 +568,11 @@ bool Builder::Build(string* err) {
 
     // See if we can reap any finished commands.
     if (pending_commands) {
-      bool success;
+      ExitStatus status;
       string output;
-      Edge* edge;
-      if ((edge = command_runner_->WaitForCommand(&success, &output))) {
+      Edge* edge = command_runner_->WaitForCommand(&status, &output);
+      if (edge && status != ExitInterrupted) {
+        bool success = (status == ExitSuccess);
         --pending_commands;
         FinishEdge(edge, success, output);
         if (!success) {
@@ -552,6 +588,12 @@ bool Builder::Build(string* err) {
         // We made some progress; start the main loop over.
         continue;
       }
+
+      if (status == ExitInterrupted) {
+        status_->BuildFinished();
+        *err = "interrupted by user";
+        return false;
+      }
     }
 
     // If we get here, we can neither enqueue new commands nor are any running.
index 586d1ff..c75bde1 100644 (file)
 #include <string>
 #include <queue>
 #include <vector>
+#include <memory>
 using namespace std;
 
+#include "exit_status.h"
+
 struct BuildLog;
 struct Edge;
 struct DiskInterface;
@@ -87,7 +90,9 @@ struct CommandRunner {
   virtual bool CanRunMore() = 0;
   virtual bool StartCommand(Edge* edge) = 0;
   /// Wait for a command to complete.
-  virtual Edge* WaitForCommand(bool* success, string* output) = 0;
+  virtual Edge* WaitForCommand(ExitStatus* status, string* output) = 0;
+  virtual vector<Edge*> GetActiveEdges() { return vector<Edge*>(); }
+  virtual void Abort() {}
 };
 
 /// Options (e.g. verbosity, parallelism) passed to a build.
@@ -109,6 +114,7 @@ struct BuildConfig {
 /// Builder wraps the build process: starting commands, updating status.
 struct Builder {
   Builder(State* state, const BuildConfig& config);
+  ~Builder();
 
   Node* AddTarget(const string& name, string* err);
 
@@ -130,9 +136,14 @@ struct Builder {
   const BuildConfig& config_;
   Plan plan_;
   DiskInterface* disk_interface_;
-  CommandRunner* command_runner_;
+  auto_ptr<CommandRunner> command_runner_;
   struct BuildStatus* status_;
   struct BuildLog* log_;
+
+private:
+  // Unimplemented copy ctor and operator= ensure we don't copy the auto_ptr.
+  Builder(const Builder &other);        // DO NOT IMPLEMENT
+  void operator=(const Builder &other); // DO NOT IMPLEMENT
 };
 
 #endif  // NINJA_BUILD_H_
index 0fa23ed..de5ddc1 100644 (file)
@@ -181,7 +181,7 @@ struct BuildTest : public StateTestWithBuiltinRules,
   BuildTest() : config_(MakeConfig()), builder_(&state_, config_), now_(1),
                 last_command_(NULL) {
     builder_.disk_interface_ = &fs_;
-    builder_.command_runner_ = this;
+    builder_.command_runner_.reset(this);
     AssertParse(&state_,
 "build cat1: cat in1\n"
 "build cat2: cat in1 in2\n"
@@ -191,13 +191,17 @@ struct BuildTest : public StateTestWithBuiltinRules,
     fs_.Create("in2", now_, "");
   }
 
+  ~BuildTest() {
+    builder_.command_runner_.release();
+  }
+
   // Mark a path dirty.
   void Dirty(const string& path);
 
   // CommandRunner impl
   virtual bool CanRunMore();
   virtual bool StartCommand(Edge* edge);
-  virtual Edge* WaitForCommand(bool* success, string* output);
+  virtual Edge* WaitForCommand(ExitStatus* status, string* output);
 
   BuildConfig MakeConfig() {
     BuildConfig config;
@@ -251,15 +255,16 @@ bool BuildTest::StartCommand(Edge* edge) {
   return true;
 }
 
-Edge* BuildTest::WaitForCommand(bool* success, string* output) {
+Edge* BuildTest::WaitForCommand(ExitStatus* status, string* /* output */) {
   if (Edge* edge = last_command_) {
     if (edge->rule().name() == "fail")
-      *success = false;
+      *status = ExitFailure;
     else
-      *success = true;
+      *status = ExitSuccess;
     last_command_ = NULL;
     return edge;
   }
+  *status = ExitFailure;
   return NULL;
 }
 
diff --git a/src/exit_status.h b/src/exit_status.h
new file mode 100644 (file)
index 0000000..a714ece
--- /dev/null
@@ -0,0 +1,24 @@
+// Copyright 2011 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef NINJA_EXIT_STATUS_H_
+#define NINJA_EXIT_STATUS_H_
+
+enum ExitStatus {
+  ExitSuccess,
+  ExitFailure,
+  ExitInterrupted
+};
+
+#endif  // NINJA_EXIT_STATUS_H_
index e757f7e..6b7e942 100644 (file)
@@ -32,6 +32,10 @@ Subprocess::Subprocess() : child_(NULL) , overlapped_() {
 }
 
 Subprocess::~Subprocess() {
+  if (pipe_) {
+    if (!CloseHandle(pipe_))
+      Win32Fatal("CloseHandle");
+  }
   // Reap child if forgotten.
   if (child_)
     Finish();
@@ -92,7 +96,7 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) {
   // Do not prepend 'cmd /c' on Windows, this breaks command
   // lines greater than 8,191 chars.
   if (!CreateProcessA(NULL, (char*)command.c_str(), NULL, NULL,
-                      /* inherit handles */ TRUE, 0,
+                      /* inherit handles */ TRUE, CREATE_NEW_PROCESS_GROUP,
                       NULL, NULL,
                       &startup_info, &process_info)) {
     DWORD error = GetLastError();
@@ -149,10 +153,9 @@ void Subprocess::OnPipeReady() {
   // function again later and get them at that point.
 }
 
-bool Subprocess::Finish() {
-  if (! child_) {
-    return false;
-  }
+ExitStatus Subprocess::Finish() {
+  if (!child_)
+    return ExitFailure;
 
   // TODO: add error handling for all of these.
   WaitForSingleObject(child_, INFINITE);
@@ -163,7 +166,9 @@ bool Subprocess::Finish() {
   CloseHandle(child_);
   child_ = NULL;
 
-  return exit_code == 0;
+  return exit_code == 0              ? ExitSuccess :
+         exit_code == CONTROL_C_EXIT ? ExitInterrupted :
+                                       ExitFailure;
 }
 
 bool Subprocess::Done() const {
@@ -174,24 +179,47 @@ const string& Subprocess::GetOutput() const {
   return buf_;
 }
 
+HANDLE SubprocessSet::ioport_;
+
 SubprocessSet::SubprocessSet() {
   ioport_ = ::CreateIoCompletionPort(INVALID_HANDLE_VALUE, NULL, 0, 1);
   if (!ioport_)
     Win32Fatal("CreateIoCompletionPort");
+  if (!SetConsoleCtrlHandler(NotifyInterrupted, TRUE))
+    Win32Fatal("SetConsoleCtrlHandler");
 }
 
 SubprocessSet::~SubprocessSet() {
+  Clear();
+
+  SetConsoleCtrlHandler(NotifyInterrupted, FALSE);
   CloseHandle(ioport_);
 }
 
-void SubprocessSet::Add(Subprocess* subprocess) {
+BOOL WINAPI SubprocessSet::NotifyInterrupted(DWORD dwCtrlType) {
+  if (dwCtrlType == CTRL_C_EVENT || dwCtrlType == CTRL_BREAK_EVENT) {
+    if (!PostQueuedCompletionStatus(ioport_, 0, 0, NULL))
+      Win32Fatal("PostQueuedCompletionStatus");
+    return TRUE;
+  }
+
+  return FALSE;
+}
+
+Subprocess *SubprocessSet::Add(const string &command) {
+  Subprocess *subprocess = new Subprocess;
+  if (!subprocess->Start(this, command)) {
+    delete subprocess;
+    return 0;
+  }
   if (subprocess->child_)
     running_.push_back(subprocess);
   else
     finished_.push(subprocess);
+  return subprocess;
 }
 
-void SubprocessSet::DoWork() {
+bool SubprocessSet::DoWork() {
   DWORD bytes_read;
   Subprocess* subproc;
   OVERLAPPED* overlapped;
@@ -202,6 +230,10 @@ void SubprocessSet::DoWork() {
       Win32Fatal("GetQueuedCompletionStatus");
   }
 
+  if (!subproc) // A NULL subproc indicates that we were interrupted and is
+                // delivered by NotifyInterrupted above.
+    return true;
+
   subproc->OnPipeReady();
 
   if (subproc->Done()) {
@@ -212,6 +244,8 @@ void SubprocessSet::DoWork() {
       running_.resize(end - running_.begin());
     }
   }
+
+  return false;
 }
 
 Subprocess* SubprocessSet::NextFinished() {
@@ -221,3 +255,16 @@ Subprocess* SubprocessSet::NextFinished() {
   finished_.pop();
   return subproc;
 }
+
+void SubprocessSet::Clear() {
+  for (vector<Subprocess*>::iterator i = running_.begin();
+       i != running_.end(); ++i) {
+    if ((*i)->child_)
+      if (!GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, GetProcessId((*i)->child_)))
+        Win32Fatal("GenerateConsoleCtrlEvent");
+  }
+  for (vector<Subprocess*>::iterator i = running_.begin();
+       i != running_.end(); ++i)
+    delete *i;
+  running_.clear();
+}
index 74eded0..d4a7d03 100644 (file)
@@ -42,6 +42,10 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) {
   if (pipe(output_pipe) < 0)
     Fatal("pipe: %s", strerror(errno));
   fd_ = output_pipe[0];
+  // fd_ may be a member of the pselect set in SubprocessSet::DoWork.  Check
+  // that it falls below the system limit.
+  if (fd_ >= FD_SETSIZE)
+    Fatal("pipe: %s", strerror(EMFILE));
   SetCloseOnExec(fd_);
 
   pid_ = fork();
@@ -54,6 +58,14 @@ 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 (setpgid(0, 0) < 0)
+        break;
+
+      if (sigaction(SIGINT, &set->old_act_, 0) < 0)
+        break;
+      if (sigprocmask(SIG_SETMASK, &set->old_mask_, 0) < 0)
+        break;
+
       // Open /dev/null over stdin.
       int devnull = open("/dev/null", O_WRONLY);
       if (devnull < 0)
@@ -100,7 +112,7 @@ void Subprocess::OnPipeReady() {
   }
 }
 
-bool Subprocess::Finish() {
+ExitStatus Subprocess::Finish() {
   assert(pid_ != -1);
   int status;
   if (waitpid(pid_, &status, 0) < 0)
@@ -110,9 +122,12 @@ bool Subprocess::Finish() {
   if (WIFEXITED(status)) {
     int exit = WEXITSTATUS(status);
     if (exit == 0)
-      return true;
+      return ExitSuccess;
+  } else if (WIFSIGNALED(status)) {
+    if (WTERMSIG(status) == SIGINT)
+      return ExitInterrupted;
   }
-  return false;
+  return ExitFailure;
 }
 
 bool Subprocess::Done() const {
@@ -123,48 +138,89 @@ const string& Subprocess::GetOutput() const {
   return buf_;
 }
 
-SubprocessSet::SubprocessSet() {}
-SubprocessSet::~SubprocessSet() {}
+bool SubprocessSet::interrupted_;
+
+void SubprocessSet::SetInterruptedFlag(int signum) {
+  (void) signum;
+  interrupted_ = true;
+}
+
+SubprocessSet::SubprocessSet() {
+  interrupted_ = false;
+
+  sigset_t set;
+  sigemptyset(&set);
+  sigaddset(&set, SIGINT);
+  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)
+    Fatal("sigaction: %s", strerror(errno));
+}
+
+SubprocessSet::~SubprocessSet() {
+  Clear();
 
-void SubprocessSet::Add(Subprocess* subprocess) {
+  if (sigaction(SIGINT, &old_act_, 0) < 0)
+    Fatal("sigaction: %s", strerror(errno));
+  if (sigprocmask(SIG_SETMASK, &old_mask_, 0) < 0)
+    Fatal("sigprocmask: %s", strerror(errno));
+}
+
+Subprocess *SubprocessSet::Add(const string &command) {
+  Subprocess *subprocess = new Subprocess;
+  if (!subprocess->Start(this, command)) {
+    delete subprocess;
+    return 0;
+  }
   running_.push_back(subprocess);
+  return subprocess;
 }
 
-void SubprocessSet::DoWork() {
-  vector<pollfd> fds;
+bool SubprocessSet::DoWork() {
+  fd_set set;
+  int nfds = 0;
+  FD_ZERO(&set);
 
-  map<int, Subprocess*> fd_to_subprocess;
   for (vector<Subprocess*>::iterator i = running_.begin();
        i != running_.end(); ++i) {
     int fd = (*i)->fd_;
     if (fd >= 0) {
-      fd_to_subprocess[fd] = *i;
-      fds.resize(fds.size() + 1);
-      pollfd* newfd = &fds.back();
-      newfd->fd = fd;
-      newfd->events = POLLIN;
-      newfd->revents = 0;
+      FD_SET(fd, &set);
+      if (nfds < fd+1)
+        nfds = fd+1;
     }
   }
 
-  int ret = poll(&fds.front(), fds.size(), -1);
+  int ret = pselect(nfds, &set, 0, 0, 0, &old_mask_);
   if (ret == -1) {
-    if (errno != EINTR)
-      perror("ninja: poll");
-    return;
+    if (errno != EINTR) {
+      perror("ninja: pselect");
+      return false;
+    }
+    bool interrupted = interrupted_;
+    interrupted_ = false;
+    return interrupted;
   }
 
-  for (size_t i = 0; i < fds.size(); ++i) {
-    if (fds[i].revents) {
-      Subprocess* subproc = fd_to_subprocess[fds[i].fd];
-      subproc->OnPipeReady();
-      if (subproc->Done()) {
-        finished_.push(subproc);
-        std::remove(running_.begin(), running_.end(), subproc);
-        running_.resize(running_.size() - 1);
+  for (vector<Subprocess*>::iterator i = running_.begin();
+       i != running_.end(); ) {
+    int fd = (*i)->fd_;
+    if (fd >= 0 && FD_ISSET(fd, &set)) {
+      (*i)->OnPipeReady();
+      if ((*i)->Done()) {
+        finished_.push(*i);
+        running_.erase(i);
+        continue;
       }
     }
+    ++i;
   }
+
+  return false;
 }
 
 Subprocess* SubprocessSet::NextFinished() {
@@ -174,3 +230,13 @@ Subprocess* SubprocessSet::NextFinished() {
   finished_.pop();
   return subproc;
 }
+
+void SubprocessSet::Clear() {
+  for (vector<Subprocess*>::iterator i = running_.begin();
+       i != running_.end(); ++i)
+    kill(-(*i)->pid_, SIGINT);
+  for (vector<Subprocess*>::iterator i = running_.begin();
+       i != running_.end(); ++i)
+    delete *i;
+  running_.clear();
+}
index 9828bf4..5150eea 100644 (file)
@@ -22,25 +22,32 @@ using namespace std;
 
 #ifdef _WIN32
 #include <windows.h>
+#else
+#include <signal.h>
 #endif
 
+#include "exit_status.h"
+
 /// Subprocess wraps a single async subprocess.  It is entirely
 /// passive: it expects the caller to notify it when its fds are ready
 /// for reading, as well as call Finish() to reap the child once done()
 /// is true.
 struct Subprocess {
-  Subprocess();
   ~Subprocess();
-  bool Start(struct SubprocessSet* set, const string& command);
-  void OnPipeReady();
-  /// Returns true on successful process exit.
-  bool Finish();
+
+  /// Returns ExitSuccess on successful process exit, ExitInterrupted if
+  /// the process was interrupted, ExitFailure if it otherwise failed.
+  ExitStatus Finish();
 
   bool Done() const;
 
   const string& GetOutput() const;
 
  private:
+  Subprocess();
+  bool Start(struct SubprocessSet* set, const string& command);
+  void OnPipeReady();
+
   string buf_;
 
 #ifdef _WIN32
@@ -60,22 +67,30 @@ struct Subprocess {
   friend struct SubprocessSet;
 };
 
-/// SubprocessSet runs a poll() loop around a set of Subprocesses.
+/// SubprocessSet runs a pselect() loop around a set of Subprocesses.
 /// DoWork() waits for any state change in subprocesses; finished_
 /// is a queue of subprocesses as they finish.
 struct SubprocessSet {
   SubprocessSet();
   ~SubprocessSet();
 
-  void Add(Subprocess* subprocess);
-  void DoWork();
+  Subprocess *Add(const string &command);
+  bool DoWork();
   Subprocess* NextFinished();
+  void Clear();
 
   vector<Subprocess*> running_;
   queue<Subprocess*> finished_;
 
 #ifdef _WIN32
-  HANDLE ioport_;
+  static BOOL WINAPI NotifyInterrupted(DWORD dwCtrlType);
+  static HANDLE ioport_;
+#else
+  static void SetInterruptedFlag(int signum);
+  static bool interrupted_;
+
+  struct sigaction old_act_;
+  sigset_t old_mask_;
 #endif
 };
 
index 840287b..5b3e8a3 100644 (file)
@@ -32,46 +32,71 @@ struct SubprocessTest : public testing::Test {
 
 // Run a command that fails and emits to stderr.
 TEST_F(SubprocessTest, BadCommandStderr) {
-  Subprocess* subproc = new Subprocess;
-  EXPECT_TRUE(subproc->Start(&subprocs_, "cmd /c ninja_no_such_command"));
-  subprocs_.Add(subproc);
+  Subprocess* subproc = subprocs_.Add("cmd /c ninja_no_such_command");
+  ASSERT_NE((Subprocess *) 0, subproc);
 
   while (!subproc->Done()) {
     // Pretend we discovered that stderr was ready for writing.
     subprocs_.DoWork();
   }
 
-  EXPECT_FALSE(subproc->Finish());
+  EXPECT_EQ(ExitFailure, subproc->Finish());
   EXPECT_NE("", subproc->GetOutput());
 }
 
 // Run a command that does not exist
 TEST_F(SubprocessTest, NoSuchCommand) {
-  Subprocess* subproc = new Subprocess;
-  EXPECT_TRUE(subproc->Start(&subprocs_, "ninja_no_such_command"));
-  subprocs_.Add(subproc);
+  Subprocess* subproc = subprocs_.Add("ninja_no_such_command");
+  ASSERT_NE((Subprocess *) 0, subproc);
 
   while (!subproc->Done()) {
     // Pretend we discovered that stderr was ready for writing.
     subprocs_.DoWork();
   }
 
-  EXPECT_FALSE(subproc->Finish());
+  EXPECT_EQ(ExitFailure, subproc->Finish());
   EXPECT_NE("", subproc->GetOutput());
 #ifdef _WIN32
   ASSERT_EQ("CreateProcess failed: The system cannot find the file specified.\n", subproc->GetOutput());
 #endif
 }
 
+#ifndef _WIN32
+
+TEST_F(SubprocessTest, InterruptChild) {
+  Subprocess* subproc = subprocs_.Add("kill -INT $$");
+  ASSERT_NE((Subprocess *) 0, subproc);
+
+  while (!subproc->Done()) {
+    subprocs_.DoWork();
+  }
+
+  EXPECT_EQ(ExitInterrupted, subproc->Finish());
+}
+
+TEST_F(SubprocessTest, InterruptParent) {
+  Subprocess* subproc = subprocs_.Add("kill -INT $PPID ; sleep 1");
+  ASSERT_NE((Subprocess *) 0, subproc);
+
+  while (!subproc->Done()) {
+    bool interrupted = subprocs_.DoWork();
+    if (interrupted)
+      return;
+  }
+
+  ADD_FAILURE() << "We should have been interrupted";
+}
+
+#endif
+
 TEST_F(SubprocessTest, SetWithSingle) {
-  Subprocess* subproc = new Subprocess;
-  EXPECT_TRUE(subproc->Start(&subprocs_, kSimpleCommand));
-  subprocs_.Add(subproc);
+  Subprocess* subproc = subprocs_.Add(kSimpleCommand);
+  ASSERT_NE((Subprocess *) 0, subproc);
 
   while (!subproc->Done()) {
     subprocs_.DoWork();
   }
-  ASSERT_TRUE(subproc->Finish());
+  ASSERT_EQ(ExitSuccess, subproc->Finish());
   ASSERT_NE("", subproc->GetOutput());
 
   ASSERT_EQ(1u, subprocs_.finished_.size());
@@ -91,9 +116,8 @@ TEST_F(SubprocessTest, SetWithMulti) {
   };
 
   for (int i = 0; i < 3; ++i) {
-    processes[i] = new Subprocess;
-    EXPECT_TRUE(processes[i]->Start(&subprocs_, kCommands[i]));
-    subprocs_.Add(processes[i]);
+    processes[i] = subprocs_.Add(kCommands[i]);
+    ASSERT_NE((Subprocess *) 0, processes[i]);
   }
 
   ASSERT_EQ(3u, subprocs_.running_.size());
@@ -112,7 +136,7 @@ TEST_F(SubprocessTest, SetWithMulti) {
   ASSERT_EQ(3u, subprocs_.finished_.size());
 
   for (int i = 0; i < 3; ++i) {
-    ASSERT_TRUE(processes[i]->Finish());
+    ASSERT_EQ(ExitSuccess, processes[i]->Finish());
     ASSERT_NE("", processes[i]->GetOutput());
     delete processes[i];
   }