* Wrapper class around pids.

4 files changed, 162 insertions, 87 deletions

diff --git a/src/libstore/normalise.cc b/src/libstore/normalise.cc
index 5d68945b07..9aeea11914 100644
--- a/src/libstore/normalise.cc
+++ b/src/libstore/normalise.cc
@@ -3,11 +3,8 @@
 #include <boost/enable_shared_from_this.hpp>
 
 #include <sys/types.h>
-#include <sys/wait.h>
 #include <sys/stat.h>
-#include <sys/time.h>
 #include <fcntl.h>
-#include <signal.h>
 #include <unistd.h>
 
 #include "normalise.hh"
@@ -60,7 +57,7 @@ protected:
 
     virtual ~Goal()
     {
-        debug("goal destroyed");
+        printMsg(lvlVomit, "goal destroyed");
     }
 
 public:
@@ -160,26 +157,6 @@ public:
 //////////////////////////////////////////////////////////////////////
 
 
-void killChild(pid_t pid)
-{
-    /* Send a KILL signal to every process in the child process group
-       (which hopefully includes *all* its children). */
-    if (kill(-pid, SIGKILL) != 0)
-        printMsg(lvlError, format("killing process %1%") % pid);
-    else {
-        /* Wait until the child dies, disregarding the exit status. */
-        int status;
-        while (waitpid(pid, &status, 0) == -1)
-            if (errno != EINTR) printMsg(lvlError,
-                format("waiting for process %1%") % pid);
-    }
-}
-
-
-
-//////////////////////////////////////////////////////////////////////
-
-
 void Goal::addWaiter(GoalPtr waiter)
 {
     waiters.insert(waiter);
@@ -233,7 +210,7 @@ private:
     map<Path, Path> inputSucs;
 
     /* The process ID of the builder. */
-    pid_t pid;
+    Pid pid;
 
     /* The temporary directory. */
     Path tmpDir;
@@ -309,7 +286,6 @@ NormalisationGoal::NormalisationGoal(const Path & _nePath, Worker & _worker)
     : Goal(_worker)
 {
     nePath = _nePath;
-    pid = -1;
     state = &NormalisationGoal::init;
 }
 
@@ -318,13 +294,6 @@ NormalisationGoal::~NormalisationGoal()
 {
     /* Careful: we should never ever throw an exception from a
        destructor. */
-
-    if (pid != -1) {
-        printMsg(lvlError, format("killing child process %1% (%2%)")
-            % pid % nePath);
-        killChild(pid);
-    }
-
     try {
         deleteTmpDir(false);
     } catch (Error & e) {
@@ -471,20 +440,16 @@ void NormalisationGoal::buildDone()
 {
     debug(format("build done for `%1%'") % nePath);
 
-    int status;
-
     /* Since we got an EOF on the logger pipe, the builder is presumed
        to have terminated.  In fact, the builder could also have
        simply have closed its end of the pipe --- just don't do that
        :-) */
     /* !!! this could block! */
-    if (waitpid(pid, &status, 0) != pid)
-        throw SysError(format("builder for `%1%' should have terminated")
-            % nePath);
+    pid_t savedPid = pid;
+    int status = pid.wait(true);
 
     /* So the child is gone now. */
-    worker.childTerminated(pid);
-    pid = -1;
+    worker.childTerminated(savedPid);
 
     /* Close the read side of the logger pipe. */
     logPipe.readSide.close();
@@ -570,7 +535,8 @@ NormalisationGoal::HookReply NormalisationGoal::tryBuildHook()
     fromHook.create();
 
     /* Fork the hook. */
-    switch (pid = fork()) {
+    pid = fork();
+    switch (pid) {
         
     case -1:
         throw SysError("unable to fork");
@@ -678,11 +644,9 @@ void NormalisationGoal::terminateBuildHook()
 {
     /* !!! drain stdout of hook */
     debug("terminating build hook");
-    int status;
-    if (waitpid(pid, &status, 0) != pid)
-        printMsg(lvlError, format("process `%1%' missing") % pid);
-    worker.childTerminated(pid);
-    pid = -1;
+    pid_t savedPid = pid;
+    pid.wait(true);
+    worker.childTerminated(savedPid);
     fromHook.readSide.close();
     toHook.writeSide.close();
     fdLogFile.close();
@@ -836,7 +800,8 @@ void NormalisationGoal::startBuilder()
        currently use forks to run and wait for the children, it
        shouldn't be hard to use threads for this on systems where
        fork() is unavailable or inefficient. */
-    switch (pid = fork()) {
+    pid = fork();
+    switch (pid) {
 
     case -1:
         throw SysError("unable to fork");
@@ -885,6 +850,7 @@ void NormalisationGoal::startBuilder()
     }
 
     /* parent */
+    pid.setSeparatePG(true);
     logPipe.writeSide.close();
     worker.childStarted(shared_from_this(),
         pid, logPipe.readSide, true);
@@ -1199,7 +1165,7 @@ private:
     Pipe logPipe;
 
     /* The process ID of the builder. */
-    pid_t pid;
+    Pid pid;
 
     /* Lock on the store path. */
     PathLocks outputLock;
@@ -1209,7 +1175,6 @@ private:
 
 public:
     SubstitutionGoal(const Path & _nePath, Worker & _worker);
-    ~SubstitutionGoal();
 
     void work();
 
@@ -1227,22 +1192,10 @@ SubstitutionGoal::SubstitutionGoal(const Path & _storePath, Worker & _worker)
     : Goal(_worker)
 {
     storePath = _storePath;
-    pid = -1;
     state = &SubstitutionGoal::init;
 }
 
 
-SubstitutionGoal::~SubstitutionGoal()
-{
-    /* !!! turn this into a destructor for pids */
-    if (pid != -1) {
-        printMsg(lvlError, format("killing child process %1% (%2%)")
-            % pid % storePath);
-        killChild(pid);
-    }
-}
-
-
 void SubstitutionGoal::work()
 {
     (this->*state)();
@@ -1345,7 +1298,8 @@ void SubstitutionGoal::tryToRun()
         deletePath(storePath);
 
     /* Fork the substitute program. */
-    switch (pid = fork()) {
+    pid = fork();
+    switch (pid) {
         
     case -1:
         throw SysError("unable to fork");
@@ -1402,6 +1356,7 @@ void SubstitutionGoal::tryToRun()
     }
     
     /* parent */
+    pid.setSeparatePG(true);
     logPipe.writeSide.close();
     worker.childStarted(shared_from_this(),
         pid, logPipe.readSide, true);
@@ -1414,18 +1369,14 @@ void SubstitutionGoal::finished()
 {
     debug(format("substitute finished of `%1%'") % storePath);
 
-    int status;
-
     /* Since we got an EOF on the logger pipe, the substitute is
        presumed to have terminated.  */
     /* !!! this could block! */
-    if (waitpid(pid, &status, 0) != pid)
-        throw SysError(format("substitute for `%1%' should have terminated")
-            % storePath);
+    pid_t savedPid = pid;
+    int status = pid.wait(true);
 
     /* So the child is gone now. */
-    worker.childTerminated(pid);
-    pid = -1;
+    worker.childTerminated(savedPid);
 
     /* Close the read side of the logger pipe. */
     logPipe.readSide.close();
@@ -1534,7 +1485,7 @@ void Worker::removeGoal(GoalPtr goal)
 
 void Worker::wakeUp(GoalPtr goal)
 {
-    debug("wake up");
+    printMsg(lvlVomit, "wake up");
     awake.insert(goal);
 }
 
@@ -1593,8 +1544,6 @@ void Worker::run()
 
     while (1) {
 
-        debug(format("main loop (%1% goals left)") % goals.size());
-
         checkInterrupt();
 
         /* Call every wake goal. */
@@ -1602,7 +1551,8 @@ void Worker::run()
             Goals awake2(awake); /* !!! why is this necessary? */
             awake.clear();
             for (Goals::iterator i = awake2.begin(); i != awake2.end(); ++i) {
-                debug("goal");
+                printMsg(lvlVomit,
+                    format("running goal (%1% left)") % goals.size());
                 checkInterrupt();
                 GoalPtr goal = *i;
                 goal->work();
@@ -1714,5 +1664,7 @@ void ensurePath(const Path & path)
     /* If the path is already valid, we're done. */
     if (isValidPath(path)) return;
 
-    /* !!! add realisation goal */
+    Worker worker;
+    worker.addSubstitutionGoal(path, GoalPtr());
+    worker.run();
 }
diff --git a/src/libstore/store.cc b/src/libstore/store.cc
index 59d4430fd6..2ec93d63bc 100644
--- a/src/libstore/store.cc
+++ b/src/libstore/store.cc
@@ -127,21 +127,22 @@ void copyPath(const Path & src, const Path & dst)
        use a thread). */
 
     /* Create a pipe. */
-    int fds[2];
-    if (pipe(fds) == -1) throw SysError("creating pipe");
+    Pipe pipe;
+    pipe.create();
 
     /* Fork. */
-    pid_t pid;
-    switch (pid = fork()) {
+    Pid pid;
+    pid = fork();
+    switch (pid) {
 
     case -1:
         throw SysError("unable to fork");
 
     case 0: /* child */
         try {
-            close(fds[1]);
+            pipe.writeSide.close();
             CopySource source;
-            source.fd = fds[0];
+            source.fd = pipe.readSide;
             restorePath(dst, source);
             _exit(0);
         } catch (exception & e) {
@@ -150,19 +151,16 @@ void copyPath(const Path & src, const Path & dst)
         _exit(1);        
     }
 
-    close(fds[0]);
-    
     /* Parent. */
 
+    pipe.readSide.close();
+    
     CopySink sink;
-    sink.fd = fds[1];
+    sink.fd = pipe.writeSide;
     dumpPath(src, sink);
 
     /* Wait for the child to finish. */
-    int status;
-    if (waitpid(pid, &status, 0) != pid)
-        throw SysError("waiting for child");
-
+    int status = pid.wait(true);
     if (!WIFEXITED(status) || WEXITSTATUS(status) != 0)
         throw Error(format("cannot copy `%1% to `%2%': child %3%")
             % src % dst % statusToString(status));
diff --git a/src/libutil/util.cc b/src/libutil/util.cc
index d3446d38a6..d4345eac36 100644
--- a/src/libutil/util.cc
+++ b/src/libutil/util.cc
@@ -7,9 +7,11 @@
 
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/wait.h>
 #include <unistd.h>
 #include <dirent.h>
 #include <fcntl.h>
+#include <signal.h>
 
 #include "util.hh"
 
@@ -337,6 +339,10 @@ void writeFull(int fd, const unsigned char * buf, size_t count)
 }
 
 
+
+//////////////////////////////////////////////////////////////////////
+
+
 AutoDelete::AutoDelete(const string & p) : path(p)
 {
     del = true;
@@ -353,16 +359,22 @@ void AutoDelete::cancel()
 }
 
 
+
+//////////////////////////////////////////////////////////////////////
+
+
 AutoCloseFD::AutoCloseFD()
 {
     fd = -1;
 }
 
+
 AutoCloseFD::AutoCloseFD(int fd)
 {
     this->fd = fd;
 }
 
+
 AutoCloseFD::~AutoCloseFD()
 {
     try {
@@ -372,17 +384,20 @@ AutoCloseFD::~AutoCloseFD()
     }
 }
 
+
 void AutoCloseFD::operator =(int fd)
 {
     if (this->fd != fd) close();
     this->fd = fd;
 }
 
+
 AutoCloseFD::operator int()
 {
     return fd;
 }
 
+
 void AutoCloseFD::close()
 {
     if (fd != -1) {
@@ -393,6 +408,7 @@ void AutoCloseFD::close()
     }
 }
 
+
 bool AutoCloseFD::isOpen()
 {
     return fd != -1;
@@ -408,32 +424,119 @@ void Pipe::create()
 }
 
 
+
+//////////////////////////////////////////////////////////////////////
+
+
 AutoCloseDir::AutoCloseDir()
 {
     dir = 0;
 }
 
+
 AutoCloseDir::AutoCloseDir(DIR * dir)
 {
     this->dir = dir;
 }
 
+
 AutoCloseDir::~AutoCloseDir()
 {
     if (dir) closedir(dir);
 }
 
+
 void AutoCloseDir::operator =(DIR * dir)
 {
     this->dir = dir;
 }
 
+
 AutoCloseDir::operator DIR *()
 {
     return dir;
 }
 
 
+
+//////////////////////////////////////////////////////////////////////
+
+
+Pid::Pid()
+{
+    pid = -1;
+    separatePG = false;
+}
+
+
+Pid::~Pid()
+{
+    kill();
+}
+
+
+void Pid::operator =(pid_t pid)
+{
+    if (this->pid != pid) kill();
+    this->pid = pid;
+}
+
+
+Pid::operator pid_t()
+{
+    return pid;
+}
+
+
+void Pid::kill()
+{
+    if (pid == -1) return;
+    
+    printMsg(lvlError, format("killing child process %1%") % pid);
+
+    /* Send a KILL signal to the child.  If it has its own process
+       group, send the signal to every process in the child process
+       group (which hopefully includes *all* its children). */
+    if (::kill(separatePG ? -pid : pid, SIGKILL) != 0)
+        printMsg(lvlError, format("killing process %1%") % pid);
+    else {
+        /* Wait until the child dies, disregarding the exit status. */
+        int status;
+        while (waitpid(pid, &status, 0) == -1)
+            if (errno != EINTR) printMsg(lvlError,
+                format("waiting for process %1%") % pid);
+    }
+
+    pid = -1;
+}
+
+
+int Pid::wait(bool block)
+{
+    while (1) {
+        int status;
+        int res = waitpid(pid, &status, block ? 0 : WNOHANG);
+        if (res == pid) {
+            pid = -1;
+            return status;
+        }
+        if (res == 0 && !block) return -1;
+        if (errno != EINTR)
+            throw SysError("cannot get child exit status");
+    }
+}
+
+
+void Pid::setSeparatePG(bool separatePG)
+{
+    this->separatePG = separatePG;
+}
+
+
+
+//////////////////////////////////////////////////////////////////////
+
+
 volatile sig_atomic_t _isInterrupted = 0;
 
 void _interrupted()
@@ -448,6 +551,10 @@ void _interrupted()
 }
 
 
+
+//////////////////////////////////////////////////////////////////////
+
+
 string packStrings(const Strings & strings)
 {
     string d;
diff --git a/src/libutil/util.hh b/src/libutil/util.hh
index 21c6774b9d..67661eb5f8 100644
--- a/src/libutil/util.hh
+++ b/src/libutil/util.hh
@@ -172,6 +172,7 @@ public:
     void cancel();
 };
 
+
 class AutoCloseFD
 {
     int fd;
@@ -185,6 +186,7 @@ public:
     bool isOpen();
 };
 
+
 class Pipe
 {
 public:
@@ -192,6 +194,7 @@ public:
     void create();
 };
 
+
 class AutoCloseDir
 {
     DIR * dir;
@@ -204,6 +207,21 @@ public:
 };
 
 
+class Pid
+{
+    pid_t pid;
+    bool separatePG;
+public:
+    Pid();
+    ~Pid();
+    void operator =(pid_t pid);
+    operator pid_t();
+    void kill();
+    int wait(bool block);
+    void setSeparatePG(bool separatePG);
+};
+
+
 /* User interruption. */
 
 extern volatile sig_atomic_t _isInterrupted;