From 9bf0d11e138cec3a346ce5d04d2ba41b96cd2b35 Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Sat, 15 Feb 2014 11:10:08 -0500
Subject: [PATCH 01/20] Client-side handling of recursive nix

If NIX_REMOTE=recursive, the client checks
NIX_REMOTE_RECURSIVE_PROTOCOL_VERSION for an integer version. If it is
set and its major version matches the client major version (current
protocol version 0x101), it then reads a connected unix domain socket
file descriptor from NIX_REMOTE_RECURSIVE_FD. It sends a message
containing the client protocol version in the message data and a
connected unix domain socket in the ancillary data, and the other end of
that socket is then used for the normal daemon protocol.
---
 src/libstore/remote-store.cc    | 57 +++++++++++++++++++++++++++++++--
 src/libstore/worker-protocol.hh |  1 +
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index 46192069329..aee53bce8d6 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -16,6 +16,13 @@
 #include <unistd.h>
 #include <cstring>
 
+extern "C" {
+    struct ancillary {
+        struct cmsghdr hdr;
+        int fd;
+    };
+}
+
 namespace nix {
 
 
@@ -54,9 +61,56 @@ void RemoteStore::openConnection(bool reserveSpace)
         /* Connect to a daemon that does the privileged work for
            us. */
         connectToDaemon();
-    else
+    else if (remoteMode == "recursive") {
+        unsigned int daemonRecursiveVersion;
+        if (!string2Int(getEnv("NIX_REMOTE_RECURSIVE_PROTOCOL_VERSION"), daemonRecursiveVersion))
+            throw Error("Expected an integer in NIX_REMOTE_RECURSIVE_PROTOCOL_VERSION");
+
+        if (GET_PROTOCOL_MAJOR(daemonRecursiveVersion) != GET_PROTOCOL_MAJOR(RECURSIVE_PROTOCOL_VERSION))
+            throw Error("nix daemon recursive protocol version not supported");
+
+        int sfd;
+        if (!string2Int(getEnv("NIX_REMOTE_RECURSIVE_FD"), sfd))
+            throw Error("Expected an integer in NIX_REMOTE_RECURSIVE_FD");
+
+        int fds[2];
+        if (socketpair(PF_UNIX, SOCK_STREAM, 0, fds) == -1)
+            throw SysError("creating recursive daemon socketpair");
+
+        /* Send our protocol version and a socket to the daemon */
+        unsigned char buf[sizeof RECURSIVE_PROTOCOL_VERSION];
+        for (size_t byte = 0; byte < sizeof buf; ++byte)
+            buf[sizeof buf - (byte + 1)] =
+                (unsigned char) ((RECURSIVE_PROTOCOL_VERSION >> (8 * byte)) & 0xFF);
+        struct iovec iov;
+        iov.iov_base = buf;
+        iov.iov_len = sizeof buf;
+        struct msghdr msg;
+        memset(&msg, 0, sizeof msg);
+        msg.msg_iov = &iov;
+        msg.msg_iovlen = 1;
+        ancillary data;
+        msg.msg_control = &data;
+        msg.msg_controllen = sizeof data;
+        struct cmsghdr *cmsg = CMSG_FIRSTHDR(&msg);
+        cmsg->cmsg_len = msg.msg_controllen;
+        cmsg->cmsg_level = SOL_SOCKET;
+        cmsg->cmsg_type = SCM_RIGHTS;
+        memmove(CMSG_DATA(cmsg), &fds[1], sizeof fds[1]);
+        ssize_t count = sendmsg(sfd, &msg, 0);
+        if (count == -1)
+            throw SysError("sending socket descriptor to daemon");
+        else if ((size_t) count != iov.iov_len)
+            throw Error(format("Tried to send %1% bytes to the daemon, but only %2% were sent")
+                    % iov.iov_len % count);
+        close(fds[1]);
+
+        fdSocket = fds[0];
+    } else
         throw Error(format("invalid setting for NIX_REMOTE, `%1%'") % remoteMode);
 
+    closeOnExec(fdSocket);
+
     from.fd = fdSocket;
     to.fd = fdSocket;
 
@@ -100,7 +154,6 @@ void RemoteStore::connectToDaemon()
     fdSocket = socket(PF_UNIX, SOCK_STREAM, 0);
     if (fdSocket == -1)
         throw SysError("cannot create Unix domain socket");
-    closeOnExec(fdSocket);
 
     string socketPath = settings.nixDaemonSocketFile;
 
diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh
index 9317f89c376..ab8ed5d2264 100644
--- a/src/libstore/worker-protocol.hh
+++ b/src/libstore/worker-protocol.hh
@@ -7,6 +7,7 @@ namespace nix {
 #define WORKER_MAGIC_2 0x6478696f
 
 #define PROTOCOL_VERSION 0x10e
+#define RECURSIVE_PROTOCOL_VERSION 0x101
 #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00)
 #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff)
 

From 0490a561f8556180fa21d9adb7fc63a1618b8469 Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Sun, 16 Feb 2014 20:20:13 -0500
Subject: [PATCH 02/20] Recursive nix client side: Report new paths

With recursive nix, it is possible for a build to legally gain access to
paths that weren't specified as its inputs (indeed, that is the whole
point). In order for the nix process that started the build to be able
to scan for dependencies properly, it must be told when these new paths
are (potentially) accessed.

This amends version 0x101 of the recursive protocol to put the socket fd
into NIX_REMOTE_RECURSIVE_SOCKET_FD and to put another fd into
NIX_REMOTE_RECURSIVE_PATHS_FD. This new fd is assumed to point to a new
kind of temporary gc root, where the root is kept from being considered
invalid by the parent nix process staying alive and thus only a write
lock is needed to avoid a race between the gc reading the root file and
a process adding to it. Upcoming commits should clarify this if it's
confusing.
---
 src/libstore/remote-store.cc | 78 ++++++++++++++++++++++++++++++------
 src/libstore/remote-store.hh |  2 +
 2 files changed, 67 insertions(+), 13 deletions(-)

diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index aee53bce8d6..5bcf7627cdc 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -5,6 +5,7 @@
 #include "archive.hh"
 #include "affinity.hh"
 #include "globals.hh"
+#include "pathlocks.hh"
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -64,14 +65,14 @@ void RemoteStore::openConnection(bool reserveSpace)
     else if (remoteMode == "recursive") {
         unsigned int daemonRecursiveVersion;
         if (!string2Int(getEnv("NIX_REMOTE_RECURSIVE_PROTOCOL_VERSION"), daemonRecursiveVersion))
-            throw Error("Expected an integer in NIX_REMOTE_RECURSIVE_PROTOCOL_VERSION");
+            throw Error("expected an integer in NIX_REMOTE_RECURSIVE_PROTOCOL_VERSION");
 
         if (GET_PROTOCOL_MAJOR(daemonRecursiveVersion) != GET_PROTOCOL_MAJOR(RECURSIVE_PROTOCOL_VERSION))
             throw Error("nix daemon recursive protocol version not supported");
 
         int sfd;
-        if (!string2Int(getEnv("NIX_REMOTE_RECURSIVE_FD"), sfd))
-            throw Error("Expected an integer in NIX_REMOTE_RECURSIVE_FD");
+        if (!string2Int(getEnv("NIX_REMOTE_RECURSIVE_SOCKET_FD"), sfd))
+            throw Error("expected an fd in NIX_REMOTE_RECURSIVE_SOCKET_FD");
 
         int fds[2];
         if (socketpair(PF_UNIX, SOCK_STREAM, 0, fds) == -1)
@@ -106,6 +107,11 @@ void RemoteStore::openConnection(bool reserveSpace)
         close(fds[1]);
 
         fdSocket = fds[0];
+
+        int pathsFd;
+        if (!string2Int(getEnv("NIX_REMOTE_RECURSIVE_PATHS_FD"), pathsFd))
+            throw Error("expected an fd in NIX_REMOTE_RECURSIVE_PATHS_FD");
+        fdRecursivePaths = pathsFd;
     } else
         throw Error(format("invalid setting for NIX_REMOTE, `%1%'") % remoteMode);
 
@@ -184,6 +190,17 @@ void RemoteStore::connectToDaemon()
 RemoteStore::~RemoteStore()
 {
     try {
+        /* As long as we do this *before* closing the fdSocket, the daemon
+           should keep our paths alive */
+        if (fdRecursivePaths != -1 && !recursivePaths.empty()) {
+            string s = "";
+            foreach (PathSet::const_iterator, i, recursivePaths) {
+                s += *i + '\0';
+            }
+            lockFile(fdRecursivePaths, ltWrite, true);
+            writeFull(fdRecursivePaths, (const unsigned char *) s.data(), s.size());
+            lockFile(fdRecursivePaths, ltNone, true);
+        }
         to.flush();
         fdSocket.close();
         if (child != -1)
@@ -262,7 +279,9 @@ PathSet RemoteStore::queryAllValidPaths()
     openConnection();
     writeInt(wopQueryAllValidPaths, to);
     processStderr();
-    return readStorePaths<PathSet>(from);
+    PathSet res = readStorePaths<PathSet>(from);
+    recursivePaths.insert(res.begin(), res.end());
+    return res;
 }
 
 
@@ -306,8 +325,12 @@ void RemoteStore::querySubstitutablePathInfos(const PathSet & paths,
             unsigned int reply = readInt(from);
             if (reply == 0) continue;
             info.deriver = readString(from);
-            if (info.deriver != "") assertStorePath(info.deriver);
+            if (info.deriver != "") {
+                assertStorePath(info.deriver);
+                recursivePaths.insert(info.deriver);
+            }
             info.references = readStorePaths<PathSet>(from);
+            recursivePaths.insert(info.references.begin(), info.references.end());
             info.downloadSize = readLongLong(from);
             info.narSize = GET_PROTOCOL_MINOR(daemonVersion) >= 7 ? readLongLong(from) : 0;
             infos[*i] = info;
@@ -323,8 +346,12 @@ void RemoteStore::querySubstitutablePathInfos(const PathSet & paths,
             Path path = readStorePath(from);
             SubstitutablePathInfo & info(infos[path]);
             info.deriver = readString(from);
-            if (info.deriver != "") assertStorePath(info.deriver);
+            if (info.deriver != "") {
+                assertStorePath(info.deriver);
+                recursivePaths.insert(info.deriver);
+            }
             info.references = readStorePaths<PathSet>(from);
+            recursivePaths.insert(info.references.begin(), info.references.end());
             info.downloadSize = readLongLong(from);
             info.narSize = readLongLong(from);
         }
@@ -342,9 +369,13 @@ ValidPathInfo RemoteStore::queryPathInfo(const Path & path)
     ValidPathInfo info;
     info.path = path;
     info.deriver = readString(from);
-    if (info.deriver != "") assertStorePath(info.deriver);
+    if (info.deriver != "") {
+        assertStorePath(info.deriver);
+        recursivePaths.insert(info.deriver);
+    }
     info.hash = parseHash(htSHA256, readString(from));
     info.references = readStorePaths<PathSet>(from);
+    recursivePaths.insert(info.references.begin(), info.references.end());
     info.registrationTime = readInt(from);
     info.narSize = readLongLong(from);
     return info;
@@ -370,6 +401,7 @@ void RemoteStore::queryReferences(const Path & path,
     writeString(path, to);
     processStderr();
     PathSet references2 = readStorePaths<PathSet>(from);
+    recursivePaths.insert(references2.begin(), references2.end());
     references.insert(references2.begin(), references2.end());
 }
 
@@ -382,6 +414,7 @@ void RemoteStore::queryReferrers(const Path & path,
     writeString(path, to);
     processStderr();
     PathSet referrers2 = readStorePaths<PathSet>(from);
+    /* no need to add to recursive paths, they're brought in automatically */
     referrers.insert(referrers2.begin(), referrers2.end());
 }
 
@@ -393,7 +426,10 @@ Path RemoteStore::queryDeriver(const Path & path)
     writeString(path, to);
     processStderr();
     Path drvPath = readString(from);
-    if (drvPath != "") assertStorePath(drvPath);
+    if (drvPath != "") {
+        assertStorePath(drvPath);
+        recursivePaths.insert(drvPath);
+    }
     return drvPath;
 }
 
@@ -404,7 +440,9 @@ PathSet RemoteStore::queryValidDerivers(const Path & path)
     writeInt(wopQueryValidDerivers, to);
     writeString(path, to);
     processStderr();
-    return readStorePaths<PathSet>(from);
+    PathSet res = readStorePaths<PathSet>(from);
+    recursivePaths.insert(res.begin(), res.end());
+    return res;
 }
 
 
@@ -414,6 +452,10 @@ PathSet RemoteStore::queryDerivationOutputs(const Path & path)
     writeInt(wopQueryDerivationOutputs, to);
     writeString(path, to);
     processStderr();
+    /* derivations are treated specially: Unless they became accessible
+       via unsafeDiscardOutputDependency, their outputs are treated as
+       inputs to scan for even if, in the case of recursive nix, they
+       may not be valid. So no need to add them to recursivePaths */
     return readStorePaths<PathSet>(from);
 }
 
@@ -435,7 +477,10 @@ Path RemoteStore::queryPathFromHashPart(const string & hashPart)
     writeString(hashPart, to);
     processStderr();
     Path path = readString(from);
-    if (!path.empty()) assertStorePath(path);
+    if (!path.empty()) {
+        assertStorePath(path);
+        recursivePaths.insert(path);
+    }
     return path;
 }
 
@@ -457,7 +502,9 @@ Path RemoteStore::addToStore(const Path & _srcPath,
     writeString(printHashType(hashAlgo), to);
     dumpPath(srcPath, to, filter);
     processStderr();
-    return readStorePath(from);
+    Path res = readStorePath(from);
+    recursivePaths.insert(res);
+    return res;
 }
 
 
@@ -473,7 +520,9 @@ Path RemoteStore::addTextToStore(const string & name, const string & s,
     writeStrings(references, to);
 
     processStderr();
-    return readStorePath(from);
+    Path res = readStorePath(from);
+    recursivePaths.insert(res);
+    return res;
 }
 
 
@@ -496,7 +545,9 @@ Paths RemoteStore::importPaths(bool requireSignature, Source & source)
     /* We ignore requireSignature, since the worker forces it to true
        anyway. */
     processStderr(0, &source);
-    return readStorePaths<Paths>(from);
+    Paths res = readStorePaths<Paths>(from);
+    recursivePaths.insert(res.begin(), res.end());
+    return res;
 }
 
 
@@ -569,6 +620,7 @@ Roots RemoteStore::findRoots()
     while (count--) {
         Path link = readString(from);
         Path target = readStorePath(from);
+        recursivePaths.insert(target);
         result[link] = target;
     }
     return result;
diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh
index 04b60fce4b4..a7d84266c88 100644
--- a/src/libstore/remote-store.hh
+++ b/src/libstore/remote-store.hh
@@ -90,6 +90,8 @@ private:
     Pid child;
     unsigned int daemonVersion;
     bool initialised;
+    AutoCloseFD fdRecursivePaths;
+    PathSet recursivePaths;
 
     void openConnection(bool reserveSpace = true);
 

From e259ce8e590b16cc4ff8702ae05c0f761a617806 Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Sun, 16 Feb 2014 21:06:42 -0500
Subject: [PATCH 03/20] nix-daemon handling of recursive nix

nix-daemon now supports listening on an already-connected socket. In the
normal case, nix-daemon will listen on its traditional socket (passed
through systemd or not) and makes a new socketpair for listening for the
new kind of connection. If, however, nix-daemon was started with
NIX_RECURSIVE_FDS set, it uses the two fds listed in that env var as the
two ends of the connected socket and doesn't listen on the traditional
socket at all. In a future commit, LocalStores that were *not* launched
from a nix-daemon child process will launch nix-daemon with that env var
properly set if they need to perform a build.

In either case, nix-daemon now has a connected socket to read from and
possibly an unconnected socket it's listening to to connect to. When the
connected socket is readable, it reads the client recursive version from
the message data and the client domain socket from the ancillary data to
form a new connection. If the listening socket is connectable, a
connection is formed as usual.

Finally, when a child process is launched, its LocalStore gets passed
the other side of the connected socket so it can use the existing
nix-daemon process for recursive nix instead of having to spawn a new
nix-daemon process every time (to be implemented in a future commit)
---
 src/libstore/local-store.cc     |   3 +-
 src/libstore/local-store.hh     |   4 +-
 src/libstore/remote-store.cc    |   7 --
 src/libstore/worker-protocol.hh |   8 +++
 src/libutil/util.cc             |   9 +++
 src/libutil/util.hh             |   3 +
 src/nix-daemon/nix-daemon.cc    | 116 +++++++++++++++++++++++++++-----
 7 files changed, 126 insertions(+), 24 deletions(-)

diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index aca98412ae1..bb07453b916 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -222,8 +222,9 @@ void checkStoreNotSymlink()
 }
 
 
-LocalStore::LocalStore(bool reserveSpace)
+LocalStore::LocalStore(bool reserveSpace, int fd)
     : didSetSubstituterEnv(false)
+    , fdRecursiveDaemon(fd)
 {
     schemaPath = settings.nixDBPath + "/schema";
 
diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index 09639e74cf4..5ca0851b1f5 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -93,7 +93,7 @@ public:
 
     /* Initialise the local store, upgrading the schema if
        necessary. */
-    LocalStore(bool reserveSpace = true);
+    LocalStore(bool reserveSpace = true, int fd = -1);
 
     ~LocalStore();
 
@@ -308,6 +308,8 @@ private:
     // Internal versions that are not wrapped in retry_sqlite.
     bool isValidPath_(const Path & path);
     void queryReferrers_(const Path & path, PathSet & referrers);
+
+    AutoCloseFD fdRecursiveDaemon;
 };
 
 
diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index 5bcf7627cdc..41868baeafb 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -17,13 +17,6 @@
 #include <unistd.h>
 #include <cstring>
 
-extern "C" {
-    struct ancillary {
-        struct cmsghdr hdr;
-        int fd;
-    };
-}
-
 namespace nix {
 
 
diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh
index ab8ed5d2264..538056bec16 100644
--- a/src/libstore/worker-protocol.hh
+++ b/src/libstore/worker-protocol.hh
@@ -1,4 +1,12 @@
 #pragma once
+#include <sys/socket.h>
+
+extern "C" {
+    struct ancillary {
+        struct cmsghdr hdr;
+        int fd;
+    };
+}
 
 namespace nix {
 
diff --git a/src/libutil/util.cc b/src/libutil/util.cc
index 740d767a4ea..c50dcf07519 100644
--- a/src/libutil/util.cc
+++ b/src/libutil/util.cc
@@ -921,6 +921,15 @@ void closeOnExec(int fd)
 }
 
 
+void setNonBlocking(int fd)
+{
+    int prev;
+    if ((prev = fcntl(fd, F_GETFL, 0)) == -1 ||
+        fcntl(fd, F_SETFL, prev | O_NONBLOCK) == -1)
+        throw SysError("setting non-blocking flag");
+}
+
+
 #if HAVE_VFORK
 pid_t (*maybeVfork)() = vfork;
 #else
diff --git a/src/libutil/util.hh b/src/libutil/util.hh
index 0351220c2a2..ca88957ec1d 100644
--- a/src/libutil/util.hh
+++ b/src/libutil/util.hh
@@ -261,6 +261,9 @@ void closeMostFDs(const set<int> & exceptions);
 /* Set the close-on-exec flag for the given file descriptor. */
 void closeOnExec(int fd);
 
+/* Put the given file descriptor into non-blocking mode. */
+void setNonBlocking(int fd);
+
 /* Call vfork() if available, otherwise fork(). */
 extern pid_t (*maybeVfork)();
 
diff --git a/src/nix-daemon/nix-daemon.cc b/src/nix-daemon/nix-daemon.cc
index f40cdd51b4b..c76d30fc768 100644
--- a/src/nix-daemon/nix-daemon.cc
+++ b/src/nix-daemon/nix-daemon.cc
@@ -42,6 +42,7 @@ using namespace nix;
 
 static FdSource from(STDIN_FILENO);
 static FdSink to(STDOUT_FILENO);
+static AutoCloseFD fdRecursiveTo;
 
 bool canSendStderr;
 pid_t myPid;
@@ -695,8 +696,8 @@ static void processConnection(bool trusted)
             throw Error("if you run `nix-daemon' as root, then you MUST set `build-users-group'!");
 #endif
 
-        /* Open the store. */
-        store = boost::shared_ptr<StoreAPI>(new LocalStore(reserveSpace));
+        /* Open the store, with this process's parent socket used for recursive nix */
+        store = boost::shared_ptr<StoreAPI>(new LocalStore(reserveSpace, fdRecursiveTo.borrow()));
 
         stopWork();
         to.flush();
@@ -776,6 +777,7 @@ static void daemonLoop()
 
     AutoCloseFD fdSocket;
 
+    AutoCloseFD fdRecursiveFrom;
     /* Handle socket-based activation by systemd. */
     if (getEnv("LISTEN_FDS") != "") {
         if (getEnv("LISTEN_PID") != int2String(getpid()) || getEnv("LISTEN_FDS") != "1")
@@ -783,10 +785,24 @@ static void daemonLoop()
         fdSocket = SD_LISTEN_FDS_START;
     }
 
+    /* Handle nix-daemon run as a child of build.cc for recursive nix */
+    else if (getEnv("NIX_RECURSIVE_FDS") != "") {
+        Strings tokenized = tokenizeString<Strings>(getEnv("NIX_RECURSIVE_FDS"));
+        string first = tokenized.front();
+        int fd;
+        if (!string2Int(first, fd))
+            throw Error(format("invalid value for NIX_RECURSIVE_FDS, `%1%'") % getEnv("NIX_RECURSIVE_FDS"));
+        fdRecursiveFrom = fd;
+
+        tokenized.pop_front();
+        string second = tokenized.front();
+        if (!string2Int(second, fd))
+            throw Error(format("invalid value for NIX_RECURSIVE_FDS, `%1%'") % getEnv("NIX_RECURSIVE_FDS"));
+        fdRecursiveTo = fd;
+    }
+
     /* Otherwise, create and bind to a Unix domain socket. */
     else {
-
-        /* Create and bind to a Unix domain socket. */
         fdSocket = socket(PF_UNIX, SOCK_STREAM, 0);
         if (fdSocket == -1)
             throw SysError("cannot create Unix domain socket");
@@ -824,7 +840,24 @@ static void daemonLoop()
             throw SysError(format("cannot listen on socket `%1%'") % socketPath);
     }
 
-    closeOnExec(fdSocket);
+    if (fdRecursiveFrom == -1) {
+        int fds[2];
+        if (socketpair(PF_UNIX, SOCK_STREAM, 0, fds) == -1)
+            throw SysError("creating recursive daemon socketpair");
+        fdRecursiveFrom = fds[0];
+        fdRecursiveTo = fds[1];
+        if (shutdown(fdRecursiveFrom, SHUT_WR) == -1)
+            throw SysError("disabling writes to fdRecursiveFrom");
+        if (shutdown(fdRecursiveTo, SHUT_RD) == -1)
+            throw SysError("disabling reads from fdRecursiveTo");
+    }
+
+    closeOnExec(fdRecursiveFrom);
+
+    if (fdSocket != -1) {
+        setNonBlocking(fdSocket);
+        closeOnExec(fdSocket);
+    }
 
     /* Loop accepting connections. */
     while (1) {
@@ -834,18 +867,71 @@ static void daemonLoop()
                database, because it doesn't like forks very much. */
             assert(!store);
 
-            /* Accept a connection. */
-            struct sockaddr_un remoteAddr;
-            socklen_t remoteAddrLen = sizeof(remoteAddr);
+            fd_set set;
+            FD_ZERO(&set);
+            FD_SET(fdRecursiveFrom, &set);
+            /* Sigh, POSIX requires valid fds for FD_SET/FD_ISSET */
+            if (fdSocket != -1)
+                FD_SET(fdSocket, &set);
+            int nfds =
+                ((fdSocket > fdRecursiveFrom) ? fdSocket : fdRecursiveFrom) + 1;
 
-            AutoCloseFD remote = accept(fdSocket,
-                (struct sockaddr *) &remoteAddr, &remoteAddrLen);
+            int res = select(nfds, &set, 0, 0, 0);
             checkInterrupt();
-            if (remote == -1) {
-                if (errno == EINTR)
-                    continue;
-                else
-                    throw SysError("accepting connection");
+            if (res == -1) {
+                if (errno == EINTR) continue;
+                throw SysError("waiting for a new connection");
+            }
+
+            AutoCloseFD remote;
+            if ((fdSocket != -1) && FD_ISSET(fdSocket, &set)) {
+                /* Accept a connection. */
+                struct sockaddr_un remoteAddr;
+                socklen_t remoteAddrLen = sizeof(remoteAddr);
+
+                remote = accept(fdSocket,
+                    (struct sockaddr *) &remoteAddr, &remoteAddrLen);
+                checkInterrupt();
+                if (remote == -1) {
+                    if (errno == EINTR || errno == EWOULDBLOCK || errno == EAGAIN)
+                        continue;
+                    else
+                        throw SysError("accepting connection");
+                }
+
+            } else {
+                /* Lots of connections on fdSocket could theoretically starve
+                   the recursive socket, oh well */
+                assert(FD_ISSET(fdRecursiveFrom, &set));
+
+                /* Get the socket from the other end */
+                int fd;
+                char buf[sizeof RECURSIVE_PROTOCOL_VERSION];
+                struct iovec iov;
+                iov.iov_base = buf;
+                iov.iov_len = sizeof buf;
+                struct msghdr msg;
+                memset(&msg, 0, sizeof msg);
+                msg.msg_iov = &iov;
+                msg.msg_iovlen = 1;
+                ancillary data;
+                msg.msg_control = &data;
+                msg.msg_controllen = sizeof data;
+                ssize_t count = recvmsg(fdRecursiveFrom, &msg, 0);
+                if (count == -1)
+                    throw SysError("recieving socket descriptor from client");
+                else if (((size_t) count) < sizeof buf)
+                    throw Error(format("Expected to receive %1% bytes from client, got %2%") % sizeof buf % count);
+
+                memmove(&fd, CMSG_DATA(CMSG_FIRSTHDR(&msg)), sizeof fd);
+
+                remote = fd;
+
+                unsigned int recursiveClientVersion = 0;
+                for (size_t byte = 0; byte < sizeof RECURSIVE_PROTOCOL_VERSION; ++byte)
+                    recursiveClientVersion += ((int) buf[byte]) << (8 * byte);
+                if (GET_PROTOCOL_MAJOR(recursiveClientVersion) != GET_PROTOCOL_MAJOR(RECURSIVE_PROTOCOL_VERSION))
+                    throw Error("nix client recursive protocol version not supported");
             }
 
             closeOnExec(remote);

From 5123a5c96f2e2b4fbdf8a282b36a98288ec3c293 Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Mon, 17 Feb 2014 15:00:36 -0500
Subject: [PATCH 04/20] Replace per-pid temproots file with per-pid temproots
 dir

This will allow one process to keep several temproots files alive, as is
required for recursive nix.

The semantics are as follows:

* When creating its first temproot file, a process takes the gc lock,
  creates /nix/var/nix/temproots/{pid} directory (deleting it if it
  exits), takes a read lock on /nix/var/nix/temproots/{pid}/lock, and
  releases the gc lock
* Before adding a new temproot file (including the first one), the
  process upgrades its read lock on /nix/var/nix/temproots/{pid}/lock to
  a write lock. It can then make as many temproot files in
  /nix/var/nix/temproots/{pid}/lock as it likes, so long as they are not
  named "lock", and then it downgrades the write lock to a read lock
* Before adding a new root to a temproot file, the process takes a write
  lock on the file. Afterwards, it releases the lock. The locking
  process *never* holds a read lock on individual temproot files
* For each directory in /nix/var/nix/temproots, the gc first tries to
  get a write lock on /nix/var/nix/temproots/{dirname}/lock. If it
  succeeds, or if the lock file does not exist, the directory must be
  stale so it deletes it and moves on. If it fails, it waits until it
  can get a read lock on /nix/var/nix/temproots/{dirname}/lock (after
  which the owning process can't create more temproot files until
  released) and then iterates through every file not named "lock" in
  /nix/var/nix/tmproots/{dirname}, taking a read lock on the file then
  reading roots out of it as before. The gc does not release any of the
  read locks it takes here until collection is done.

For now, local-store uses /nix/var/nix/temproots/{pid}/main for its
temproots file, but future commits will also put recursive nix temproots
there.
---
 src/libstore/gc.cc | 188 +++++++++++++++++++++++++++++----------------
 1 file changed, 123 insertions(+), 65 deletions(-)

diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index 79bd7d56b3f..454ec2d212b 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -147,69 +147,105 @@ Path addPermRoot(StoreAPI & store, const Path & _storePath,
 
 
 /* The file to which we write our temporary roots. */
-static Path fnTempRoots;
-static AutoCloseFD fdTempRoots;
-
+static Path dnTempRoots;
+static AutoCloseFD fdTempRootsDirLock;
+static AutoCloseFD fdTempRootsMain;
 
 void LocalStore::addTempRoot(const Path & path)
 {
-    /* Create the temporary roots file for this process. */
-    if (fdTempRoots == -1) {
+    /* Create the temporary roots directory for this process. */
+    if (fdTempRootsDirLock == -1) {
+        Path dir = (format("%1%/%2%") % settings.nixStateDir % tempRootsDir).str();
+        createDirs(dir);
 
-        while (1) {
-            Path dir = (format("%1%/%2%") % settings.nixStateDir % tempRootsDir).str();
-            createDirs(dir);
+        dnTempRoots = (format("%1%/%2%")
+            % dir % getpid()).str();
 
-            fnTempRoots = (format("%1%/%2%")
-                % dir % getpid()).str();
+        Path fnTempRootsDirLock = (format("%1%/lock") % dnTempRoots).str();
 
-            AutoCloseFD fdGCLock = openGCLock(ltRead);
+        AutoCloseFD fdGCLock = openGCLock(ltRead);
 
-            if (pathExists(fnTempRoots))
-                /* It *must* be stale, since there can be no two
-                   processes with the same pid. */
-                unlink(fnTempRoots.c_str());
+        if (pathExists(dnTempRoots))
+            /* It *must* be stale, since there can be no two
+               processes with the same pid. Since we're in the gclock,
+               we're the only process who can be deleting this right now */
+            deletePath(dnTempRoots);
 
-            fdTempRoots = openLockFile(fnTempRoots, true);
+        if (mkdir(dnTempRoots.c_str(), 0700) == -1)
+            throw SysError("creating temporary roots directory");
 
-            fdGCLock.close();
+        fdTempRootsDirLock = openLockFile(fnTempRootsDirLock, true);
 
-            debug(format("acquiring read lock on `%1%'") % fnTempRoots);
-            lockFile(fdTempRoots, ltRead, true);
+        /* We need to take this lock before releasing the gc lock so
+           that the gc doesn't delete the directory */
+        debug(format("acquiring read lock on `%1%/lock'") % dnTempRoots);
+        lockFile(fdTempRootsDirLock, ltRead, true);
 
-            /* Check whether the garbage collector didn't get in our
-               way. */
-            struct stat st;
-            if (fstat(fdTempRoots, &st) == -1)
-                throw SysError(format("statting `%1%'") % fnTempRoots);
-            if (st.st_size == 0) break;
+        fdGCLock.close();
+    }
 
-            /* The garbage collector deleted this file before we could
-               get a lock.  (It won't delete the file after we get a
-               lock.)  Try again. */
-        }
+    /* Create the main temporary roots file for this process. */
+    if (fdTempRootsMain == -1) {
+        /* Upgrade the dir lock to a write lock.  This will cause us to block
+           if the garbage collector is holding our lock. */
+        debug(format("acquiring write lock on `%1%/lock'") % dnTempRoots);
+        lockFile(fdTempRootsDirLock, ltWrite, true);
+
+        Path fnMainTempRoots = (format("%1%/main") % dnTempRoots).str();
+
+        fdTempRootsMain = openLockFile(fnMainTempRoots, true);
+
+        /* Downgrade to a read lock. */
+        debug(format("downgrading to read lock on `%1%/lock'") % dnTempRoots);
+        lockFile(fdTempRootsDirLock, ltRead, true);
 
     }
 
-    /* Upgrade the lock to a write lock.  This will cause us to block
-       if the garbage collector is holding our lock. */
-    debug(format("acquiring write lock on `%1%'") % fnTempRoots);
-    lockFile(fdTempRoots, ltWrite, true);
+    /* Get a write lock on the main temp roots file so we don't write
+       to it after the gc has already started reading from it but before
+       gc has finished */
+    debug(format("acquiring write lock on `%1%/main'") % dnTempRoots);
+    lockFile(fdTempRootsMain, ltWrite, true);
 
     string s = path + '\0';
-    writeFull(fdTempRoots, (const unsigned char *) s.data(), s.size());
+    writeFull(fdTempRootsMain, (const unsigned char *) s.data(), s.size());
+
+    /* Release the lock */
+    debug(format("releasing write lock on `%1%/main'") % dnTempRoots);
+    lockFile(fdTempRootsMain, ltNone, true);
+}
 
-    /* Downgrade to a read lock. */
-    debug(format("downgrading to read lock on `%1%'") % fnTempRoots);
-    lockFile(fdTempRoots, ltRead, true);
+
+void removeDir(const Path & dir) {
+    /* First delete all files in the directory */
+    Paths files;
+    try {
+        files = readDirectory(dir);
+    } catch (SysError e) {
+        if (e.errNo == ENOENT)
+            return;
+        throw;
+    }
+    foreach (Paths::iterator, i, files) {
+        Path file = (format("%1%/%2%") % dir % *i).str();
+        if (unlink(file.c_str()) == -1)
+            /* OK if it's already deleted */
+            if (errno != ENOENT)
+                throw SysError(format("unlinking %1%") % file);
+    }
+    if (rmdir(dir.c_str()) == -1)
+        /* OK if it's already deleted */
+        if (errno != ENOENT)
+            throw SysError(format("removing %1%") % dir);
 }
 
 
 void removeTempRoots()
 {
-    if (fdTempRoots != -1) {
-        fdTempRoots.close();
-        unlink(fnTempRoots.c_str());
+    if (fdTempRootsDirLock != -1) {
+        fdTempRootsDirLock.close();
+        fdTempRootsMain.close();
+        removeDir(dnTempRoots);
     }
 }
 
@@ -233,32 +269,31 @@ typedef list<FDPtr> FDs;
 static void readTempRoots(PathSet & tempRoots, FDs & fds)
 {
     /* Read the `temproots' directory for per-process temporary root
-       files. */
+       directories. */
     Strings tempRootFiles = readDirectory(
         (format("%1%/%2%") % settings.nixStateDir % tempRootsDir).str());
 
     foreach (Strings::iterator, i, tempRootFiles) {
-        Path path = (format("%1%/%2%/%3%") % settings.nixStateDir % tempRootsDir % *i).str();
+        Path dir = (format("%1%/%2%/%3%") % settings.nixStateDir % tempRootsDir % *i).str();
+        Path path = (format("%1%/lock") % dir).str();
 
-        debug(format("reading temporary root file `%1%'") % path);
-        FDPtr fd(new AutoCloseFD(open(path.c_str(), O_RDWR, 0666)));
+        debug(format("reading temporary roots directory lock `%1%'") % path);
+        FDPtr fd(new AutoCloseFD(open(path.c_str(), O_RDWR)));
         if (*fd == -1) {
-            /* It's okay if the file has disappeared. */
-            if (errno == ENOENT) continue;
-            throw SysError(format("opening temporary roots file `%1%'") % path);
+            /* It's okay if the directory lock has disappeared. */
+            if (errno == ENOENT) {
+                removeDir(dir);
+                continue;
+            }
+            throw SysError(format("opening temporary roots directory lock `%1%'") % path);
         }
 
-        /* This should work, but doesn't, for some reason. */
-        //FDPtr fd(new AutoCloseFD(openLockFile(path, false)));
-        //if (*fd == -1) continue;
-
         /* Try to acquire a write lock without blocking.  This can
            only succeed if the owning process has died.  In that case
            we don't care about its temporary roots. */
         if (lockFile(*fd, ltWrite, false)) {
-            printMsg(lvlError, format("removing stale temporary roots file `%1%'") % path);
-            unlink(path.c_str());
-            writeFull(*fd, (const unsigned char *) "d", 1);
+            printMsg(lvlError, format("removing stale temporary roots directory `%1%'") % dir);
+            removeDir(dir);
             continue;
         }
 
@@ -268,20 +303,43 @@ static void readTempRoots(PathSet & tempRoots, FDs & fds)
         debug(format("waiting for read lock on `%1%'") % path);
         lockFile(*fd, ltRead, true);
 
-        /* Read the entire file. */
-        string contents = readFile(*fd);
+        /* Each file in this directory contains roots */
+        Strings files = readDirectory(dir);
+        foreach (Strings::iterator, j, files) {
+            if (*j == "lock")
+                continue;
+            Path root = (format("%1%/%2%") % dir % *j).str();
+
+            debug(format("reading temporary root file `%1%'") % root);
+            FDPtr fdRoot(new AutoCloseFD(open(root.c_str(), O_RDONLY)));
+            if (*fdRoot == -1) {
+                /* It's okay if the file has disappeared. */
+                if (errno == ENOENT) continue;
+                throw SysError(format("opening temporary roots file `%1%'") % root);
+            }
+
+            /* Acquire a read lock on this root. Will cause owning process
+               to block in addTempRoot or when reporting recursive paths
+               until gc is done */
+            debug(format("waiting for read lock on `%1%'") % root);
+            lockFile(*fdRoot, ltRead, true);
 
-        /* Extract the roots. */
-        string::size_type pos = 0, end;
+            /* Read the entire file. */
+            string contents = readFile(*fdRoot);
 
-        while ((end = contents.find((char) 0, pos)) != string::npos) {
-            Path root(contents, pos, end - pos);
-            debug(format("got temporary root `%1%'") % root);
-            assertStorePath(root);
-            tempRoots.insert(root);
-            pos = end + 1;
-        }
+            /* Extract the roots. */
+            string::size_type pos = 0, end;
 
+            while ((end = contents.find((char) 0, pos)) != string::npos) {
+                Path r(contents, pos, end - pos);
+                debug(format("got temporary root `%1%'") % r);
+                assertStorePath(r);
+                tempRoots.insert(r);
+                pos = end + 1;
+            }
+
+            fds.push_back(fdRoot); /* keep open */
+        }
         fds.push_back(fd); /* keep open */
     }
 }

From bfff2a95d74f18a08bb7d02230031f6bb7f26203 Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Tue, 18 Feb 2014 07:18:22 -0500
Subject: [PATCH 05/20] fix openLockFile when create is false and the file
 doesn't exist

---
 src/libstore/pathlocks.cc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/libstore/pathlocks.cc b/src/libstore/pathlocks.cc
index b858ed238de..a7a60fe123a 100644
--- a/src/libstore/pathlocks.cc
+++ b/src/libstore/pathlocks.cc
@@ -20,7 +20,8 @@ int openLockFile(const Path & path, bool create)
     if (fd == -1 && (create || errno != ENOENT))
         throw SysError(format("opening lock file `%1%'") % path);
 
-    closeOnExec(fd);
+    if (fd != -1)
+        closeOnExec(fd);
 
     return fd.borrow();
 }

From 54a0e678514bee01ec2e2feb50c86db01ece8962 Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Tue, 18 Feb 2014 08:29:25 -0500
Subject: [PATCH 06/20] Start derivations with recursive nix set up

This involves:

* Setting up the NIX_STORE_PATH etc. env vars to match the current
  process
* Packing settings into _NIX_OPTIONS
* Setting NIX_REMOTE=recursive and NIX_REMOTE_RECURSIVE_PROTOCOL_VERSION
  to the protocol version
* If this process is not itself a daemon worker and it has not started a
  recursive nix daemon instance yet, making a new unidirectional
  socketpair and a pipe, sending both ends of the socketpair and the
  read end of the pipe to a new nix-daemon process in NIX_RECURSIVE_FDS,
  keeping the write end of the pipe open (so that nix-daemon can know
  if/when we die), and using the write end of the new socketpair as the
  store's recursive nix daemon socket
* Setting NIX_REMOTE_RECURSIVE_SOCKET_FD to the store's recursive nix
  daemon socket (and keeping it alive past closeMostFDs)
* Creating a new temproots file with the same name as this drv's path
* Setting NIX_REMOTE_RECURSIVE_PATHS to the new temproots file (and
  keeping it alive past closeMostFDs)
* After the build (if not using the build hook, which for now must use
  nix-store --import if it wants to report references not in the inputs
  closure), read through the new temproots file, add the closure of each
  temproot (include outputs of derivations) to allPaths, and add each
  path as a temproot so we can delete the new temproots file

This required adding functions to gc.cc to create and delete arbitrary
temproots files (which addTempRoot uses to creat the main temproots
file). nix-daemon was updated to add the third fd in NIX_RECURSIVE_FDS
(the read side of the close notification pipe) to its select set so that
it can exit once its parent dies (closes the write side of the pipe).

Now recursive nix should work! Next up: Test case.
---
 src/libstore/build.cc        | 105 ++++++++++++++++++++++++++++++++++-
 src/libstore/gc.cc           |  68 +++++++++++++++--------
 src/libstore/local-store.cc  |   4 +-
 src/libstore/local-store.hh  |  10 +++-
 src/libstore/remote-store.cc |   6 +-
 src/nix-daemon/nix-daemon.cc |  55 +++++++++++++-----
 6 files changed, 201 insertions(+), 47 deletions(-)

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index cec03fee42a..21764258413 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -8,6 +8,7 @@
 #include "util.hh"
 #include "archive.hh"
 #include "affinity.hh"
+#include "worker-protocol.hh"
 
 #include <map>
 #include <sstream>
@@ -1435,6 +1436,35 @@ void DerivationGoal::buildDone()
                 % drvPath % statusToString(status));
         }
 
+        if (!hook) {
+            /* !!! Should hooks get a recursive paths fd, or just use importPaths? */
+            AutoCloseFD recursivePaths = worker.store.openTempRootsFile(baseNameOf(drvPath));
+
+            /* Read the entire file. */
+            string contents = readFile(recursivePaths);
+
+            /* Extract the paths. */
+            string::size_type pos = 0, end;
+
+            PathSet newPaths;
+
+            while ((end = contents.find((char) 0, pos)) != string::npos) {
+                Path r(contents, pos, end - pos);
+                debug(format("got recursive path `%1%'") % r);
+                assertStorePath(r);
+                /* If a derivation was made visible to the build, we
+                   need to also check for its outputs */
+                computeFSClosure(worker.store, r, newPaths, false, true);
+                pos = end + 1;
+            }
+
+            foreach (PathSet::iterator, i, newPaths)
+                worker.store.addTempRoot(*i);
+            allPaths.insert(newPaths.begin(), newPaths.end());
+            worker.store.deleteTempRootsFile(baseNameOf(drvPath));
+        }
+
+
         /* Compute the FS closure of the outputs and register them as
            being valid. */
         registerOutputs();
@@ -1662,6 +1692,68 @@ void DerivationGoal::startBuilder()
        getdents returns the inode of the mount point). */
     env["PWD"] = tmpDir;
 
+    /* Set up recursive nix */
+    /* !!! Should we set all of these to real values, or can we set some to
+       /var/empty ? */
+    env["NIX_STORE_DIR"] = settings.nixStore;
+    env["NIX_DATA_DIR"] = settings.nixDataDir;
+    env["NIX_LOG_DIR"] = settings.nixLogDir;
+    env["NIX_STATE_DIR"] = settings.nixStateDir;
+    env["NIX_DB_DIR"] = settings.nixDBPath;
+    /* This isn't in the chroot, but we pass the settings pack */
+    env["NIX_CONF_DIR"] = settings.nixConfDir;
+    /* Can't set NIX_LIBEXEC_DIR or NIX_BIN_DIR, obviously */
+    env["NIX_SUBSTITUTERS"] = getEnv("NIX_SUBSTITUTERS", "default");
+    env["NIX_AFFINITY_HACK"] = settings.lockCPU ? "1" : "";
+    env["_NIX_OPTIONS"] = settings.pack();
+    env["NIX_REMOTE"] = "recursive";
+    env["NIX_REMOTE_RECURSIVE_PROTOCOL_VERSION"] = int2String(RECURSIVE_PROTOCOL_VERSION);
+    if (worker.store.fdRecursiveDaemon == -1) {
+        /* This process isn't a daemon worker, and we haven't spawned a child daemon yet */
+        int sockets[2];
+        if (socketpair(PF_UNIX, SOCK_STREAM, 0, sockets) == -1)
+            throw SysError("creating recursive daemon socketpair");
+
+        if (shutdown(sockets[0], SHUT_WR) == -1)
+            throw SysError("disabling writes to listening side of daemon socketpair");
+
+        if (shutdown(sockets[1], SHUT_RD) == -1)
+            throw SysError("disabling reads from connecting side of daemon socketpair");
+
+        /* No portable way to kill children when parents die, so
+           we pass the daemon the read side of a pipe, when it gets
+           EOF there it knows we died and it exits */
+        int close_pipe[2];
+        closeOnExec(close_pipe[1]);
+        if (pipe(close_pipe) == -1)
+            throw SysError("creating recursive nix daemon close notification pipe");
+
+        set<int> kept;
+        kept.insert(sockets[0]);
+        kept.insert(sockets[1]);
+        kept.insert(close_pipe[0]);
+        string recursiveFds = (format("%1% %2% %3%") % sockets[0] % sockets[1] % close_pipe[0]).str();
+        Path progPath = (format("%1%/nix-daemon") % settings.nixBinDir).str();
+        switch (fork()) {
+            case -1:
+                throw SysError("forking to run recursive nix-daemon");
+            case 0:
+                setenv("NIX_RECURSIVE_FDS", recursiveFds.c_str(), true);
+                setenv("_NIX_OPTIONS", env["_NIX_OPTIONS"].c_str(), true);
+                closeMostFDs(kept);
+                execl(progPath.c_str(), "nix-daemon", NULL);
+                throw SysError("executing nix-daemon");
+        }
+        close(close_pipe[0]);
+        /* close_pipe[1] purposefully leaked here */
+        close(sockets[0]);
+        worker.store.fdRecursiveDaemon = sockets[1];
+    }
+    env["NIX_REMOTE_RECURSIVE_SOCKET_FD"] = int2String((int) worker.store.fdRecursiveDaemon);
+
+    AutoCloseFD recursivePaths = worker.store.openTempRootsFile(baseNameOf(drvPath));
+    env["NIX_REMOTE_RECURSIVE_PATHS_FD"] = int2String((int) recursivePaths);
+
     /* Compatibility hack with Nix <= 0.7: if this is a fixed-output
        derivation, tell the builder, so that for instance `fetchurl'
        can skip checking the output.  On older Nixes, this environment
@@ -2061,8 +2153,17 @@ void DerivationGoal::initChild()
         if (chdir(tmpDir.c_str()) == -1)
             throw SysError(format("changing into `%1%'") % tmpDir);
 
-        /* Close all other file descriptors. */
-        closeMostFDs(set<int>());
+        /* Close all file descriptors except stdio and recursive nix files. */
+        set<int> kept;
+        int fd;
+        bool ok = string2Int(env["NIX_REMOTE_RECURSIVE_SOCKET_FD"], fd);
+        kept.insert(fd);
+        assert(ok);
+        ok = string2Int(env["NIX_REMOTE_RECURSIVE_PATHS_FD"], fd);
+        kept.insert(fd);
+        assert(ok);
+
+        closeMostFDs(kept);
 
 #ifdef CAN_DO_LINUX32_BUILDS
         /* Change the personality to 32-bit if we're doing an
diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index 454ec2d212b..c7b18a8f498 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -146,13 +146,12 @@ Path addPermRoot(StoreAPI & store, const Path & _storePath,
 }
 
 
-/* The file to which we write our temporary roots. */
+/* The directory in which we store our temporary roots files. */
 static Path dnTempRoots;
 static AutoCloseFD fdTempRootsDirLock;
-static AutoCloseFD fdTempRootsMain;
 
-void LocalStore::addTempRoot(const Path & path)
-{
+int LocalStore::openTempRootsFile(const string & name) {
+    assert(name != "lock");
     /* Create the temporary roots directory for this process. */
     if (fdTempRootsDirLock == -1) {
         Path dir = (format("%1%/%2%") % settings.nixStateDir % tempRootsDir).str();
@@ -168,7 +167,9 @@ void LocalStore::addTempRoot(const Path & path)
         if (pathExists(dnTempRoots))
             /* It *must* be stale, since there can be no two
                processes with the same pid. Since we're in the gclock,
-               we're the only process who can be deleting this right now */
+               we're the only process who can be deleting this right now,
+               so use the non-ENOENT friendly deletePath instead
+               of removeDir */
             deletePath(dnTempRoots);
 
         if (mkdir(dnTempRoots.c_str(), 0700) == -1)
@@ -184,22 +185,44 @@ void LocalStore::addTempRoot(const Path & path)
         fdGCLock.close();
     }
 
-    /* Create the main temporary roots file for this process. */
-    if (fdTempRootsMain == -1) {
+    /* If the file already exists, just return it */
+    Path fn = (format("%1%/%2%") % dnTempRoots % name).str();
+    AutoCloseFD res = openLockFile(fn, false);
+    if (res == -1) {
+        /* file doesn't exist yet */
         /* Upgrade the dir lock to a write lock.  This will cause us to block
            if the garbage collector is holding our lock. */
         debug(format("acquiring write lock on `%1%/lock'") % dnTempRoots);
         lockFile(fdTempRootsDirLock, ltWrite, true);
 
-        Path fnMainTempRoots = (format("%1%/main") % dnTempRoots).str();
-
-        fdTempRootsMain = openLockFile(fnMainTempRoots, true);
+        res = openLockFile(fn, true);
 
         /* Downgrade to a read lock. */
         debug(format("downgrading to read lock on `%1%/lock'") % dnTempRoots);
         lockFile(fdTempRootsDirLock, ltRead, true);
+    }
+    return res.borrow();
+}
+
 
+void LocalStore::deleteTempRootsFile(const string & name) {
+    Path fn = (format("%1%/%2%") % dnTempRoots % name).str();
+    /* If the file doesn't exist, noop */
+    AutoCloseFD fd = openLockFile(fn, false);
+    if (fd != -1) {
+        /* If we delete between the gc opening the file and it
+           finishing the read, it will get an error. So take the
+           write lock */
+        lockFile(fd, ltWrite, true);
+        deletePath(fn);
     }
+    /* Lock released automatically */
+}
+
+
+void LocalStore::addTempRoot(const Path & path)
+{
+    static AutoCloseFD fdTempRootsMain = openTempRootsFile("main");
 
     /* Get a write lock on the main temp roots file so we don't write
        to it after the gc has already started reading from it but before
@@ -210,7 +233,6 @@ void LocalStore::addTempRoot(const Path & path)
     string s = path + '\0';
     writeFull(fdTempRootsMain, (const unsigned char *) s.data(), s.size());
 
-    /* Release the lock */
     debug(format("releasing write lock on `%1%/main'") % dnTempRoots);
     lockFile(fdTempRootsMain, ltNone, true);
 }
@@ -222,9 +244,10 @@ void removeDir(const Path & dir) {
     try {
         files = readDirectory(dir);
     } catch (SysError e) {
-        if (e.errNo == ENOENT)
-            return;
-        throw;
+        /* OK if it's already deleted */
+        if (e.errNo != ENOENT)
+            throw;
+        return;
     }
     foreach (Paths::iterator, i, files) {
         Path file = (format("%1%/%2%") % dir % *i).str();
@@ -244,7 +267,6 @@ void removeTempRoots()
 {
     if (fdTempRootsDirLock != -1) {
         fdTempRootsDirLock.close();
-        fdTempRootsMain.close();
         removeDir(dnTempRoots);
     }
 }
@@ -270,22 +292,22 @@ static void readTempRoots(PathSet & tempRoots, FDs & fds)
 {
     /* Read the `temproots' directory for per-process temporary root
        directories. */
-    Strings tempRootFiles = readDirectory(
+    Strings tempRootDirs = readDirectory(
         (format("%1%/%2%") % settings.nixStateDir % tempRootsDir).str());
 
-    foreach (Strings::iterator, i, tempRootFiles) {
+    foreach (Strings::iterator, i, tempRootDirs) {
         Path dir = (format("%1%/%2%/%3%") % settings.nixStateDir % tempRootsDir % *i).str();
-        Path path = (format("%1%/lock") % dir).str();
+        Path lock = (format("%1%/lock") % dir).str();
 
-        debug(format("reading temporary roots directory lock `%1%'") % path);
-        FDPtr fd(new AutoCloseFD(open(path.c_str(), O_RDWR)));
+        debug(format("reading temporary roots directory lock `%1%'") % lock);
+        FDPtr fd(new AutoCloseFD(open(lock.c_str(), O_RDWR)));
         if (*fd == -1) {
             /* It's okay if the directory lock has disappeared. */
             if (errno == ENOENT) {
                 removeDir(dir);
                 continue;
             }
-            throw SysError(format("opening temporary roots directory lock `%1%'") % path);
+            throw SysError(format("opening temporary roots directory lock `%1%'") % lock);
         }
 
         /* Try to acquire a write lock without blocking.  This can
@@ -300,10 +322,10 @@ static void readTempRoots(PathSet & tempRoots, FDs & fds)
         /* Acquire a read lock.  This will prevent the owning process
            from upgrading to a write lock, therefore it will block in
            addTempRoot(). */
-        debug(format("waiting for read lock on `%1%'") % path);
+        debug(format("waiting for read lock on `%1%'") % lock);
         lockFile(*fd, ltRead, true);
 
-        /* Each file in this directory contains roots */
+        /* Each file (except "lock") in this directory contains roots */
         Strings files = readDirectory(dir);
         foreach (Strings::iterator, j, files) {
             if (*j == "lock")
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index bb07453b916..407a4f4f9a9 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -223,8 +223,8 @@ void checkStoreNotSymlink()
 
 
 LocalStore::LocalStore(bool reserveSpace, int fd)
-    : didSetSubstituterEnv(false)
-    , fdRecursiveDaemon(fd)
+    : fdRecursiveDaemon(fd)
+    , didSetSubstituterEnv(false)
 {
     schemaPath = settings.nixDBPath + "/schema";
 
diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index 5ca0851b1f5..81173827c8c 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -210,6 +210,14 @@ public:
 
     void setSubstituterEnv();
 
+    /* create/open a new temproot file */
+    int openTempRootsFile(const string & name);
+
+    /* delete a temproot file */
+    void deleteTempRootsFile(const string & name);
+
+    AutoCloseFD fdRecursiveDaemon;
+
 private:
 
     Path schemaPath;
@@ -308,8 +316,6 @@ private:
     // Internal versions that are not wrapped in retry_sqlite.
     bool isValidPath_(const Path & path);
     void queryReferrers_(const Path & path, PathSet & referrers);
-
-    AutoCloseFD fdRecursiveDaemon;
 };
 
 
diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index 41868baeafb..eb33f72c9e5 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -94,9 +94,9 @@ void RemoteStore::openConnection(bool reserveSpace)
         ssize_t count = sendmsg(sfd, &msg, 0);
         if (count == -1)
             throw SysError("sending socket descriptor to daemon");
-        else if ((size_t) count != iov.iov_len)
-            throw Error(format("Tried to send %1% bytes to the daemon, but only %2% were sent")
-                    % iov.iov_len % count);
+        else if ((size_t) count != sizeof buf)
+            throw Error(format("tried to send %1% bytes to the daemon, but only %2% were sent")
+                    % sizeof buf % count);
         close(fds[1]);
 
         fdSocket = fds[0];
diff --git a/src/nix-daemon/nix-daemon.cc b/src/nix-daemon/nix-daemon.cc
index c76d30fc768..2cbbe9754ef 100644
--- a/src/nix-daemon/nix-daemon.cc
+++ b/src/nix-daemon/nix-daemon.cc
@@ -778,6 +778,8 @@ static void daemonLoop()
     AutoCloseFD fdSocket;
 
     AutoCloseFD fdRecursiveFrom;
+    AutoCloseFD fdRecursiveClose;
+    string fds;
     /* Handle socket-based activation by systemd. */
     if (getEnv("LISTEN_FDS") != "") {
         if (getEnv("LISTEN_PID") != int2String(getpid()) || getEnv("LISTEN_FDS") != "1")
@@ -786,19 +788,33 @@ static void daemonLoop()
     }
 
     /* Handle nix-daemon run as a child of build.cc for recursive nix */
-    else if (getEnv("NIX_RECURSIVE_FDS") != "") {
-        Strings tokenized = tokenizeString<Strings>(getEnv("NIX_RECURSIVE_FDS"));
-        string first = tokenized.front();
+    else if ((fds = getEnv("NIX_RECURSIVE_FDS")) != "") {
+        Strings tokenized = tokenizeString<Strings>(fds);
+
+        Strings::iterator token = tokenized.begin();
+        if (token == tokenized.end())
+            throw Error(format("invalid value for NIX_RECURSIVE_FDS, `%1%'") % fds);
         int fd;
-        if (!string2Int(first, fd))
-            throw Error(format("invalid value for NIX_RECURSIVE_FDS, `%1%'") % getEnv("NIX_RECURSIVE_FDS"));
+        if (!string2Int(*token, fd))
+            throw Error(format("invalid value for NIX_RECURSIVE_FDS, `%1%'") % fds);
         fdRecursiveFrom = fd;
 
-        tokenized.pop_front();
-        string second = tokenized.front();
-        if (!string2Int(second, fd))
-            throw Error(format("invalid value for NIX_RECURSIVE_FDS, `%1%'") % getEnv("NIX_RECURSIVE_FDS"));
+        ++token;
+        if (token == tokenized.end())
+            throw Error(format("invalid value for NIX_RECURSIVE_FDS, `%1%'") % fds);
+        if (!string2Int(*token, fd))
+            throw Error(format("invalid value for NIX_RECURSIVE_FDS, `%1%'") % fds);
         fdRecursiveTo = fd;
+
+        ++token;
+        if (token == tokenized.end())
+            throw Error(format("invalid value for NIX_RECURSIVE_FDS, `%1%'") % fds);
+        if (!string2Int(*token, fd))
+            throw Error(format("invalid value for NIX_RECURSIVE_FDS, `%1%'") % fds);
+        fdRecursiveClose = fd;
+        closeOnExec(fdRecursiveClose);
+
+        unsetenv("NIX_RECURSIVE_FDS");
     }
 
     /* Otherwise, create and bind to a Unix domain socket. */
@@ -871,10 +887,16 @@ static void daemonLoop()
             FD_ZERO(&set);
             FD_SET(fdRecursiveFrom, &set);
             /* Sigh, POSIX requires valid fds for FD_SET/FD_ISSET */
-            if (fdSocket != -1)
+            int nfds;
+            if (fdSocket != -1) {
                 FD_SET(fdSocket, &set);
-            int nfds =
-                ((fdSocket > fdRecursiveFrom) ? fdSocket : fdRecursiveFrom) + 1;
+                nfds =
+                    ((fdSocket > fdRecursiveFrom) ? fdSocket : fdRecursiveFrom) + 1;
+            } else {
+                FD_SET(fdRecursiveClose, &set);
+                nfds =
+                    ((fdRecursiveClose > fdRecursiveFrom) ? fdRecursiveClose : fdRecursiveFrom) + 1;
+            }
 
             int res = select(nfds, &set, 0, 0, 0);
             checkInterrupt();
@@ -899,6 +921,9 @@ static void daemonLoop()
                         throw SysError("accepting connection");
                 }
 
+            } else if (fdRecursiveClose != -1 && FD_ISSET(fdRecursiveClose, &set)) {
+                /* Parent died, so we're done */
+                return;
             } else {
                 /* Lots of connections on fdSocket could theoretically starve
                    the recursive socket, oh well */
@@ -920,8 +945,8 @@ static void daemonLoop()
                 ssize_t count = recvmsg(fdRecursiveFrom, &msg, 0);
                 if (count == -1)
                     throw SysError("recieving socket descriptor from client");
-                else if (((size_t) count) < sizeof buf)
-                    throw Error(format("Expected to receive %1% bytes from client, got %2%") % sizeof buf % count);
+                else if (((size_t) count) != sizeof buf)
+                    throw Error(format("expected to receive %1% bytes from client, got %2%") % sizeof buf % count);
 
                 memmove(&fd, CMSG_DATA(CMSG_FIRSTHDR(&msg)), sizeof fd);
 
@@ -929,7 +954,7 @@ static void daemonLoop()
 
                 unsigned int recursiveClientVersion = 0;
                 for (size_t byte = 0; byte < sizeof RECURSIVE_PROTOCOL_VERSION; ++byte)
-                    recursiveClientVersion += ((int) buf[byte]) << (8 * byte);
+                    recursiveClientVersion += ((unsigned int) buf[byte]) << (8 * byte);
                 if (GET_PROTOCOL_MAJOR(recursiveClientVersion) != GET_PROTOCOL_MAJOR(RECURSIVE_PROTOCOL_VERSION))
                     throw Error("nix client recursive protocol version not supported");
             }

From 332144f2a5ac9cffef1219643a1390a7af8b9bfb Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Tue, 18 Feb 2014 10:34:42 -0500
Subject: [PATCH 07/20] Add test for recursive nix

---
 tests/config.nix           |  2 +-
 tests/local.mk             |  2 +-
 tests/recursive.builder.sh |  1 +
 tests/recursive.nix        |  8 ++++++++
 tests/recursive.sh         | 19 +++++++++++++++++++
 5 files changed, 30 insertions(+), 2 deletions(-)
 create mode 100644 tests/recursive.builder.sh
 create mode 100644 tests/recursive.nix
 create mode 100644 tests/recursive.sh

diff --git a/tests/config.nix b/tests/config.nix
index 6244a15fa48..5ad6d20feab 100644
--- a/tests/config.nix
+++ b/tests/config.nix
@@ -1,7 +1,7 @@
 with import <nix/config.nix>;
 
 rec {
-  inherit shell;
+  inherit shell nixBinDir;
 
   path = coreutils;
 
diff --git a/tests/local.mk b/tests/local.mk
index ab170dfe51e..c58496dad11 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -10,7 +10,7 @@ nix_tests = \
   remote-store.sh export.sh export-graph.sh negative-caching.sh \
   binary-patching.sh timeout.sh secure-drv-outputs.sh nix-channel.sh \
   multiple-outputs.sh import-derivation.sh fetchurl.sh optimise-store.sh \
-  binary-cache.sh nix-profile.sh repair.sh dump-db.sh
+  binary-cache.sh nix-profile.sh repair.sh dump-db.sh recursive.sh
 
 install-tests += $(foreach x, $(nix_tests), tests/$(x))
 
diff --git a/tests/recursive.builder.sh b/tests/recursive.builder.sh
new file mode 100644
index 00000000000..f5db2d4cdad
--- /dev/null
+++ b/tests/recursive.builder.sh
@@ -0,0 +1 @@
+nix-store -r $(nix-instantiate $simple) --add-root $out
diff --git a/tests/recursive.nix b/tests/recursive.nix
new file mode 100644
index 00000000000..3617dfa199c
--- /dev/null
+++ b/tests/recursive.nix
@@ -0,0 +1,8 @@
+with import ./config.nix;
+
+mkDerivation {
+  name = "recursive";
+  builder = ./recursive.builder.sh;
+  PATH = "${path}:${nixBinDir}";
+  simple = builtins.toString ./simple.nix;
+}
diff --git a/tests/recursive.sh b/tests/recursive.sh
new file mode 100644
index 00000000000..1863973bb32
--- /dev/null
+++ b/tests/recursive.sh
@@ -0,0 +1,19 @@
+source common.sh
+
+drvPath=$(nix-instantiate recursive.nix)
+
+test "$(nix-store -q --binding system "$drvPath")" = "$system"
+
+echo "derivation is $drvPath"
+
+outPath=$(nix-store -rvv "$drvPath")
+
+echo "output path is $outPath"
+
+text=$(cat "$outPath"/hello)
+if test "$text" != "Hello World!"; then exit 1; fi
+
+# Directed delete: $outPath is not reachable from a root, so it should
+# be deleteable.
+nix-store --delete $outPath
+if test -e $outPath/hello; then false; fi

From 1825f27c3a3ee5140d9088f4cb8d04c66fc24662 Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Tue, 18 Feb 2014 10:35:05 -0500
Subject: [PATCH 08/20] remote-store.cc: Send the version little-endian

---
 src/libstore/remote-store.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index eb33f72c9e5..687649ec1ff 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -74,7 +74,7 @@ void RemoteStore::openConnection(bool reserveSpace)
         /* Send our protocol version and a socket to the daemon */
         unsigned char buf[sizeof RECURSIVE_PROTOCOL_VERSION];
         for (size_t byte = 0; byte < sizeof buf; ++byte)
-            buf[sizeof buf - (byte + 1)] =
+            buf[byte] =
                 (unsigned char) ((RECURSIVE_PROTOCOL_VERSION >> (8 * byte)) & 0xFF);
         struct iovec iov;
         iov.iov_base = buf;

From d816a829cf6b359d1490ae165189ccec80acfa56 Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Tue, 18 Feb 2014 10:44:28 -0500
Subject: [PATCH 09/20] Can't do --add-root on a nix store path

---
 tests/recursive.builder.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/recursive.builder.sh b/tests/recursive.builder.sh
index f5db2d4cdad..aa1dd0a8a1f 100644
--- a/tests/recursive.builder.sh
+++ b/tests/recursive.builder.sh
@@ -1 +1 @@
-nix-store -r $(nix-instantiate $simple) --add-root $out
+ln -sv $(nix-store -r $(nix-instantiate $simple)) $out

From 1967cc166ec9b4cbf0138e2da6e60da13dc0a326 Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Tue, 18 Feb 2014 11:00:51 -0500
Subject: [PATCH 10/20] The recursive paths fd passed to the build can't be
 close-on-exec

---
 src/libstore/build.cc | 1 +
 src/libutil/util.cc   | 9 +++++++++
 src/libutil/util.hh   | 3 +++
 3 files changed, 13 insertions(+)

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index 21764258413..81d2cc942ac 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -1752,6 +1752,7 @@ void DerivationGoal::startBuilder()
     env["NIX_REMOTE_RECURSIVE_SOCKET_FD"] = int2String((int) worker.store.fdRecursiveDaemon);
 
     AutoCloseFD recursivePaths = worker.store.openTempRootsFile(baseNameOf(drvPath));
+    noCloseOnExec(recursivePaths);
     env["NIX_REMOTE_RECURSIVE_PATHS_FD"] = int2String((int) recursivePaths);
 
     /* Compatibility hack with Nix <= 0.7: if this is a fixed-output
diff --git a/src/libutil/util.cc b/src/libutil/util.cc
index c50dcf07519..a1b0dc44968 100644
--- a/src/libutil/util.cc
+++ b/src/libutil/util.cc
@@ -921,6 +921,15 @@ void closeOnExec(int fd)
 }
 
 
+void noCloseOnExec(int fd)
+{
+    int prev;
+    if ((prev = fcntl(fd, F_GETFD, 0)) == -1 ||
+        fcntl(fd, F_SETFD, prev & ~FD_CLOEXEC) == -1)
+        throw SysError("unsetting close-on-exec flag");
+}
+
+
 void setNonBlocking(int fd)
 {
     int prev;
diff --git a/src/libutil/util.hh b/src/libutil/util.hh
index ca88957ec1d..b782db547b4 100644
--- a/src/libutil/util.hh
+++ b/src/libutil/util.hh
@@ -261,6 +261,9 @@ void closeMostFDs(const set<int> & exceptions);
 /* Set the close-on-exec flag for the given file descriptor. */
 void closeOnExec(int fd);
 
+/* Unset the close-on-exec flag for the given file descriptor. */
+void noCloseOnExec(int fd);
+
 /* Put the given file descriptor into non-blocking mode. */
 void setNonBlocking(int fd);
 

From 24b34d5848aaba0ffceb96ac7980e02a861e131c Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Tue, 18 Feb 2014 11:01:09 -0500
Subject: [PATCH 11/20] Make recursive test test a bit more

---
 tests/recursive.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/recursive.sh b/tests/recursive.sh
index 1863973bb32..25f8c30c746 100644
--- a/tests/recursive.sh
+++ b/tests/recursive.sh
@@ -15,5 +15,5 @@ if test "$text" != "Hello World!"; then exit 1; fi
 
 # Directed delete: $outPath is not reachable from a root, so it should
 # be deleteable.
-nix-store --delete $outPath
-if test -e $outPath/hello; then false; fi
+nix-store --delete $(readlink $outPath)
+if test -e $outPath; then false; fi

From 4a902149355f6f492db0c7bf6003526ec1bfeb69 Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Tue, 18 Feb 2014 11:38:03 -0500
Subject: [PATCH 12/20] Clean up NIX_* variables passed to derivations

---
 src/libstore/build.cc | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index 81d2cc942ac..daef2b9868c 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -1693,17 +1693,17 @@ void DerivationGoal::startBuilder()
     env["PWD"] = tmpDir;
 
     /* Set up recursive nix */
-    /* !!! Should we set all of these to real values, or can we set some to
-       /var/empty ? */
     env["NIX_STORE_DIR"] = settings.nixStore;
-    env["NIX_DATA_DIR"] = settings.nixDataDir;
-    env["NIX_LOG_DIR"] = settings.nixLogDir;
-    env["NIX_STATE_DIR"] = settings.nixStateDir;
-    env["NIX_DB_DIR"] = settings.nixDBPath;
-    /* This isn't in the chroot, but we pass the settings pack */
-    env["NIX_CONF_DIR"] = settings.nixConfDir;
-    /* Can't set NIX_LIBEXEC_DIR or NIX_BIN_DIR, obviously */
-    env["NIX_SUBSTITUTERS"] = getEnv("NIX_SUBSTITUTERS", "default");
+    /* NIX_DATA_DIR, NIX_LIBEXEC_DIR, NIX_BIN_DIR purposefully not set */
+    /* If we want log reading from recursive nix, we'll
+       need to put the log dir in the chroot */
+    env["NIX_LOG_DIR"] = "/var/empty";
+    env["NIX_STATE_DIR"] = "/var/empty";
+    env["NIX_DB_DIR"] = "/var/empty";
+    /* We pass the settings pack. Would need to put this
+       in the chroot if we really wanted it */
+    env["NIX_CONF_DIR"] = "/var/empty";
+    env["NIX_SUBSTITUTERS"] = "";
     env["NIX_AFFINITY_HACK"] = settings.lockCPU ? "1" : "";
     env["_NIX_OPTIONS"] = settings.pack();
     env["NIX_REMOTE"] = "recursive";

From a51fd938a90628d6dc95f441996f4e82ee43e5a3 Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Tue, 18 Feb 2014 13:30:55 -0500
Subject: [PATCH 13/20] Open temproots files in O_APPEND mode so parallel
 remote nix invocations don't step on each other

---
 src/libstore/gc.cc        | 4 ++--
 src/libstore/pathlocks.cc | 4 ++--
 src/libstore/pathlocks.hh | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index c7b18a8f498..757f9fe09ef 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -187,7 +187,7 @@ int LocalStore::openTempRootsFile(const string & name) {
 
     /* If the file already exists, just return it */
     Path fn = (format("%1%/%2%") % dnTempRoots % name).str();
-    AutoCloseFD res = openLockFile(fn, false);
+    AutoCloseFD res = openLockFile(fn, false, true);
     if (res == -1) {
         /* file doesn't exist yet */
         /* Upgrade the dir lock to a write lock.  This will cause us to block
@@ -195,7 +195,7 @@ int LocalStore::openTempRootsFile(const string & name) {
         debug(format("acquiring write lock on `%1%/lock'") % dnTempRoots);
         lockFile(fdTempRootsDirLock, ltWrite, true);
 
-        res = openLockFile(fn, true);
+        res = openLockFile(fn, true, true);
 
         /* Downgrade to a read lock. */
         debug(format("downgrading to read lock on `%1%/lock'") % dnTempRoots);
diff --git a/src/libstore/pathlocks.cc b/src/libstore/pathlocks.cc
index a7a60fe123a..14a2b125ad3 100644
--- a/src/libstore/pathlocks.cc
+++ b/src/libstore/pathlocks.cc
@@ -12,11 +12,11 @@
 namespace nix {
 
 
-int openLockFile(const Path & path, bool create)
+int openLockFile(const Path & path, bool create, bool append)
 {
     AutoCloseFD fd;
 
-    fd = open(path.c_str(), O_RDWR | (create ? O_CREAT : 0), 0600);
+    fd = open(path.c_str(), O_RDWR | (create ? O_CREAT : 0) | (append ? O_APPEND : 0), 0600);
     if (fd == -1 && (create || errno != ENOENT))
         throw SysError(format("opening lock file `%1%'") % path);
 
diff --git a/src/libstore/pathlocks.hh b/src/libstore/pathlocks.hh
index 8a6b1450da2..3a2d3b3377f 100644
--- a/src/libstore/pathlocks.hh
+++ b/src/libstore/pathlocks.hh
@@ -9,7 +9,7 @@ namespace nix {
 /* Open (possibly create) a lock file and return the file descriptor.
    -1 is returned if create is false and the lock could not be opened
    because it doesn't exist.  Any other error throws an exception. */
-int openLockFile(const Path & path, bool create);
+int openLockFile(const Path & path, bool create, bool append = false);
 
 /* Delete an open lock file. */
 void deleteLockFile(const Path & path, int fd);

From b8dd0cd67efdef748a668afb5aa5b8e076869b5b Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Wed, 26 Feb 2014 15:47:15 -0500
Subject: [PATCH 14/20] Report recursive paths immediately and wait for
 confirmation

This allows the parent nix process to add any needed new paths to the
chroot. The reporting protocol is changed to have a cap of 4096 bytes
per write and to require reading a single confirmation byte after each
write.

Temp roots handling is reverted back to the pre-recursive-nix way, since
we no longer create a new temp roots file for each build.
---
 src/libstore/build.cc        | 120 +++++++++++++------
 src/libstore/gc.cc           | 220 +++++++++++------------------------
 src/libstore/local-store.hh  |   6 -
 src/libstore/remote-store.cc |  74 +++++++++---
 src/libstore/remote-store.hh |   4 +
 5 files changed, 212 insertions(+), 212 deletions(-)

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index daef2b9868c..00add94d7ec 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -734,6 +734,9 @@ class DerivationGoal : public Goal
     /* Pipe for the builder's standard output/error. */
     Pipe builderOut;
 
+    /* Our end of the recursive paths reporting socket. */
+    AutoCloseFD recursivePathsParentSide;
+
     /* The build hook. */
     boost::shared_ptr<HookInstance> hook;
 
@@ -1436,35 +1439,6 @@ void DerivationGoal::buildDone()
                 % drvPath % statusToString(status));
         }
 
-        if (!hook) {
-            /* !!! Should hooks get a recursive paths fd, or just use importPaths? */
-            AutoCloseFD recursivePaths = worker.store.openTempRootsFile(baseNameOf(drvPath));
-
-            /* Read the entire file. */
-            string contents = readFile(recursivePaths);
-
-            /* Extract the paths. */
-            string::size_type pos = 0, end;
-
-            PathSet newPaths;
-
-            while ((end = contents.find((char) 0, pos)) != string::npos) {
-                Path r(contents, pos, end - pos);
-                debug(format("got recursive path `%1%'") % r);
-                assertStorePath(r);
-                /* If a derivation was made visible to the build, we
-                   need to also check for its outputs */
-                computeFSClosure(worker.store, r, newPaths, false, true);
-                pos = end + 1;
-            }
-
-            foreach (PathSet::iterator, i, newPaths)
-                worker.store.addTempRoot(*i);
-            allPaths.insert(newPaths.begin(), newPaths.end());
-            worker.store.deleteTempRootsFile(baseNameOf(drvPath));
-        }
-
-
         /* Compute the FS closure of the outputs and register them as
            being valid. */
         registerOutputs();
@@ -1751,9 +1725,17 @@ void DerivationGoal::startBuilder()
     }
     env["NIX_REMOTE_RECURSIVE_SOCKET_FD"] = int2String((int) worker.store.fdRecursiveDaemon);
 
-    AutoCloseFD recursivePaths = worker.store.openTempRootsFile(baseNameOf(drvPath));
-    noCloseOnExec(recursivePaths);
-    env["NIX_REMOTE_RECURSIVE_PATHS_FD"] = int2String((int) recursivePaths);
+    int recursivePaths[2];
+    if (socketpair(PF_UNIX, SOCK_STREAM, 0, recursivePaths) == -1)
+        throw SysError("creating recursive paths socketpair");
+
+    AutoCloseFD recursivePathsChildSide = recursivePaths[0];
+    recursivePathsParentSide = recursivePaths[1];
+    /* We set our end of the socket non-blocking, so a malicious
+       builder can't make us block writing the confirmation */
+    setNonBlocking(recursivePathsParentSide);
+    closeOnExec(recursivePathsParentSide);
+    env["NIX_REMOTE_RECURSIVE_PATHS_FD"] = int2String((int) recursivePathsChildSide);
 
     /* Compatibility hack with Nix <= 0.7: if this is a fixed-output
        derivation, tell the builder, so that for instance `fetchurl'
@@ -2051,8 +2033,10 @@ void DerivationGoal::startBuilder()
     /* parent */
     pid.setSeparatePG(true);
     builderOut.writeSide.close();
-    worker.childStarted(shared_from_this(), pid,
-        singleton<set<int> >(builderOut.readSide), true, true);
+    set<int> fds;
+    fds.insert(builderOut.readSide);
+    fds.insert(recursivePathsParentSide);
+    worker.childStarted(shared_from_this(), pid, fds, true, true);
 
     if (settings.printBuildTrace) {
         printMsg(lvlError, format("@ build-started %1% - %2% %3%")
@@ -2531,16 +2515,78 @@ void DerivationGoal::handleChildOutput(int fd, const string & data)
             if (err != BZ_OK) throw Error(format("cannot write to compressed log file (BZip2 error = %1%)") % err);
         } else if (fdLogFile != -1)
             writeFull(fdLogFile, (unsigned char *) data.data(), data.size());
-    }
+    } else if (!hook && fd == recursivePathsParentSide) {
+        /* Extract the paths. */
+        string::size_type pos = 0, end;
+
+        PathSet newPaths;
+
+        while ((end = data.find((char) 0, pos)) != string::npos) {
+            Path r(data, pos, end - pos);
+            debug(format("got recursive path `%1%'") % r);
+            assertStorePath(r);
+            /* If a derivation was made visible to the build, we
+               need to also check for its outputs */
+            computeFSClosure(worker.store, r, newPaths, false, true);
+            pos = end + 1;
+        }
 
-    if (hook && fd == hook->fromHook.readSide)
+        foreach (PathSet::iterator, i, newPaths) {
+            worker.store.addTempRoot(*i);
+#if CHROOT_ENABLED
+            if (useChroot && worker.store.isValidPath(*i)) {
+                /* We need this path to be available in the chroot */
+                Path target = chrootRootDir + *i;
+                if (access(target.c_str(), F_OK) == 0)
+                    /* Path already exists, assume it's the right one */
+                    continue;
+                struct stat st;
+                if (lstat(i->c_str(), &st))
+                    throw SysError(format("getting attributes of path `%1%'") % *i);
+                if (S_ISDIR(st.st_mode)) {
+                    debug(format("bind mounting `%1%' to `%2%'") % *i % target);
+                    createDirs(target);
+                    if (mount(i->c_str(), target.c_str(), "", MS_BIND, 0) == -1)
+                        throw SysError(format("bind mount from `%1%' to `%2%' failed") % *i % target);
+                } else {
+                    if (link(i->c_str(), target.c_str()) == -1) {
+                        /* Hard-linking fails if we exceed the maximum
+                           link count on a file (e.g. 32000 of ext3),
+                           which is quite possible after a `nix-store
+                           --optimise'. */
+                        if (errno != EMLINK)
+                            throw SysError(format("linking `%1%' to `%2%'") % target % *i);
+                        StringSink sink;
+                        dumpPath(*i, sink);
+                        StringSource source(sink.s);
+                        restorePath(target, source);
+                    }
+                }
+            }
+#endif
+        }
+
+        allPaths.insert(newPaths.begin(), newPaths.end());
+
+        /* Notify the child that we're done */
+        unsigned char c = '\0';
+        try {
+            writeFull(fd, &c, sizeof c);
+        } catch (SysError & e) {
+            /* Don't let a rogue builder kill us */
+            printMsg(lvlError, format("error confirming recursive paths read: %1%") % e.what());
+        }
+    } else if (hook && fd == hook->fromHook.readSide)
         writeToStderr(data);
 }
 
 
 void DerivationGoal::handleEOF(int fd)
 {
-    worker.wakeUp(shared_from_this());
+    if (!hook && fd == recursivePathsParentSide)
+        recursivePathsParentSide.close();
+    else
+        worker.wakeUp(shared_from_this());
 }
 
 
diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index 757f9fe09ef..79bd7d56b3f 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -146,128 +146,70 @@ Path addPermRoot(StoreAPI & store, const Path & _storePath,
 }
 
 
-/* The directory in which we store our temporary roots files. */
-static Path dnTempRoots;
-static AutoCloseFD fdTempRootsDirLock;
+/* The file to which we write our temporary roots. */
+static Path fnTempRoots;
+static AutoCloseFD fdTempRoots;
 
-int LocalStore::openTempRootsFile(const string & name) {
-    assert(name != "lock");
-    /* Create the temporary roots directory for this process. */
-    if (fdTempRootsDirLock == -1) {
-        Path dir = (format("%1%/%2%") % settings.nixStateDir % tempRootsDir).str();
-        createDirs(dir);
 
-        dnTempRoots = (format("%1%/%2%")
-            % dir % getpid()).str();
+void LocalStore::addTempRoot(const Path & path)
+{
+    /* Create the temporary roots file for this process. */
+    if (fdTempRoots == -1) {
 
-        Path fnTempRootsDirLock = (format("%1%/lock") % dnTempRoots).str();
+        while (1) {
+            Path dir = (format("%1%/%2%") % settings.nixStateDir % tempRootsDir).str();
+            createDirs(dir);
 
-        AutoCloseFD fdGCLock = openGCLock(ltRead);
+            fnTempRoots = (format("%1%/%2%")
+                % dir % getpid()).str();
 
-        if (pathExists(dnTempRoots))
-            /* It *must* be stale, since there can be no two
-               processes with the same pid. Since we're in the gclock,
-               we're the only process who can be deleting this right now,
-               so use the non-ENOENT friendly deletePath instead
-               of removeDir */
-            deletePath(dnTempRoots);
+            AutoCloseFD fdGCLock = openGCLock(ltRead);
 
-        if (mkdir(dnTempRoots.c_str(), 0700) == -1)
-            throw SysError("creating temporary roots directory");
+            if (pathExists(fnTempRoots))
+                /* It *must* be stale, since there can be no two
+                   processes with the same pid. */
+                unlink(fnTempRoots.c_str());
 
-        fdTempRootsDirLock = openLockFile(fnTempRootsDirLock, true);
+            fdTempRoots = openLockFile(fnTempRoots, true);
 
-        /* We need to take this lock before releasing the gc lock so
-           that the gc doesn't delete the directory */
-        debug(format("acquiring read lock on `%1%/lock'") % dnTempRoots);
-        lockFile(fdTempRootsDirLock, ltRead, true);
+            fdGCLock.close();
 
-        fdGCLock.close();
-    }
+            debug(format("acquiring read lock on `%1%'") % fnTempRoots);
+            lockFile(fdTempRoots, ltRead, true);
 
-    /* If the file already exists, just return it */
-    Path fn = (format("%1%/%2%") % dnTempRoots % name).str();
-    AutoCloseFD res = openLockFile(fn, false, true);
-    if (res == -1) {
-        /* file doesn't exist yet */
-        /* Upgrade the dir lock to a write lock.  This will cause us to block
-           if the garbage collector is holding our lock. */
-        debug(format("acquiring write lock on `%1%/lock'") % dnTempRoots);
-        lockFile(fdTempRootsDirLock, ltWrite, true);
-
-        res = openLockFile(fn, true, true);
-
-        /* Downgrade to a read lock. */
-        debug(format("downgrading to read lock on `%1%/lock'") % dnTempRoots);
-        lockFile(fdTempRootsDirLock, ltRead, true);
-    }
-    return res.borrow();
-}
+            /* Check whether the garbage collector didn't get in our
+               way. */
+            struct stat st;
+            if (fstat(fdTempRoots, &st) == -1)
+                throw SysError(format("statting `%1%'") % fnTempRoots);
+            if (st.st_size == 0) break;
 
+            /* The garbage collector deleted this file before we could
+               get a lock.  (It won't delete the file after we get a
+               lock.)  Try again. */
+        }
 
-void LocalStore::deleteTempRootsFile(const string & name) {
-    Path fn = (format("%1%/%2%") % dnTempRoots % name).str();
-    /* If the file doesn't exist, noop */
-    AutoCloseFD fd = openLockFile(fn, false);
-    if (fd != -1) {
-        /* If we delete between the gc opening the file and it
-           finishing the read, it will get an error. So take the
-           write lock */
-        lockFile(fd, ltWrite, true);
-        deletePath(fn);
     }
-    /* Lock released automatically */
-}
 
-
-void LocalStore::addTempRoot(const Path & path)
-{
-    static AutoCloseFD fdTempRootsMain = openTempRootsFile("main");
-
-    /* Get a write lock on the main temp roots file so we don't write
-       to it after the gc has already started reading from it but before
-       gc has finished */
-    debug(format("acquiring write lock on `%1%/main'") % dnTempRoots);
-    lockFile(fdTempRootsMain, ltWrite, true);
+    /* Upgrade the lock to a write lock.  This will cause us to block
+       if the garbage collector is holding our lock. */
+    debug(format("acquiring write lock on `%1%'") % fnTempRoots);
+    lockFile(fdTempRoots, ltWrite, true);
 
     string s = path + '\0';
-    writeFull(fdTempRootsMain, (const unsigned char *) s.data(), s.size());
+    writeFull(fdTempRoots, (const unsigned char *) s.data(), s.size());
 
-    debug(format("releasing write lock on `%1%/main'") % dnTempRoots);
-    lockFile(fdTempRootsMain, ltNone, true);
-}
-
-
-void removeDir(const Path & dir) {
-    /* First delete all files in the directory */
-    Paths files;
-    try {
-        files = readDirectory(dir);
-    } catch (SysError e) {
-        /* OK if it's already deleted */
-        if (e.errNo != ENOENT)
-            throw;
-        return;
-    }
-    foreach (Paths::iterator, i, files) {
-        Path file = (format("%1%/%2%") % dir % *i).str();
-        if (unlink(file.c_str()) == -1)
-            /* OK if it's already deleted */
-            if (errno != ENOENT)
-                throw SysError(format("unlinking %1%") % file);
-    }
-    if (rmdir(dir.c_str()) == -1)
-        /* OK if it's already deleted */
-        if (errno != ENOENT)
-            throw SysError(format("removing %1%") % dir);
+    /* Downgrade to a read lock. */
+    debug(format("downgrading to read lock on `%1%'") % fnTempRoots);
+    lockFile(fdTempRoots, ltRead, true);
 }
 
 
 void removeTempRoots()
 {
-    if (fdTempRootsDirLock != -1) {
-        fdTempRootsDirLock.close();
-        removeDir(dnTempRoots);
+    if (fdTempRoots != -1) {
+        fdTempRoots.close();
+        unlink(fnTempRoots.c_str());
     }
 }
 
@@ -291,77 +233,55 @@ typedef list<FDPtr> FDs;
 static void readTempRoots(PathSet & tempRoots, FDs & fds)
 {
     /* Read the `temproots' directory for per-process temporary root
-       directories. */
-    Strings tempRootDirs = readDirectory(
+       files. */
+    Strings tempRootFiles = readDirectory(
         (format("%1%/%2%") % settings.nixStateDir % tempRootsDir).str());
 
-    foreach (Strings::iterator, i, tempRootDirs) {
-        Path dir = (format("%1%/%2%/%3%") % settings.nixStateDir % tempRootsDir % *i).str();
-        Path lock = (format("%1%/lock") % dir).str();
+    foreach (Strings::iterator, i, tempRootFiles) {
+        Path path = (format("%1%/%2%/%3%") % settings.nixStateDir % tempRootsDir % *i).str();
 
-        debug(format("reading temporary roots directory lock `%1%'") % lock);
-        FDPtr fd(new AutoCloseFD(open(lock.c_str(), O_RDWR)));
+        debug(format("reading temporary root file `%1%'") % path);
+        FDPtr fd(new AutoCloseFD(open(path.c_str(), O_RDWR, 0666)));
         if (*fd == -1) {
-            /* It's okay if the directory lock has disappeared. */
-            if (errno == ENOENT) {
-                removeDir(dir);
-                continue;
-            }
-            throw SysError(format("opening temporary roots directory lock `%1%'") % lock);
+            /* It's okay if the file has disappeared. */
+            if (errno == ENOENT) continue;
+            throw SysError(format("opening temporary roots file `%1%'") % path);
         }
 
+        /* This should work, but doesn't, for some reason. */
+        //FDPtr fd(new AutoCloseFD(openLockFile(path, false)));
+        //if (*fd == -1) continue;
+
         /* Try to acquire a write lock without blocking.  This can
            only succeed if the owning process has died.  In that case
            we don't care about its temporary roots. */
         if (lockFile(*fd, ltWrite, false)) {
-            printMsg(lvlError, format("removing stale temporary roots directory `%1%'") % dir);
-            removeDir(dir);
+            printMsg(lvlError, format("removing stale temporary roots file `%1%'") % path);
+            unlink(path.c_str());
+            writeFull(*fd, (const unsigned char *) "d", 1);
             continue;
         }
 
         /* Acquire a read lock.  This will prevent the owning process
            from upgrading to a write lock, therefore it will block in
            addTempRoot(). */
-        debug(format("waiting for read lock on `%1%'") % lock);
+        debug(format("waiting for read lock on `%1%'") % path);
         lockFile(*fd, ltRead, true);
 
-        /* Each file (except "lock") in this directory contains roots */
-        Strings files = readDirectory(dir);
-        foreach (Strings::iterator, j, files) {
-            if (*j == "lock")
-                continue;
-            Path root = (format("%1%/%2%") % dir % *j).str();
-
-            debug(format("reading temporary root file `%1%'") % root);
-            FDPtr fdRoot(new AutoCloseFD(open(root.c_str(), O_RDONLY)));
-            if (*fdRoot == -1) {
-                /* It's okay if the file has disappeared. */
-                if (errno == ENOENT) continue;
-                throw SysError(format("opening temporary roots file `%1%'") % root);
-            }
-
-            /* Acquire a read lock on this root. Will cause owning process
-               to block in addTempRoot or when reporting recursive paths
-               until gc is done */
-            debug(format("waiting for read lock on `%1%'") % root);
-            lockFile(*fdRoot, ltRead, true);
-
-            /* Read the entire file. */
-            string contents = readFile(*fdRoot);
+        /* Read the entire file. */
+        string contents = readFile(*fd);
 
-            /* Extract the roots. */
-            string::size_type pos = 0, end;
+        /* Extract the roots. */
+        string::size_type pos = 0, end;
 
-            while ((end = contents.find((char) 0, pos)) != string::npos) {
-                Path r(contents, pos, end - pos);
-                debug(format("got temporary root `%1%'") % r);
-                assertStorePath(r);
-                tempRoots.insert(r);
-                pos = end + 1;
-            }
-
-            fds.push_back(fdRoot); /* keep open */
+        while ((end = contents.find((char) 0, pos)) != string::npos) {
+            Path root(contents, pos, end - pos);
+            debug(format("got temporary root `%1%'") % root);
+            assertStorePath(root);
+            tempRoots.insert(root);
+            pos = end + 1;
         }
+
         fds.push_back(fd); /* keep open */
     }
 }
diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index 81173827c8c..7c2a5791fd8 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -210,12 +210,6 @@ public:
 
     void setSubstituterEnv();
 
-    /* create/open a new temproot file */
-    int openTempRootsFile(const string & name);
-
-    /* delete a temproot file */
-    void deleteTempRootsFile(const string & name);
-
     AutoCloseFD fdRecursiveDaemon;
 
 private:
diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index 687649ec1ff..b99c09f3121 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -183,17 +183,6 @@ void RemoteStore::connectToDaemon()
 RemoteStore::~RemoteStore()
 {
     try {
-        /* As long as we do this *before* closing the fdSocket, the daemon
-           should keep our paths alive */
-        if (fdRecursivePaths != -1 && !recursivePaths.empty()) {
-            string s = "";
-            foreach (PathSet::const_iterator, i, recursivePaths) {
-                s += *i + '\0';
-            }
-            lockFile(fdRecursivePaths, ltWrite, true);
-            writeFull(fdRecursivePaths, (const unsigned char *) s.data(), s.size());
-            lockFile(fdRecursivePaths, ltNone, true);
-        }
         to.flush();
         fdSocket.close();
         if (child != -1)
@@ -273,7 +262,7 @@ PathSet RemoteStore::queryAllValidPaths()
     writeInt(wopQueryAllValidPaths, to);
     processStderr();
     PathSet res = readStorePaths<PathSet>(from);
-    recursivePaths.insert(res.begin(), res.end());
+    reportRecursivePaths(res);
     return res;
 }
 
@@ -308,6 +297,8 @@ void RemoteStore::querySubstitutablePathInfos(const PathSet & paths,
 
     if (GET_PROTOCOL_MINOR(daemonVersion) < 3) return;
 
+    PathSet recursivePaths;
+
     if (GET_PROTOCOL_MINOR(daemonVersion) < 12) {
 
         foreach (PathSet::const_iterator, i, paths) {
@@ -350,6 +341,8 @@ void RemoteStore::querySubstitutablePathInfos(const PathSet & paths,
         }
 
     }
+
+    reportRecursivePaths(recursivePaths);
 }
 
 
@@ -362,6 +355,7 @@ ValidPathInfo RemoteStore::queryPathInfo(const Path & path)
     ValidPathInfo info;
     info.path = path;
     info.deriver = readString(from);
+    PathSet recursivePaths;
     if (info.deriver != "") {
         assertStorePath(info.deriver);
         recursivePaths.insert(info.deriver);
@@ -371,6 +365,8 @@ ValidPathInfo RemoteStore::queryPathInfo(const Path & path)
     recursivePaths.insert(info.references.begin(), info.references.end());
     info.registrationTime = readInt(from);
     info.narSize = readLongLong(from);
+
+    reportRecursivePaths(recursivePaths);
     return info;
 }
 
@@ -394,7 +390,7 @@ void RemoteStore::queryReferences(const Path & path,
     writeString(path, to);
     processStderr();
     PathSet references2 = readStorePaths<PathSet>(from);
-    recursivePaths.insert(references2.begin(), references2.end());
+    reportRecursivePaths(references2);
     references.insert(references2.begin(), references2.end());
 }
 
@@ -421,7 +417,7 @@ Path RemoteStore::queryDeriver(const Path & path)
     Path drvPath = readString(from);
     if (drvPath != "") {
         assertStorePath(drvPath);
-        recursivePaths.insert(drvPath);
+        reportRecursivePath(drvPath);
     }
     return drvPath;
 }
@@ -434,7 +430,7 @@ PathSet RemoteStore::queryValidDerivers(const Path & path)
     writeString(path, to);
     processStderr();
     PathSet res = readStorePaths<PathSet>(from);
-    recursivePaths.insert(res.begin(), res.end());
+    reportRecursivePaths(res);
     return res;
 }
 
@@ -472,7 +468,7 @@ Path RemoteStore::queryPathFromHashPart(const string & hashPart)
     Path path = readString(from);
     if (!path.empty()) {
         assertStorePath(path);
-        recursivePaths.insert(path);
+        reportRecursivePath(path);
     }
     return path;
 }
@@ -496,7 +492,7 @@ Path RemoteStore::addToStore(const Path & _srcPath,
     dumpPath(srcPath, to, filter);
     processStderr();
     Path res = readStorePath(from);
-    recursivePaths.insert(res);
+    reportRecursivePath(res);
     return res;
 }
 
@@ -514,7 +510,7 @@ Path RemoteStore::addTextToStore(const string & name, const string & s,
 
     processStderr();
     Path res = readStorePath(from);
-    recursivePaths.insert(res);
+    reportRecursivePath(res);
     return res;
 }
 
@@ -539,7 +535,7 @@ Paths RemoteStore::importPaths(bool requireSignature, Source & source)
        anyway. */
     processStderr(0, &source);
     Paths res = readStorePaths<Paths>(from);
-    recursivePaths.insert(res.begin(), res.end());
+    reportRecursivePaths(PathSet(res.begin(), res.end()));
     return res;
 }
 
@@ -610,12 +606,14 @@ Roots RemoteStore::findRoots()
     processStderr();
     unsigned int count = readInt(from);
     Roots result;
+    PathSet recursivePaths;
     while (count--) {
         Path link = readString(from);
         Path target = readStorePath(from);
         recursivePaths.insert(target);
         result[link] = target;
     }
+    reportRecursivePaths(recursivePaths);
     return result;
 }
 
@@ -663,6 +661,44 @@ void RemoteStore::clearFailedPaths(const PathSet & paths)
 }
 
 
+static void writeRecursivePaths(int fd, const string & s) {
+    lockFile(fd, ltWrite, true);
+    writeFull(fd, (const unsigned char *) s.data(), s.size());
+    unsigned char c;
+    readFull(fd, &c, sizeof c);
+    lockFile(fd, ltNone, true);
+}
+
+
+void RemoteStore::reportRecursivePath(const Path & path)
+{
+    if (fdRecursivePaths != -1) {
+        if (path.size() >= 4096)
+            throw Error("reporting a path name bigger than 4096 bytes not allowed");
+        string s = path + '\0';
+        writeRecursivePaths(fdRecursivePaths, s);
+    }
+}
+
+
+void RemoteStore::reportRecursivePaths(const PathSet & paths)
+{
+    if (fdRecursivePaths != -1 && !paths.empty()) {
+        string s = "";
+        foreach (PathSet::const_iterator, i, recursivePaths) {
+            if (s.size() + i->size() >= 4096) {
+                if (i->size() >= 4096)
+                    throw Error("reporting a path name bigger than 4096 bytes not allowed");
+                writeRecursivePaths(fdRecursivePaths, s);
+                s = "";
+            }
+            s += *i + '\0';
+        }
+        writeRecursivePaths(fdRecursivePaths, s);
+    }
+}
+
+
 void RemoteStore::processStderr(Sink * sink, Source * source)
 {
     to.flush();
diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh
index a7d84266c88..7dddaf41e35 100644
--- a/src/libstore/remote-store.hh
+++ b/src/libstore/remote-store.hh
@@ -97,6 +97,10 @@ private:
 
     void processStderr(Sink * sink = 0, Source * source = 0);
 
+    void reportRecursivePath(const Path & path);
+
+    void reportRecursivePaths(const PathSet & paths);
+
     void connectToDaemon();
 
     void setOptions();

From ce51fc3248b133ca7e1436ff4e3c9c7ee7175d12 Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Wed, 26 Feb 2014 16:36:14 -0500
Subject: [PATCH 15/20] Report results of building/ensuring paths

Also fix a dumb bug
---
 src/libstore/remote-store.cc | 16 +++++++++++++++-
 src/libstore/remote-store.hh |  1 -
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index b99c09f3121..4fdef74362f 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -6,6 +6,7 @@
 #include "affinity.hh"
 #include "globals.hh"
 #include "pathlocks.hh"
+#include "derivations.hh"
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -557,6 +558,17 @@ void RemoteStore::buildPaths(const PathSet & drvPaths, BuildMode buildMode)
     }
     processStderr();
     readInt(from);
+
+    /* Report paths again so new ones can be added to chroot */
+    PathSet recursivePaths;
+    foreach (PathSet::const_iterator, i, drvPaths) {
+        DrvPathWithOutputs i2 = parseDrvPathWithOutputs(*i);
+        if (isDerivation(i2.first))
+            recursivePaths.insert(i2.first);
+        else
+            recursivePaths.insert(*i);
+    }
+    reportRecursivePaths(recursivePaths);
 }
 
 
@@ -567,6 +579,8 @@ void RemoteStore::ensurePath(const Path & path)
     writeString(path, to);
     processStderr();
     readInt(from);
+    /* Report path again so it can be added to chroot if new */
+    reportRecursivePath(path);
 }
 
 
@@ -685,7 +699,7 @@ void RemoteStore::reportRecursivePaths(const PathSet & paths)
 {
     if (fdRecursivePaths != -1 && !paths.empty()) {
         string s = "";
-        foreach (PathSet::const_iterator, i, recursivePaths) {
+        foreach (PathSet::const_iterator, i, paths) {
             if (s.size() + i->size() >= 4096) {
                 if (i->size() >= 4096)
                     throw Error("reporting a path name bigger than 4096 bytes not allowed");
diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh
index 7dddaf41e35..2d6c0190aea 100644
--- a/src/libstore/remote-store.hh
+++ b/src/libstore/remote-store.hh
@@ -91,7 +91,6 @@ private:
     unsigned int daemonVersion;
     bool initialised;
     AutoCloseFD fdRecursivePaths;
-    PathSet recursivePaths;
 
     void openConnection(bool reserveSpace = true);
 

From 6fda1f7ede0d8e35c5db2cf1b1906ddca66700e6 Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Wed, 26 Feb 2014 16:53:14 -0500
Subject: [PATCH 16/20] Remove now-unused argument

---
 src/libstore/pathlocks.cc | 4 ++--
 src/libstore/pathlocks.hh | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/libstore/pathlocks.cc b/src/libstore/pathlocks.cc
index 14a2b125ad3..a7a60fe123a 100644
--- a/src/libstore/pathlocks.cc
+++ b/src/libstore/pathlocks.cc
@@ -12,11 +12,11 @@
 namespace nix {
 
 
-int openLockFile(const Path & path, bool create, bool append)
+int openLockFile(const Path & path, bool create)
 {
     AutoCloseFD fd;
 
-    fd = open(path.c_str(), O_RDWR | (create ? O_CREAT : 0) | (append ? O_APPEND : 0), 0600);
+    fd = open(path.c_str(), O_RDWR | (create ? O_CREAT : 0), 0600);
     if (fd == -1 && (create || errno != ENOENT))
         throw SysError(format("opening lock file `%1%'") % path);
 
diff --git a/src/libstore/pathlocks.hh b/src/libstore/pathlocks.hh
index 3a2d3b3377f..8a6b1450da2 100644
--- a/src/libstore/pathlocks.hh
+++ b/src/libstore/pathlocks.hh
@@ -9,7 +9,7 @@ namespace nix {
 /* Open (possibly create) a lock file and return the file descriptor.
    -1 is returned if create is false and the lock could not be opened
    because it doesn't exist.  Any other error throws an exception. */
-int openLockFile(const Path & path, bool create, bool append = false);
+int openLockFile(const Path & path, bool create);
 
 /* Delete an open lock file. */
 void deleteLockFile(const Path & path, int fd);

From 4e090474d730ac1664f842a7bbfc47a37064a1d3 Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Thu, 27 Feb 2014 12:45:15 -0500
Subject: [PATCH 17/20] Sending fds is shockingly non-portable

---
 src/libstore/remote-store.cc    |  6 +++---
 src/libstore/worker-protocol.hh | 27 ++++++++++++++++++++-------
 src/nix-daemon/nix-daemon.cc    |  4 ++--
 3 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index 4fdef74362f..5da75ebcb54 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -84,11 +84,11 @@ void RemoteStore::openConnection(bool reserveSpace)
         memset(&msg, 0, sizeof msg);
         msg.msg_iov = &iov;
         msg.msg_iovlen = 1;
-        ancillary data;
-        msg.msg_control = &data;
+        char data[CMSG_SPACE(sizeof fds[1])];
+        msg.msg_control = data;
         msg.msg_controllen = sizeof data;
         struct cmsghdr *cmsg = CMSG_FIRSTHDR(&msg);
-        cmsg->cmsg_len = msg.msg_controllen;
+        cmsg->cmsg_len = CMSG_LEN(sizeof fds[1]);
         cmsg->cmsg_level = SOL_SOCKET;
         cmsg->cmsg_type = SCM_RIGHTS;
         memmove(CMSG_DATA(cmsg), &fds[1], sizeof fds[1]);
diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh
index 538056bec16..4a366ab9b94 100644
--- a/src/libstore/worker-protocol.hh
+++ b/src/libstore/worker-protocol.hh
@@ -1,13 +1,6 @@
 #pragma once
 #include <sys/socket.h>
 
-extern "C" {
-    struct ancillary {
-        struct cmsghdr hdr;
-        int fd;
-    };
-}
-
 namespace nix {
 
 
@@ -67,3 +60,23 @@ template<class T> T readStorePaths(Source & from);
 
 
 }
+
+/* Borrowed from http://swtch.com/usr/local/plan9/src/lib9/sendfd.c, should
+   really be part of POSIX.
+   Copyright (C) 2003, Lucent Technologies Inc. and others. All Rights Reserved. */
+
+#ifndef CMSG_ALIGN
+#   ifdef __sun__
+#       define CMSG_ALIGN _CMSG_DATA_ALIGN
+#   else
+#       define CMSG_ALIGN(len) (((len)+sizeof(long)-1) & ~(sizeof(long)-1))
+#   endif
+#endif
+
+#ifndef CMSG_SPACE
+#   define CMSG_SPACE(len) (CMSG_ALIGN(sizeof(struct cmsghdr))+CMSG_ALIGN(len))
+#endif
+
+#ifndef CMSG_LEN
+#   define CMSG_LEN(len) (CMSG_ALIGN(sizeof(struct cmsghdr))+(len))
+#endif
diff --git a/src/nix-daemon/nix-daemon.cc b/src/nix-daemon/nix-daemon.cc
index 2cbbe9754ef..5f8d22582b2 100644
--- a/src/nix-daemon/nix-daemon.cc
+++ b/src/nix-daemon/nix-daemon.cc
@@ -939,8 +939,8 @@ static void daemonLoop()
                 memset(&msg, 0, sizeof msg);
                 msg.msg_iov = &iov;
                 msg.msg_iovlen = 1;
-                ancillary data;
-                msg.msg_control = &data;
+                char data[CMSG_SPACE(sizeof fdRecursiveFrom)];
+                msg.msg_control = data;
                 msg.msg_controllen = sizeof data;
                 ssize_t count = recvmsg(fdRecursiveFrom, &msg, 0);
                 if (count == -1)

From 90433876c8b86780481e7dc5ce3fdce8c5090e86 Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Thu, 27 Feb 2014 12:46:19 -0500
Subject: [PATCH 18/20] Mount recursive paths in the child's namespace, not the
 parents

---
 src/libstore/build.cc | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index 00add94d7ec..d5fae1c2386 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -2531,6 +2531,23 @@ void DerivationGoal::handleChildOutput(int fd, const string & data)
             pos = end + 1;
         }
 
+#if CHROOT_ENABLED
+        AutoCloseFD myNs, childNs;
+        if (useChroot && !newPaths.empty()) {
+            /* Enter the child's mount namespace */
+            myNs = open("/proc/self/ns/mnt", O_RDONLY);
+            if (myNs == -1)
+                throw SysError("opening own mount namespace");
+            Path childNsPath = (format("/proc/%1%/ns/mnt") % pid).str();
+            childNs = open(childNsPath.c_str(), O_RDONLY);
+            if (childNs == -1)
+                throw SysError("opening child's mount namespace");
+
+            if (setns(childNs, 0) == -1)
+                throw SysError("entering child's mount namespace");
+        }
+#endif
+
         foreach (PathSet::iterator, i, newPaths) {
             worker.store.addTempRoot(*i);
 #if CHROOT_ENABLED
@@ -2566,6 +2583,14 @@ void DerivationGoal::handleChildOutput(int fd, const string & data)
 #endif
         }
 
+#if CHROOT_ENABLED
+        if (myNs != -1) {
+            /* Back in our own namespace */
+            if (setns(myNs, 0) == -1)
+                throw SysError("leaving child's mount namespace");
+        }
+#endif
+
         allPaths.insert(newPaths.begin(), newPaths.end());
 
         /* Notify the child that we're done */

From 86bb85b2f46c84111af3a9053f1421d411b70815 Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Thu, 27 Feb 2014 16:36:34 -0500
Subject: [PATCH 19/20] Be stingier reporting recursive paths

Recursive path reporting is expensive, especially in the chroot case. So
now we only report valid paths and are less defensive about derivations
doing stupid things with paths they aren't sure are valid.
---
 src/libstore/build.cc        |   9 ++--
 src/libstore/remote-store.cc | 102 ++++++++++++++++++++---------------
 src/libstore/remote-store.hh |   4 ++
 3 files changed, 65 insertions(+), 50 deletions(-)

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index d5fae1c2386..e81b44d915e 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -2524,16 +2524,13 @@ void DerivationGoal::handleChildOutput(int fd, const string & data)
         while ((end = data.find((char) 0, pos)) != string::npos) {
             Path r(data, pos, end - pos);
             debug(format("got recursive path `%1%'") % r);
-            assertStorePath(r);
-            /* If a derivation was made visible to the build, we
-               need to also check for its outputs */
-            computeFSClosure(worker.store, r, newPaths, false, true);
+            computeFSClosure(worker.store, r, newPaths);
             pos = end + 1;
         }
 
 #if CHROOT_ENABLED
         AutoCloseFD myNs, childNs;
-        if (useChroot && !newPaths.empty()) {
+        if (useChroot) {
             /* Enter the child's mount namespace */
             myNs = open("/proc/self/ns/mnt", O_RDONLY);
             if (myNs == -1)
@@ -2551,7 +2548,7 @@ void DerivationGoal::handleChildOutput(int fd, const string & data)
         foreach (PathSet::iterator, i, newPaths) {
             worker.store.addTempRoot(*i);
 #if CHROOT_ENABLED
-            if (useChroot && worker.store.isValidPath(*i)) {
+            if (useChroot) {
                 /* We need this path to be available in the chroot */
                 Path target = chrootRootDir + *i;
                 if (access(target.c_str(), F_OK) == 0)
diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index 5da75ebcb54..83912622401 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -7,6 +7,7 @@
 #include "globals.hh"
 #include "pathlocks.hh"
 #include "derivations.hh"
+#include "errno.h"
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -298,8 +299,6 @@ void RemoteStore::querySubstitutablePathInfos(const PathSet & paths,
 
     if (GET_PROTOCOL_MINOR(daemonVersion) < 3) return;
 
-    PathSet recursivePaths;
-
     if (GET_PROTOCOL_MINOR(daemonVersion) < 12) {
 
         foreach (PathSet::const_iterator, i, paths) {
@@ -310,12 +309,8 @@ void RemoteStore::querySubstitutablePathInfos(const PathSet & paths,
             unsigned int reply = readInt(from);
             if (reply == 0) continue;
             info.deriver = readString(from);
-            if (info.deriver != "") {
-                assertStorePath(info.deriver);
-                recursivePaths.insert(info.deriver);
-            }
+            if (info.deriver != "") assertStorePath(info.deriver);
             info.references = readStorePaths<PathSet>(from);
-            recursivePaths.insert(info.references.begin(), info.references.end());
             info.downloadSize = readLongLong(from);
             info.narSize = GET_PROTOCOL_MINOR(daemonVersion) >= 7 ? readLongLong(from) : 0;
             infos[*i] = info;
@@ -331,19 +326,13 @@ void RemoteStore::querySubstitutablePathInfos(const PathSet & paths,
             Path path = readStorePath(from);
             SubstitutablePathInfo & info(infos[path]);
             info.deriver = readString(from);
-            if (info.deriver != "") {
-                assertStorePath(info.deriver);
-                recursivePaths.insert(info.deriver);
-            }
+            if (info.deriver != "") assertStorePath(info.deriver);
             info.references = readStorePaths<PathSet>(from);
-            recursivePaths.insert(info.references.begin(), info.references.end());
             info.downloadSize = readLongLong(from);
             info.narSize = readLongLong(from);
         }
 
     }
-
-    reportRecursivePaths(recursivePaths);
 }
 
 
@@ -356,18 +345,15 @@ ValidPathInfo RemoteStore::queryPathInfo(const Path & path)
     ValidPathInfo info;
     info.path = path;
     info.deriver = readString(from);
-    PathSet recursivePaths;
     if (info.deriver != "") {
         assertStorePath(info.deriver);
-        recursivePaths.insert(info.deriver);
+        if (fdRecursivePaths != -1 && isValidPath(info.deriver))
+            _reportRecursivePath(info.deriver);
     }
     info.hash = parseHash(htSHA256, readString(from));
     info.references = readStorePaths<PathSet>(from);
-    recursivePaths.insert(info.references.begin(), info.references.end());
     info.registrationTime = readInt(from);
     info.narSize = readLongLong(from);
-
-    reportRecursivePaths(recursivePaths);
     return info;
 }
 
@@ -391,7 +377,7 @@ void RemoteStore::queryReferences(const Path & path,
     writeString(path, to);
     processStderr();
     PathSet references2 = readStorePaths<PathSet>(from);
-    reportRecursivePaths(references2);
+    /* References are brought in automatically, no need to report */
     references.insert(references2.begin(), references2.end());
 }
 
@@ -404,7 +390,7 @@ void RemoteStore::queryReferrers(const Path & path,
     writeString(path, to);
     processStderr();
     PathSet referrers2 = readStorePaths<PathSet>(from);
-    /* no need to add to recursive paths, they're brought in automatically */
+    reportRecursivePaths(referrers2);
     referrers.insert(referrers2.begin(), referrers2.end());
 }
 
@@ -418,7 +404,8 @@ Path RemoteStore::queryDeriver(const Path & path)
     Path drvPath = readString(from);
     if (drvPath != "") {
         assertStorePath(drvPath);
-        reportRecursivePath(drvPath);
+        if (fdRecursivePaths != -1 && isValidPath(drvPath))
+            _reportRecursivePath(drvPath);
     }
     return drvPath;
 }
@@ -442,10 +429,8 @@ PathSet RemoteStore::queryDerivationOutputs(const Path & path)
     writeInt(wopQueryDerivationOutputs, to);
     writeString(path, to);
     processStderr();
-    /* derivations are treated specially: Unless they became accessible
-       via unsafeDiscardOutputDependency, their outputs are treated as
-       inputs to scan for even if, in the case of recursive nix, they
-       may not be valid. So no need to add them to recursivePaths */
+    /* Paths determined via queryDerivationOutputs can't be assumed valid,
+       so we don't report them */
     return readStorePaths<PathSet>(from);
 }
 
@@ -559,16 +544,34 @@ void RemoteStore::buildPaths(const PathSet & drvPaths, BuildMode buildMode)
     processStderr();
     readInt(from);
 
-    /* Report paths again so new ones can be added to chroot */
-    PathSet recursivePaths;
-    foreach (PathSet::const_iterator, i, drvPaths) {
-        DrvPathWithOutputs i2 = parseDrvPathWithOutputs(*i);
-        if (isDerivation(i2.first))
-            recursivePaths.insert(i2.first);
-        else
-            recursivePaths.insert(*i);
+    if (fdRecursivePaths != -1) {
+        PathSet recursivePaths;
+        foreach (PathSet::const_iterator, i, drvPaths) {
+            DrvPathWithOutputs i2 = parseDrvPathWithOutputs(*i);
+            if (isDerivation(i2.first)) {
+                Derivation drv;
+                try {
+                    drv = parseDerivation(readFile(i2.first));
+                    recursivePaths.insert(i2.first);
+                } catch (SysError & e) {
+                    if (e.errNo == ENOENT) {
+                        /* Need to ask for the drv to be added to chroot */
+                        _reportRecursivePath(i2.first);
+                        drv = parseDerivation(readFile(i2.first));
+                    } else
+                        throw;
+                }
+                if (i2.second.empty())
+                    foreach (DerivationOutputs::iterator, j, drv.outputs)
+                        recursivePaths.insert(j->second.path);
+                else
+                    foreach (StringSet::iterator, j, i2.second)
+                        recursivePaths.insert(drv.outputs[*j].path);
+            } else
+                recursivePaths.insert(*i);
+        }
+        _reportRecursivePaths(recursivePaths);
     }
-    reportRecursivePaths(recursivePaths);
 }
 
 
@@ -579,7 +582,6 @@ void RemoteStore::ensurePath(const Path & path)
     writeString(path, to);
     processStderr();
     readInt(from);
-    /* Report path again so it can be added to chroot if new */
     reportRecursivePath(path);
 }
 
@@ -684,20 +686,25 @@ static void writeRecursivePaths(int fd, const string & s) {
 }
 
 
+void RemoteStore::_reportRecursivePath(const Path & path)
+{
+    if (path.size() >= 4096)
+        throw Error("reporting a path name bigger than 4096 bytes not allowed");
+    string s = path + '\0';
+    writeRecursivePaths(fdRecursivePaths, s);
+}
+
+
 void RemoteStore::reportRecursivePath(const Path & path)
 {
-    if (fdRecursivePaths != -1) {
-        if (path.size() >= 4096)
-            throw Error("reporting a path name bigger than 4096 bytes not allowed");
-        string s = path + '\0';
-        writeRecursivePaths(fdRecursivePaths, s);
-    }
+    if (fdRecursivePaths != -1)
+        _reportRecursivePath(path);
 }
 
 
-void RemoteStore::reportRecursivePaths(const PathSet & paths)
+void RemoteStore::_reportRecursivePaths(const PathSet & paths)
 {
-    if (fdRecursivePaths != -1 && !paths.empty()) {
+    if (!paths.empty()) {
         string s = "";
         foreach (PathSet::const_iterator, i, paths) {
             if (s.size() + i->size() >= 4096) {
@@ -713,6 +720,13 @@ void RemoteStore::reportRecursivePaths(const PathSet & paths)
 }
 
 
+void RemoteStore::reportRecursivePaths(const PathSet & paths)
+{
+    if (fdRecursivePaths != -1)
+        _reportRecursivePaths(paths);
+}
+
+
 void RemoteStore::processStderr(Sink * sink, Source * source)
 {
     to.flush();
diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh
index 2d6c0190aea..6a4fcb01b44 100644
--- a/src/libstore/remote-store.hh
+++ b/src/libstore/remote-store.hh
@@ -96,6 +96,10 @@ private:
 
     void processStderr(Sink * sink = 0, Source * source = 0);
 
+    void _reportRecursivePath(const Path & path);
+
+    void _reportRecursivePaths(const PathSet & paths);
+
     void reportRecursivePath(const Path & path);
 
     void reportRecursivePaths(const PathSet & paths);

From dccc4614ac2f08d61cd2a52bd57b6e7cb7af3460 Mon Sep 17 00:00:00 2001
From: Shea Levy <shea@shealevy.com>
Date: Thu, 27 Feb 2014 16:48:07 -0500
Subject: [PATCH 20/20] Be more defensive when reading recursive paths

Builder processes can purposefully cause an exception to be thrown in
any number of ways, so don't let a rogue builder kill an entire store
process.
---
 src/libstore/build.cc | 128 +++++++++++++++++++++---------------------
 1 file changed, 65 insertions(+), 63 deletions(-)

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index e81b44d915e..e0d1e8e1fad 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -2516,68 +2516,81 @@ void DerivationGoal::handleChildOutput(int fd, const string & data)
         } else if (fdLogFile != -1)
             writeFull(fdLogFile, (unsigned char *) data.data(), data.size());
     } else if (!hook && fd == recursivePathsParentSide) {
-        /* Extract the paths. */
-        string::size_type pos = 0, end;
+#if CHROOT_ENABLED
+        AutoCloseFD myNs;
+#endif
+        try {
+            /* Extract the paths. */
+            string::size_type pos = 0, end;
 
-        PathSet newPaths;
+            PathSet newPaths;
 
-        while ((end = data.find((char) 0, pos)) != string::npos) {
-            Path r(data, pos, end - pos);
-            debug(format("got recursive path `%1%'") % r);
-            computeFSClosure(worker.store, r, newPaths);
-            pos = end + 1;
-        }
+            while ((end = data.find((char) 0, pos)) != string::npos) {
+                Path r(data, pos, end - pos);
+                debug(format("got recursive path `%1%'") % r);
+                computeFSClosure(worker.store, r, newPaths);
+                pos = end + 1;
+            }
 
 #if CHROOT_ENABLED
-        AutoCloseFD myNs, childNs;
-        if (useChroot) {
-            /* Enter the child's mount namespace */
-            myNs = open("/proc/self/ns/mnt", O_RDONLY);
-            if (myNs == -1)
-                throw SysError("opening own mount namespace");
-            Path childNsPath = (format("/proc/%1%/ns/mnt") % pid).str();
-            childNs = open(childNsPath.c_str(), O_RDONLY);
-            if (childNs == -1)
-                throw SysError("opening child's mount namespace");
-
-            if (setns(childNs, 0) == -1)
-                throw SysError("entering child's mount namespace");
-        }
+            if (useChroot) {
+                /* Enter the child's mount namespace */
+                myNs = open("/proc/self/ns/mnt", O_RDONLY);
+                if (myNs == -1)
+                    throw SysError("opening own mount namespace");
+                Path childNsPath = (format("/proc/%1%/ns/mnt") % pid).str();
+                AutoCloseFD childNs = open(childNsPath.c_str(), O_RDONLY);
+                if (childNs == -1)
+                    throw SysError("opening child's mount namespace");
+
+                if (setns(childNs, 0) == -1)
+                    throw SysError("entering child's mount namespace");
+            }
 #endif
 
-        foreach (PathSet::iterator, i, newPaths) {
-            worker.store.addTempRoot(*i);
+            foreach (PathSet::iterator, i, newPaths) {
+                worker.store.addTempRoot(*i);
 #if CHROOT_ENABLED
-            if (useChroot) {
-                /* We need this path to be available in the chroot */
-                Path target = chrootRootDir + *i;
-                if (access(target.c_str(), F_OK) == 0)
-                    /* Path already exists, assume it's the right one */
-                    continue;
-                struct stat st;
-                if (lstat(i->c_str(), &st))
-                    throw SysError(format("getting attributes of path `%1%'") % *i);
-                if (S_ISDIR(st.st_mode)) {
-                    debug(format("bind mounting `%1%' to `%2%'") % *i % target);
-                    createDirs(target);
-                    if (mount(i->c_str(), target.c_str(), "", MS_BIND, 0) == -1)
-                        throw SysError(format("bind mount from `%1%' to `%2%' failed") % *i % target);
-                } else {
-                    if (link(i->c_str(), target.c_str()) == -1) {
-                        /* Hard-linking fails if we exceed the maximum
-                           link count on a file (e.g. 32000 of ext3),
-                           which is quite possible after a `nix-store
-                           --optimise'. */
-                        if (errno != EMLINK)
-                            throw SysError(format("linking `%1%' to `%2%'") % target % *i);
-                        StringSink sink;
-                        dumpPath(*i, sink);
-                        StringSource source(sink.s);
-                        restorePath(target, source);
+                if (useChroot) {
+                    /* We need this path to be available in the chroot */
+                    Path target = chrootRootDir + *i;
+                    if (access(target.c_str(), F_OK) == 0)
+                        /* Path already exists, assume it's the right one */
+                        continue;
+                    struct stat st;
+                    if (lstat(i->c_str(), &st))
+                        throw SysError(format("getting attributes of path `%1%'") % *i);
+                    if (S_ISDIR(st.st_mode)) {
+                        debug(format("bind mounting `%1%' to `%2%'") % *i % target);
+                        createDirs(target);
+                        if (mount(i->c_str(), target.c_str(), "", MS_BIND, 0) == -1)
+                            throw SysError(format("bind mount from `%1%' to `%2%' failed") % *i % target);
+                    } else {
+                        if (link(i->c_str(), target.c_str()) == -1) {
+                            /* Hard-linking fails if we exceed the maximum
+                               link count on a file (e.g. 32000 of ext3),
+                               which is quite possible after a `nix-store
+                               --optimise'. */
+                            if (errno != EMLINK)
+                                throw SysError(format("linking `%1%' to `%2%'") % target % *i);
+                            StringSink sink;
+                            dumpPath(*i, sink);
+                            StringSource source(sink.s);
+                            restorePath(target, source);
+                        }
                     }
                 }
-            }
 #endif
+            }
+
+            allPaths.insert(newPaths.begin(), newPaths.end());
+
+            /* Notify the child that we're done */
+            unsigned char c = '\0';
+            writeFull(fd, &c, sizeof c);
+        } catch (std::exception & e) {
+            /* Don't let a rogue builder kill us */
+            printMsg(lvlError, format("error getting reported recursive paths: %1%") % e.what());
         }
 
 #if CHROOT_ENABLED
@@ -2587,17 +2600,6 @@ void DerivationGoal::handleChildOutput(int fd, const string & data)
                 throw SysError("leaving child's mount namespace");
         }
 #endif
-
-        allPaths.insert(newPaths.begin(), newPaths.end());
-
-        /* Notify the child that we're done */
-        unsigned char c = '\0';
-        try {
-            writeFull(fd, &c, sizeof c);
-        } catch (SysError & e) {
-            /* Don't let a rogue builder kill us */
-            printMsg(lvlError, format("error confirming recursive paths read: %1%") % e.what());
-        }
     } else if (hook && fd == hook->fromHook.readSide)
         writeToStderr(data);
 }