Merge pull request #5149 from edolstra/non-blocking-gc

Non-blocking garbage collector

flake.nix

@@ -489,12 +489,7 @@
             '';
         */
 
-      };
+        installTests = forAllSystems (system:
-
-      checks = forAllSystems (system: {
-        binaryTarball = self.hydraJobs.binaryTarball.${system};
-        perlBindings = self.hydraJobs.perlBindings.${system};
-        installTests =
           let pkgs = nixpkgsFor.${system}; in
           pkgs.runCommand "install-tests" {
             againstSelf = testNixVersions pkgs pkgs.nix pkgs.pkgs.nix;
@@ -506,7 +501,14 @@
             # Disabled because the latest stable version doesn't handle
             # `NIX_DAEMON_SOCKET_PATH` which is required for the tests to work
             # againstLatestStable = testNixVersions pkgs pkgs.nix pkgs.nixStable;
-          } "touch $out";
+          } "touch $out");
+
+      };
+
+      checks = forAllSystems (system: {
+        binaryTarball = self.hydraJobs.binaryTarball.${system};
+        perlBindings = self.hydraJobs.perlBindings.${system};
+        installTests = self.hydraJobs.installTests.${system};
       });
 
       packages = forAllSystems (system: {

src/libstore/binary-cache-store.cc

@@ -111,15 +111,15 @@ void BinaryCacheStore::writeNarInfo(ref<NarInfo> narInfo)
 
     upsertFile(narInfoFile, narInfo->to_string(*this), "text/x-nix-narinfo");
 
-    std::string hashPart(narInfo->path.hashPart());
-
     {
         auto state_(state.lock());
-        state_->pathInfoCache.upsert(hashPart, PathInfoCacheValue { .value = std::shared_ptr<NarInfo>(narInfo) });
+        state_->pathInfoCache.upsert(
+            std::string(narInfo->path.to_string()),
+            PathInfoCacheValue { .value = std::shared_ptr<NarInfo>(narInfo) });
     }
 
     if (diskCache)
-        diskCache->upsertNarInfo(getUri(), hashPart, std::shared_ptr<NarInfo>(narInfo));
+        diskCache->upsertNarInfo(getUri(), std::string(narInfo->path.hashPart()), std::shared_ptr<NarInfo>(narInfo));
 }
 
 AutoCloseFD openFile(const Path & path)

src/libstore/build/local-derivation-goal.cc

@@ -1353,7 +1353,7 @@ void LocalDerivationGoal::startDaemon()
             AutoCloseFD remote = accept(daemonSocket.get(),
                 (struct sockaddr *) &remoteAddr, &remoteAddrLen);
             if (!remote) {
-                if (errno == EINTR) continue;
+                if (errno == EINTR || errno == EAGAIN) continue;
                 if (errno == EINVAL) break;
                 throw SysError("accepting connection");
             }

src/libstore/daemon.cc

@@ -625,9 +625,9 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
         break;
     }
 
+    // Obsolete.
     case wopSyncWithGC: {
         logger->startWork();
-        store->syncWithGC();
         logger->stopWork();
         to << 1;
         break;

src/libstore/gc.cc

@@ -10,48 +10,22 @@
 #include <regex>
 #include <random>
 
-#include <sys/types.h>
+#include <climits>
-#include <sys/stat.h>
-#include <sys/statvfs.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <poll.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/statvfs.h>
+#include <sys/types.h>
+#include <sys/un.h>
 #include <unistd.h>
-#include <climits>
 
 namespace nix {
 
 
-static string gcLockName = "gc.lock";
+static std::string gcSocketPath = "/gc-socket/socket";
-static string gcRootsDir = "gcroots";
+static std::string gcRootsDir = "gcroots";
-
-
-/* Acquire the global GC lock.  This is used to prevent new Nix
-   processes from starting after the temporary root files have been
-   read.  To be precise: when they try to create a new temporary root
-   file, they will block until the garbage collector has finished /
-   yielded the GC lock. */
-AutoCloseFD LocalStore::openGCLock(LockType lockType)
-{
-    Path fnGCLock = (format("%1%/%2%")
-        % stateDir % gcLockName).str();
-
-    debug(format("acquiring global GC lock '%1%'") % fnGCLock);
-
-    AutoCloseFD fdGCLock = open(fnGCLock.c_str(), O_RDWR | O_CREAT | O_CLOEXEC, 0600);
-    if (!fdGCLock)
-        throw SysError("opening global GC lock '%1%'", fnGCLock);
-
-    if (!lockFile(fdGCLock.get(), lockType, false)) {
-        printInfo("waiting for the big garbage collector lock...");
-        lockFile(fdGCLock.get(), lockType, true);
-    }
-
-    /* !!! Restrict read permission on the GC root.  Otherwise any
-       process that can open the file for reading can DoS the
-       collector. */
-
-    return fdGCLock;
-}
 
 
 static void makeSymlink(const Path & link, const Path & target)
@@ -71,12 +45,6 @@ static void makeSymlink(const Path & link, const Path & target)
 }
 
 
-void LocalStore::syncWithGC()
-{
-    AutoCloseFD fdGCLock = openGCLock(ltRead);
-}
-
-
 void LocalStore::addIndirectRoot(const Path & path)
 {
     string hash = hashString(htSHA1, path).to_string(Base32, false);
@@ -95,6 +63,12 @@ Path LocalFSStore::addPermRoot(const StorePath & storePath, const Path & _gcRoot
                 "creating a garbage collector root (%1%) in the Nix store is forbidden "
                 "(are you running nix-build inside the store?)", gcRoot);
 
+    /* Register this root with the garbage collector, if it's
+       running. This should be superfluous since the caller should
+       have registered this root yet, but let's be on the safe
+       side. */
+    addTempRoot(storePath);
+
     /* Don't clobber the link if it already exists and doesn't
        point to the Nix store. */
     if (pathExists(gcRoot) && (!isLink(gcRoot) || !isInStore(readLink(gcRoot))))
@@ -102,11 +76,6 @@ Path LocalFSStore::addPermRoot(const StorePath & storePath, const Path & _gcRoot
     makeSymlink(gcRoot, printStorePath(storePath));
     addIndirectRoot(gcRoot);
 
-    /* Grab the global GC root, causing us to block while a GC is in
-       progress.  This prevents the set of permanent roots from
-       increasing while a GC is in progress. */
-    syncWithGC();
-
     return gcRoot;
 }
 
@@ -119,8 +88,6 @@ void LocalStore::addTempRoot(const StorePath & path)
     if (!state->fdTempRoots) {
 
         while (1) {
-            AutoCloseFD fdGCLock = openGCLock(ltRead);
-
             if (pathExists(fnTempRoots))
                 /* It *must* be stale, since there can be no two
                    processes with the same pid. */
@@ -128,10 +95,8 @@ void LocalStore::addTempRoot(const StorePath & path)
 
             state->fdTempRoots = openLockFile(fnTempRoots, true);
 
-            fdGCLock = -1;
+            debug("acquiring write lock on '%s'", fnTempRoots);
-
+            lockFile(state->fdTempRoots.get(), ltWrite, true);
-            debug(format("acquiring read lock on '%1%'") % fnTempRoots);
-            lockFile(state->fdTempRoots.get(), ltRead, true);
 
             /* Check whether the garbage collector didn't get in our
                way. */
@@ -147,24 +112,55 @@ void LocalStore::addTempRoot(const StorePath & path)
 
     }
 
-    /* Upgrade the lock to a write lock.  This will cause us to block
+    if (!state->fdGCLock)
-       if the garbage collector is holding our lock. */
+        state->fdGCLock = openGCLock();
-    debug(format("acquiring write lock on '%1%'") % fnTempRoots);
-    lockFile(state->fdTempRoots.get(), ltWrite, true);
 
+ restart:
+    FdLock gcLock(state->fdGCLock.get(), ltRead, false, "");
+
+    if (!gcLock.acquired) {
+        /* We couldn't get a shared global GC lock, so the garbage
+           collector is running. So we have to connect to the garbage
+           collector and inform it about our root. */
+        if (!state->fdRootsSocket) {
+            auto socketPath = stateDir.get() + gcSocketPath;
+            debug("connecting to '%s'", socketPath);
+            state->fdRootsSocket = createUnixDomainSocket();
+            nix::connect(state->fdRootsSocket.get(), socketPath);
+        }
+
+        try {
+            debug("sending GC root '%s'", printStorePath(path));
+            writeFull(state->fdRootsSocket.get(), printStorePath(path) + "\n", false);
+            char c;
+            readFull(state->fdRootsSocket.get(), &c, 1);
+            assert(c == '1');
+            debug("got ack for GC root '%s'", printStorePath(path));
+        } catch (SysError & e) {
+            /* The garbage collector may have exited, so we need to
+               restart. */
+            if (e.errNo == EPIPE) {
+                debug("GC socket disconnected");
+                state->fdRootsSocket.close();
+                goto restart;
+            }
+        } catch (EndOfFile & e) {
+            debug("GC socket disconnected");
+            state->fdRootsSocket.close();
+            goto restart;
+        }
+    }
+
+    /* Append the store path to the temporary roots file. */
     string s = printStorePath(path) + '\0';
     writeFull(state->fdTempRoots.get(), s);
-
-    /* Downgrade to a read lock. */
-    debug(format("downgrading to read lock on '%1%'") % fnTempRoots);
-    lockFile(state->fdTempRoots.get(), ltRead, true);
 }
 
 
 static std::string censored = "{censored}";
 
 
-void LocalStore::findTempRoots(FDs & fds, Roots & tempRoots, bool censor)
+void LocalStore::findTempRoots(Roots & tempRoots, bool censor)
 {
     /* Read the `temproots' directory for per-process temporary root
        files. */
@@ -179,35 +175,25 @@ void LocalStore::findTempRoots(FDs & fds, Roots & tempRoots, bool censor)
         pid_t pid = std::stoi(i.name);
 
         debug(format("reading temporary root file '%1%'") % path);
-        FDPtr fd(new AutoCloseFD(open(path.c_str(), O_CLOEXEC | O_RDWR, 0666)));
+        AutoCloseFD fd(open(path.c_str(), O_CLOEXEC | O_RDWR, 0666));
-        if (!*fd) {
+        if (!fd) {
             /* It's okay if the file has disappeared. */
             if (errno == ENOENT) continue;
             throw SysError("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->get(), ltWrite, false)) {
+        if (lockFile(fd.get(), ltWrite, false)) {
             printInfo("removing stale temporary roots file '%1%'", path);
             unlink(path.c_str());
-            writeFull(fd->get(), "d");
+            writeFull(fd.get(), "d");
             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%'") % path);
-        lockFile(fd->get(), ltRead, true);
-
         /* Read the entire file. */
-        string contents = readFile(fd->get());
+        string contents = readFile(fd.get());
 
         /* Extract the roots. */
         string::size_type pos = 0, end;
@@ -218,8 +204,6 @@ void LocalStore::findTempRoots(FDs & fds, Roots & tempRoots, bool censor)
             tempRoots[parseStorePath(root)].emplace(censor ? censored : fmt("{temp:%d}", pid));
             pos = end + 1;
         }
-
-        fds.push_back(fd); /* keep open */
     }
 }
 
@@ -304,8 +288,7 @@ Roots LocalStore::findRoots(bool censor)
     Roots roots;
     findRootsNoTemp(roots, censor);
 
-    FDs fds;
+    findTempRoots(roots, censor);
-    findTempRoots(fds, roots, censor);
 
     return roots;
 }
@@ -455,203 +438,362 @@ void LocalStore::findRuntimeRoots(Roots & roots, bool censor)
 struct GCLimitReached { };
 
 
-struct LocalStore::GCState
+void LocalStore::collectGarbage(const GCOptions & options, GCResults & results)
 {
-    const GCOptions & options;
+    bool shouldDelete = options.action == GCOptions::gcDeleteDead || options.action == GCOptions::gcDeleteSpecific;
-    GCResults & results;
+    bool gcKeepOutputs = settings.gcKeepOutputs;
-    StorePathSet roots;
+    bool gcKeepDerivations = settings.gcKeepDerivations;
-    StorePathSet tempRoots;
-    StorePathSet dead;
-    StorePathSet alive;
-    bool gcKeepOutputs;
-    bool gcKeepDerivations;
-    uint64_t bytesInvalidated;
-    bool moveToTrash = true;
-    bool shouldDelete;
-    GCState(const GCOptions & options, GCResults & results)
-        : options(options), results(results), bytesInvalidated(0) { }
-};
 
+    StorePathSet roots, dead, alive;
 
-bool LocalStore::isActiveTempFile(const GCState & state,
+    struct Shared
-    const Path & path, const string & suffix)
+    {
-{
+        // The temp roots only store the hash part to make it easier to
-    return hasSuffix(path, suffix)
+        // ignore suffixes like '.lock', '.chroot' and '.check'.
-        && state.tempRoots.count(parseStorePath(string(path, 0, path.size() - suffix.size())));
+        std::unordered_set<std::string> tempRoots;
-}
 
+        // Hash part of the store path currently being deleted, if
+        // any.
+        std::optional<std::string> pending;
+    };
 
-void LocalStore::deleteGarbage(GCState & state, const Path & path)
+    Sync<Shared> _shared;
-{
-    uint64_t bytesFreed;
-    deletePath(path, bytesFreed);
-    state.results.bytesFreed += bytesFreed;
-}
 
+    std::condition_variable wakeup;
 
-void LocalStore::deletePathRecursive(GCState & state, const Path & path)
+    /* Using `--ignore-liveness' with `--delete' can have unintended
-{
+       consequences if `keep-outputs' or `keep-derivations' are true
-    checkInterrupt();
+       (the garbage collector will recurse into deleting the outputs
-
+       or derivers, respectively).  So disable them. */
-    uint64_t size = 0;
+    if (options.action == GCOptions::gcDeleteSpecific && options.ignoreLiveness) {
-
+        gcKeepOutputs = false;
-    auto storePath = maybeParseStorePath(path);
+        gcKeepDerivations = false;
-    if (storePath && isValidPath(*storePath)) {
-        StorePathSet referrers;
-        queryReferrers(*storePath, referrers);
-        for (auto & i : referrers)
-            if (printStorePath(i) != path) deletePathRecursive(state, printStorePath(i));
-        size = queryPathInfo(*storePath)->narSize;
-        invalidatePathChecked(*storePath);
     }
 
-    Path realPath = realStoreDir + "/" + std::string(baseNameOf(path));
+    if (shouldDelete)
+        deletePath(reservedPath);
 
-    struct stat st;
+    /* Acquire the global GC root. Note: we don't use fdGCLock
-    if (lstat(realPath.c_str(), &st)) {
+       here because then in auto-gc mode, another thread could
-        if (errno == ENOENT) return;
+       downgrade our exclusive lock. */
-        throw SysError("getting status of %1%", realPath);
+    auto fdGCLock = openGCLock();
+    FdLock gcLock(fdGCLock.get(), ltWrite, true, "waiting for the big garbage collector lock...");
+
+    /* Start the server for receiving new roots. */
+    auto socketPath = stateDir.get() + gcSocketPath;
+    createDirs(dirOf(socketPath));
+    auto fdServer = createUnixDomainSocket(socketPath, 0666);
+
+    if (fcntl(fdServer.get(), F_SETFL, fcntl(fdServer.get(), F_GETFL) | O_NONBLOCK) == -1)
+        throw SysError("making socket '%1%' non-blocking", socketPath);
+
+    Pipe shutdownPipe;
+    shutdownPipe.create();
+
+    std::thread serverThread([&]() {
+        Sync<std::map<int, std::thread>> connections;
+
+        Finally cleanup([&]() {
+            debug("GC roots server shutting down");
+            while (true) {
+                auto item = remove_begin(*connections.lock());
+                if (!item) break;
+                auto & [fd, thread] = *item;
+                shutdown(fd, SHUT_RDWR);
+                thread.join();
             }
+        });
 
-    printInfo(format("deleting '%1%'") % path);
+        while (true) {
+            std::vector<struct pollfd> fds;
+            fds.push_back({.fd = shutdownPipe.readSide.get(), .events = POLLIN});
+            fds.push_back({.fd = fdServer.get(), .events = POLLIN});
+            auto count = poll(fds.data(), fds.size(), -1);
+            assert(count != -1);
 
-    state.results.paths.insert(path);
+            if (fds[0].revents)
+                /* Parent is asking us to quit. */
+                break;
 
-    /* If the path is not a regular file or symlink, move it to the
+            if (fds[1].revents) {
-       trash directory.  The move is to ensure that later (when we're
+                /* Accept a new connection. */
-       not holding the global GC lock) we can delete the path without
+                assert(fds[1].revents & POLLIN);
-       being afraid that the path has become alive again.  Otherwise
+                AutoCloseFD fdClient = accept(fdServer.get(), nullptr, nullptr);
-       delete it right away. */
+                if (!fdClient) continue;
-    if (state.moveToTrash && S_ISDIR(st.st_mode)) {
+
-        // Estimate the amount freed using the narSize field.  FIXME:
+                /* Process the connection in a separate thread. */
-        // if the path was not valid, need to determine the actual
+                auto fdClient_ = fdClient.get();
-        // size.
+                std::thread clientThread([&, fdClient = std::move(fdClient)]() {
+                    Finally cleanup([&]() {
+                        auto conn(connections.lock());
+                        auto i = conn->find(fdClient.get());
+                        if (i != conn->end()) {
+                            i->second.detach();
+                            conn->erase(i);
+                        }
+                    });
+
+                    while (true) {
                         try {
-            if (chmod(realPath.c_str(), st.st_mode | S_IWUSR) == -1)
+                            auto path = readLine(fdClient.get());
-                throw SysError("making '%1%' writable", realPath);
+                            auto storePath = maybeParseStorePath(path);
-            Path tmp = trashDir + "/" + std::string(baseNameOf(path));
+                            if (storePath) {
-            if (rename(realPath.c_str(), tmp.c_str()))
+                                debug("got new GC root '%s'", path);
-                throw SysError("unable to rename '%1%' to '%2%'", realPath, tmp);
+                                auto hashPart = std::string(storePath->hashPart());
-            state.bytesInvalidated += size;
+                                auto shared(_shared.lock());
-        } catch (SysError & e) {
+                                shared->tempRoots.insert(hashPart);
-            if (e.errNo == ENOSPC) {
+                                /* If this path is currently being
-                printInfo(format("note: can't create move '%1%': %2%") % realPath % e.msg());
+                                   deleted, then we have to wait until
-                deleteGarbage(state, realPath);
+                                   deletion is finished to ensure that
-            }
+                                   the client doesn't start
+                                   re-creating it before we're
+                                   done. FIXME: ideally we would use a
+                                   FD for this so we don't block the
+                                   poll loop. */
+                                while (shared->pending == hashPart) {
+                                    debug("synchronising with deletion of path '%s'", path);
+                                    shared.wait(wakeup);
                                 }
                             } else
-        deleteGarbage(state, realPath);
+                                printError("received garbage instead of a root from client");
+                            writeFull(fdClient.get(), "1", false);
+                        } catch (Error &) { break; }
+                    }
+                });
 
-    if (state.results.bytesFreed + state.bytesInvalidated > state.options.maxFreed) {
+                connections.lock()->insert({fdClient_, std::move(clientThread)});
-        printInfo(format("deleted or invalidated more than %1% bytes; stopping") % state.options.maxFreed);
+            }
+        }
+    });
+
+    Finally stopServer([&]() {
+        writeFull(shutdownPipe.writeSide.get(), "x", false);
+        wakeup.notify_all();
+        if (serverThread.joinable()) serverThread.join();
+    });
+
+    /* Find the roots.  Since we've grabbed the GC lock, the set of
+       permanent roots cannot increase now. */
+    printInfo("finding garbage collector roots...");
+    Roots rootMap;
+    if (!options.ignoreLiveness)
+        findRootsNoTemp(rootMap, true);
+
+    for (auto & i : rootMap) roots.insert(i.first);
+
+    /* Read the temporary roots created before we acquired the global
+       GC root. Any new roots will be sent to our socket. */
+    Roots tempRoots;
+    findTempRoots(tempRoots, true);
+    for (auto & root : tempRoots) {
+        _shared.lock()->tempRoots.insert(std::string(root.first.hashPart()));
+        roots.insert(root.first);
+    }
+
+    /* Helper function that deletes a path from the store and throws
+       GCLimitReached if we've deleted enough garbage. */
+    auto deleteFromStore = [&](std::string_view baseName)
+    {
+        Path path = storeDir + "/" + std::string(baseName);
+        Path realPath = realStoreDir + "/" + std::string(baseName);
+
+        printInfo("deleting '%1%'", path);
+
+        results.paths.insert(path);
+
+        uint64_t bytesFreed;
+        deletePath(realPath, bytesFreed);
+        results.bytesFreed += bytesFreed;
+
+        if (results.bytesFreed > options.maxFreed) {
+            printInfo("deleted more than %d bytes; stopping", options.maxFreed);
             throw GCLimitReached();
         }
-}
+    };
 
+    std::map<StorePath, StorePathSet> referrersCache;
 
-bool LocalStore::canReachRoot(GCState & state, StorePathSet & visited, const StorePath & path)
+    /* Helper function that visits all paths reachable from `start`
-{
+       via the referrers edges and optionally derivers and derivation
-    if (visited.count(path)) return false;
+       output edges. If none of those paths are roots, then all
+       visited paths are garbage and are deleted. */
+    auto deleteReferrersClosure = [&](const StorePath & start) {
+        StorePathSet visited;
+        std::queue<StorePath> todo;
 
-    if (state.alive.count(path)) return true;
+        /* Wake up any GC client waiting for deletion of the paths in
+           'visited' to finish. */
+        Finally releasePending([&]() {
+            auto shared(_shared.lock());
+            shared->pending.reset();
+            wakeup.notify_all();
+        });
 
-    if (state.dead.count(path)) return false;
+        auto enqueue = [&](const StorePath & path) {
+            if (visited.insert(path).second)
+                todo.push(path);
+        };
 
-    if (state.roots.count(path)) {
+        enqueue(start);
-        debug("cannot delete '%1%' because it's a root", printStorePath(path));
-        state.alive.insert(path);
-        return true;
-    }
 
-    visited.insert(path);
+        while (auto path = pop(todo)) {
-
-    if (!isValidPath(path)) return false;
-
-    StorePathSet incoming;
-
-    /* Don't delete this path if any of its referrers are alive. */
-    queryReferrers(path, incoming);
-
-    /* If keep-derivations is set and this is a derivation, then
-       don't delete the derivation if any of the outputs are alive. */
-    if (state.gcKeepDerivations && path.isDerivation()) {
-        for (auto & [name, maybeOutPath] : queryPartialDerivationOutputMap(path))
-            if (maybeOutPath &&
-                isValidPath(*maybeOutPath) &&
-                queryPathInfo(*maybeOutPath)->deriver == path
-                )
-                incoming.insert(*maybeOutPath);
-    }
-
-    /* If keep-outputs is set, then don't delete this path if there
-       are derivers of this path that are not garbage. */
-    if (state.gcKeepOutputs) {
-        auto derivers = queryValidDerivers(path);
-        for (auto & i : derivers)
-            incoming.insert(i);
-    }
-
-    for (auto & i : incoming)
-        if (i != path)
-            if (canReachRoot(state, visited, i)) {
-                state.alive.insert(path);
-                return true;
-            }
-
-    return false;
-}
-
-
-void LocalStore::tryToDelete(GCState & state, const Path & path)
-{
             checkInterrupt();
 
-    auto realPath = realStoreDir + "/" + std::string(baseNameOf(path));
+            /* Bail out if we've previously discovered that this path
-    if (realPath == linksDir || realPath == trashDir) return;
+               is alive. */
-
+            if (alive.count(*path)) {
-    //Activity act(*logger, lvlDebug, format("considering whether to delete '%1%'") % path);
+                alive.insert(start);
-
+                return;
-    auto storePath = maybeParseStorePath(path);
-
-    if (!storePath || !isValidPath(*storePath)) {
-        /* A lock file belonging to a path that we're building right
-           now isn't garbage. */
-        if (isActiveTempFile(state, path, ".lock")) return;
-
-        /* Don't delete .chroot directories for derivations that are
-           currently being built. */
-        if (isActiveTempFile(state, path, ".chroot")) return;
-
-        /* Don't delete .check directories for derivations that are
-           currently being built, because we may need to run
-           diff-hook. */
-        if (isActiveTempFile(state, path, ".check")) return;
             }
 
-    StorePathSet visited;
+            /* If we've previously deleted this path, we don't have to
+               handle it again. */
+            if (dead.count(*path)) continue;
 
-    if (storePath && canReachRoot(state, visited, *storePath)) {
+            auto markAlive = [&]()
-        debug("cannot delete '%s' because it's still reachable", path);
+            {
-    } else {
+                alive.insert(*path);
-        /* No path we visited was a root, so everything is garbage.
+                alive.insert(start);
-           But we only delete ‘path’ and its referrers here so that
+                try {
-           ‘nix-store --delete’ doesn't have the unexpected effect of
+                    StorePathSet closure;
-           recursing into derivations and outputs. */
+                    computeFSClosure(*path, closure);
-        for (auto & i : visited)
+                    for (auto & p : closure)
-            state.dead.insert(i);
+                        alive.insert(p);
-        if (state.shouldDelete)
+                } catch (InvalidPath &) { }
-            deletePathRecursive(state, path);
+            };
+
+            /* If this is a root, bail out. */
+            if (roots.count(*path)) {
+                debug("cannot delete '%s' because it's a root", printStorePath(*path));
+                return markAlive();
             }
-}
 
+            if (options.action == GCOptions::gcDeleteSpecific
+                && !options.pathsToDelete.count(*path))
+                return;
 
-/* Unlink all files in /nix/store/.links that have a link count of 1,
+            {
+                auto hashPart = std::string(path->hashPart());
+                auto shared(_shared.lock());
+                if (shared->tempRoots.count(hashPart)) {
+                    debug("cannot delete '%s' because it's a temporary root", printStorePath(*path));
+                    return markAlive();
+                }
+                shared->pending = hashPart;
+            }
+
+            if (isValidPath(*path)) {
+
+                /* Visit the referrers of this path. */
+                auto i = referrersCache.find(*path);
+                if (i == referrersCache.end()) {
+                    StorePathSet referrers;
+                    queryReferrers(*path, referrers);
+                    referrersCache.emplace(*path, std::move(referrers));
+                    i = referrersCache.find(*path);
+                }
+                for (auto & p : i->second)
+                    enqueue(p);
+
+                /* If keep-derivations is set and this is a
+                   derivation, then visit the derivation outputs. */
+                if (gcKeepDerivations && path->isDerivation()) {
+                    for (auto & [name, maybeOutPath] : queryPartialDerivationOutputMap(*path))
+                        if (maybeOutPath &&
+                            isValidPath(*maybeOutPath) &&
+                            queryPathInfo(*maybeOutPath)->deriver == *path)
+                            enqueue(*maybeOutPath);
+                }
+
+                /* If keep-outputs is set, then visit the derivers. */
+                if (gcKeepOutputs) {
+                    auto derivers = queryValidDerivers(*path);
+                    for (auto & i : derivers)
+                        enqueue(i);
+                }
+            }
+        }
+
+        for (auto & path : topoSortPaths(visited)) {
+            if (!dead.insert(path).second) continue;
+            if (shouldDelete) {
+                invalidatePathChecked(path);
+                deleteFromStore(path.to_string());
+                referrersCache.erase(path);
+            }
+        }
+    };
+
+    /* Synchronisation point for testing, see tests/gc-concurrent.sh. */
+    if (auto p = getEnv("_NIX_TEST_GC_SYNC"))
+        readFile(*p);
+
+    /* Either delete all garbage paths, or just the specified
+       paths (for gcDeleteSpecific). */
+    if (options.action == GCOptions::gcDeleteSpecific) {
+
+        for (auto & i : options.pathsToDelete) {
+            deleteReferrersClosure(i);
+            if (!dead.count(i))
+                throw Error(
+                    "Cannot delete path '%1%' since it is still alive. "
+                    "To find out why, use: "
+                    "nix-store --query --roots",
+                    printStorePath(i));
+        }
+
+    } else if (options.maxFreed > 0) {
+
+        if (shouldDelete)
+            printInfo("deleting garbage...");
+        else
+            printInfo("determining live/dead paths...");
+
+        try {
+            AutoCloseDir dir(opendir(realStoreDir.get().c_str()));
+            if (!dir) throw SysError("opening directory '%1%'", realStoreDir);
+
+            /* Read the store and delete all paths that are invalid or
+               unreachable. We don't use readDirectory() here so that
+               GCing can start faster. */
+            auto linksName = baseNameOf(linksDir);
+            Paths entries;
+            struct dirent * dirent;
+            while (errno = 0, dirent = readdir(dir.get())) {
+                checkInterrupt();
+                string name = dirent->d_name;
+                if (name == "." || name == ".." || name == linksName) continue;
+
+                if (auto storePath = maybeParseStorePath(storeDir + "/" + name))
+                    deleteReferrersClosure(*storePath);
+                else
+                    deleteFromStore(name);
+
+            }
+        } catch (GCLimitReached & e) {
+        }
+    }
+
+    if (options.action == GCOptions::gcReturnLive) {
+        for (auto & i : alive)
+            results.paths.insert(printStorePath(i));
+        return;
+    }
+
+    if (options.action == GCOptions::gcReturnDead) {
+        for (auto & i : dead)
+            results.paths.insert(printStorePath(i));
+        return;
+    }
+
+    /* Unlink all files in /nix/store/.links that have a link count of 1,
        which indicates that there are no other links and so they can be
        safely deleted.  FIXME: race condition with optimisePath(): we
        might see a link count of 1 just before optimisePath() increases
        the link count. */
-void LocalStore::removeUnusedLinks(const GCState & state)
+    if (options.action == GCOptions::gcDeleteDead || options.action == GCOptions::gcDeleteSpecific) {
-{
+        printInfo("deleting unused links...");
+
         AutoCloseDir dir(opendir(linksDir.c_str()));
         if (!dir) throw SysError("opening directory '%1%'", linksDir);
 
@@ -677,7 +819,7 @@ void LocalStore::removeUnusedLinks(const GCState & state)
             if (unlink(path.c_str()) == -1)
                 throw SysError("deleting '%1%'", path);
 
-        state.results.bytesFreed += st.st_size;
+            results.bytesFreed += st.st_size;
         }
 
         struct stat st;
@@ -687,159 +829,6 @@ void LocalStore::removeUnusedLinks(const GCState & state)
 
         printInfo("note: currently hard linking saves %.2f MiB",
             ((unsharedSize - actualSize - overhead) / (1024.0 * 1024.0)));
-}
-
-
-void LocalStore::collectGarbage(const GCOptions & options, GCResults & results)
-{
-    GCState state(options, results);
-    state.gcKeepOutputs = settings.gcKeepOutputs;
-    state.gcKeepDerivations = settings.gcKeepDerivations;
-
-    /* Using `--ignore-liveness' with `--delete' can have unintended
-       consequences if `keep-outputs' or `keep-derivations' are true
-       (the garbage collector will recurse into deleting the outputs
-       or derivers, respectively).  So disable them. */
-    if (options.action == GCOptions::gcDeleteSpecific && options.ignoreLiveness) {
-        state.gcKeepOutputs = false;
-        state.gcKeepDerivations = false;
-    }
-
-    state.shouldDelete = options.action == GCOptions::gcDeleteDead || options.action == GCOptions::gcDeleteSpecific;
-
-    if (state.shouldDelete)
-        deletePath(reservedPath);
-
-    /* Acquire the global GC root.  This prevents
-       a) New roots from being added.
-       b) Processes from creating new temporary root files. */
-    AutoCloseFD fdGCLock = openGCLock(ltWrite);
-
-    /* Find the roots.  Since we've grabbed the GC lock, the set of
-       permanent roots cannot increase now. */
-    printInfo("finding garbage collector roots...");
-    Roots rootMap;
-    if (!options.ignoreLiveness)
-        findRootsNoTemp(rootMap, true);
-
-    for (auto & i : rootMap) state.roots.insert(i.first);
-
-    /* Read the temporary roots.  This acquires read locks on all
-       per-process temporary root files.  So after this point no paths
-       can be added to the set of temporary roots. */
-    FDs fds;
-    Roots tempRoots;
-    findTempRoots(fds, tempRoots, true);
-    for (auto & root : tempRoots) {
-        state.tempRoots.insert(root.first);
-        state.roots.insert(root.first);
-    }
-
-    /* After this point the set of roots or temporary roots cannot
-       increase, since we hold locks on everything.  So everything
-       that is not reachable from `roots' is garbage. */
-
-    if (state.shouldDelete) {
-        if (pathExists(trashDir)) deleteGarbage(state, trashDir);
-        try {
-            createDirs(trashDir);
-        } catch (SysError & e) {
-            if (e.errNo == ENOSPC) {
-                printInfo("note: can't create trash directory: %s", e.msg());
-                state.moveToTrash = false;
-            }
-        }
-    }
-
-    /* Now either delete all garbage paths, or just the specified
-       paths (for gcDeleteSpecific). */
-
-    if (options.action == GCOptions::gcDeleteSpecific) {
-
-        for (auto & i : options.pathsToDelete) {
-            tryToDelete(state, printStorePath(i));
-            if (state.dead.find(i) == state.dead.end())
-                throw Error(
-                    "cannot delete path '%1%' since it is still alive. "
-                    "To find out why use: "
-                    "nix-store --query --roots",
-                    printStorePath(i));
-        }
-
-    } else if (options.maxFreed > 0) {
-
-        if (state.shouldDelete)
-            printInfo("deleting garbage...");
-        else
-            printInfo("determining live/dead paths...");
-
-        try {
-
-            AutoCloseDir dir(opendir(realStoreDir.get().c_str()));
-            if (!dir) throw SysError("opening directory '%1%'", realStoreDir);
-
-            /* Read the store and immediately delete all paths that
-               aren't valid.  When using --max-freed etc., deleting
-               invalid paths is preferred over deleting unreachable
-               paths, since unreachable paths could become reachable
-               again.  We don't use readDirectory() here so that GCing
-               can start faster. */
-            Paths entries;
-            struct dirent * dirent;
-            while (errno = 0, dirent = readdir(dir.get())) {
-                checkInterrupt();
-                string name = dirent->d_name;
-                if (name == "." || name == "..") continue;
-                Path path = storeDir + "/" + name;
-                auto storePath = maybeParseStorePath(path);
-                if (storePath && isValidPath(*storePath))
-                    entries.push_back(path);
-                else
-                    tryToDelete(state, path);
-            }
-
-            dir.reset();
-
-            /* Now delete the unreachable valid paths.  Randomise the
-               order in which we delete entries to make the collector
-               less biased towards deleting paths that come
-               alphabetically first (e.g. /nix/store/000...).  This
-               matters when using --max-freed etc. */
-            vector<Path> entries_(entries.begin(), entries.end());
-            std::mt19937 gen(1);
-            std::shuffle(entries_.begin(), entries_.end(), gen);
-
-            for (auto & i : entries_)
-                tryToDelete(state, i);
-
-        } catch (GCLimitReached & e) {
-        }
-    }
-
-    if (state.options.action == GCOptions::gcReturnLive) {
-        for (auto & i : state.alive)
-            state.results.paths.insert(printStorePath(i));
-        return;
-    }
-
-    if (state.options.action == GCOptions::gcReturnDead) {
-        for (auto & i : state.dead)
-            state.results.paths.insert(printStorePath(i));
-        return;
-    }
-
-    /* Allow other processes to add to the store from here on. */
-    fdGCLock = -1;
-    fds.clear();
-
-    /* Delete the trash directory. */
-    printInfo(format("deleting '%1%'") % trashDir);
-    deleteGarbage(state, trashDir);
-
-    /* Clean up the links directory. */
-    if (options.action == GCOptions::gcDeleteDead || options.action == GCOptions::gcDeleteSpecific) {
-        printInfo("deleting unused links...");
-        removeUnusedLinks(state);
     }
 
     /* While we're at it, vacuum the database. */

src/libstore/local-store.cc

@@ -145,7 +145,6 @@ LocalStore::LocalStore(const Params & params)
     , linksDir(realStoreDir + "/.links")
     , reservedPath(dbDir + "/reserved")
     , schemaPath(dbDir + "/schema")
-    , trashDir(realStoreDir + "/trash")
     , tempRootsDir(stateDir + "/temproots")
     , fnTempRoots(fmt("%s/%d", tempRootsDir, getpid()))
     , locksHeld(tokenizeString<PathSet>(getEnv("NIX_HELD_LOCKS").value_or("")))
@@ -386,6 +385,16 @@ LocalStore::LocalStore(const Params & params)
 }
 
 
+AutoCloseFD LocalStore::openGCLock()
+{
+    Path fnGCLock = stateDir + "/gc.lock";
+    auto fdGCLock = open(fnGCLock.c_str(), O_RDWR | O_CREAT | O_CLOEXEC, 0600);
+    if (!fdGCLock)
+        throw SysError("opening global GC lock '%1%'", fnGCLock);
+    return fdGCLock;
+}
+
+
 LocalStore::~LocalStore()
 {
     std::shared_future<void> future;
@@ -825,7 +834,7 @@ uint64_t LocalStore::addValidPath(State & state,
 
     {
         auto state_(Store::state.lock());
-        state_->pathInfoCache.upsert(std::string(info.path.hashPart()),
+        state_->pathInfoCache.upsert(std::string(info.path.to_string()),
             PathInfoCacheValue{ .value = std::make_shared<const ValidPathInfo>(info) });
     }
 
@@ -1198,7 +1207,7 @@ void LocalStore::invalidatePath(State & state, const StorePath & path)
 
     {
         auto state_(Store::state.lock());
-        state_->pathInfoCache.erase(std::string(path.hashPart()));
+        state_->pathInfoCache.erase(std::string(path.to_string()));
     }
 }
 
@@ -1505,7 +1514,8 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
 
     /* Acquire the global GC lock to get a consistent snapshot of
        existing and valid paths. */
-    AutoCloseFD fdGCLock = openGCLock(ltWrite);
+    auto fdGCLock = openGCLock();
+    FdLock gcLock(fdGCLock.get(), ltRead, true, "waiting for the big garbage collector lock...");
 
     StringSet store;
     for (auto & i : readDirectory(realStoreDir)) store.insert(i.name);
@@ -1516,8 +1526,6 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
     StorePathSet validPaths;
     PathSet done;
 
-    fdGCLock = -1;
-
     for (auto & i : queryAllValidPaths())
         verifyPath(printStorePath(i), store, done, validPaths, repair, errors);
 

src/libstore/local-store.hh

@@ -58,9 +58,15 @@ private:
         struct Stmts;
         std::unique_ptr<Stmts> stmts;
 
+        /* The global GC lock  */
+        AutoCloseFD fdGCLock;
+
         /* The file to which we write our temporary roots. */
         AutoCloseFD fdTempRoots;
 
+        /* Connection to the garbage collector. */
+        AutoCloseFD fdRootsSocket;
+
         /* The last time we checked whether to do an auto-GC, or an
            auto-GC finished. */
         std::chrono::time_point<std::chrono::steady_clock> lastGCCheck;
@@ -87,7 +93,6 @@ public:
     const Path linksDir;
     const Path reservedPath;
     const Path schemaPath;
-    const Path trashDir;
     const Path tempRootsDir;
     const Path fnTempRoots;
 
@@ -149,14 +154,11 @@ public:
 
     void addIndirectRoot(const Path & path) override;
 
-    void syncWithGC() override;
-
 private:
 
-    typedef std::shared_ptr<AutoCloseFD> FDPtr;
+    void findTempRoots(Roots & roots, bool censor);
-    typedef list<FDPtr> FDs;
 
-    void findTempRoots(FDs & fds, Roots & roots, bool censor);
+    AutoCloseFD openGCLock();
 
 public:
 
@@ -236,29 +238,12 @@ private:
     PathSet queryValidPathsOld();
     ValidPathInfo queryPathInfoOld(const Path & path);
 
-    struct GCState;
-
-    void deleteGarbage(GCState & state, const Path & path);
-
-    void tryToDelete(GCState & state, const Path & path);
-
-    bool canReachRoot(GCState & state, StorePathSet & visited, const StorePath & path);
-
-    void deletePathRecursive(GCState & state, const Path & path);
-
-    bool isActiveTempFile(const GCState & state,
-        const Path & path, const string & suffix);
-
-    AutoCloseFD openGCLock(LockType lockType);
-
     void findRoots(const Path & path, unsigned char type, Roots & roots);
 
     void findRootsNoTemp(Roots & roots, bool censor);
 
     void findRuntimeRoots(Roots & roots, bool censor);
 
-    void removeUnusedLinks(const GCState & state);
-
     Path createTempDirInStore();
 
     void checkDerivationOutputs(const StorePath & drvPath, const Derivation & drv);

src/libstore/misc.cc

@@ -239,12 +239,11 @@ StorePaths Store::topoSortPaths(const StorePathSet & paths)
 {
     return topoSort(paths,
         {[&](const StorePath & path) {
-            StorePathSet references;
             try {
-                references = queryPathInfo(path)->references;
+                return queryPathInfo(path)->references;
             } catch (InvalidPath &) {
+                return StorePathSet();
             }
-            return references;
         }},
         {[&](const StorePath & path, const StorePath & parent) {
             return BuildError(

src/libstore/pathlocks.cc

@@ -176,4 +176,17 @@ void PathLocks::setDeletion(bool deletePaths)
 }
 
 
+FdLock::FdLock(int fd, LockType lockType, bool wait, std::string_view waitMsg)
+    : fd(fd)
+{
+    if (wait) {
+        if (!lockFile(fd, lockType, false)) {
+            printInfo("%s", waitMsg);
+            acquired = lockFile(fd, lockType, true);
+        }
+    } else
+        acquired = lockFile(fd, lockType, false);
+}
+
+
 }

src/libstore/pathlocks.hh

@@ -35,4 +35,18 @@ public:
     void setDeletion(bool deletePaths);
 };
 
+struct FdLock
+{
+    int fd;
+    bool acquired = false;
+
+    FdLock(int fd, LockType lockType, bool wait, std::string_view waitMsg);
+
+    ~FdLock()
+    {
+        if (acquired)
+            lockFile(fd, ltNone, false);
+    }
+};
+
 }

src/libstore/remote-store.cc

@@ -797,15 +797,6 @@ void RemoteStore::addIndirectRoot(const Path & path)
 }
 
 
-void RemoteStore::syncWithGC()
-{
-    auto conn(getConnection());
-    conn->to << wopSyncWithGC;
-    conn.processStderr();
-    readInt(conn->from);
-}
-
-
 Roots RemoteStore::findRoots(bool censor)
 {
     auto conn(getConnection());

src/libstore/remote-store.hh

@@ -101,8 +101,6 @@ public:
 
     void addIndirectRoot(const Path & path) override;
 
-    void syncWithGC() override;
-
     Roots findRoots(bool censor) override;
 
     void collectGarbage(const GCOptions & options, GCResults & results) override;

src/libstore/store-api.cc

@@ -414,11 +414,9 @@ StorePathSet Store::queryDerivationOutputs(const StorePath & path)
 
 bool Store::isValidPath(const StorePath & storePath)
 {
-    std::string hashPart(storePath.hashPart());
-
     {
         auto state_(state.lock());
-        auto res = state_->pathInfoCache.get(hashPart);
+        auto res = state_->pathInfoCache.get(std::string(storePath.to_string()));
         if (res && res->isKnownNow()) {
             stats.narInfoReadAverted++;
             return res->didExist();
@@ -426,11 +424,11 @@ bool Store::isValidPath(const StorePath & storePath)
     }
 
     if (diskCache) {
-        auto res = diskCache->lookupNarInfo(getUri(), hashPart);
+        auto res = diskCache->lookupNarInfo(getUri(), std::string(storePath.hashPart()));
         if (res.first != NarInfoDiskCache::oUnknown) {
             stats.narInfoReadAverted++;
             auto state_(state.lock());
-            state_->pathInfoCache.upsert(hashPart,
+            state_->pathInfoCache.upsert(std::string(storePath.to_string()),
                 res.first == NarInfoDiskCache::oInvalid ? PathInfoCacheValue{} : PathInfoCacheValue { .value = res.second });
             return res.first == NarInfoDiskCache::oValid;
         }
@@ -440,7 +438,7 @@ bool Store::isValidPath(const StorePath & storePath)
 
     if (diskCache && !valid)
         // FIXME: handle valid = true case.
-        diskCache->upsertNarInfo(getUri(), hashPart, 0);
+        diskCache->upsertNarInfo(getUri(), std::string(storePath.hashPart()), 0);
 
     return valid;
 }
@@ -487,13 +485,11 @@ static bool goodStorePath(const StorePath & expected, const StorePath & actual)
 void Store::queryPathInfo(const StorePath & storePath,
     Callback<ref<const ValidPathInfo>> callback) noexcept
 {
-    std::string hashPart;
+    auto hashPart = std::string(storePath.hashPart());
 
     try {
-        hashPart = storePath.hashPart();
-
         {
-            auto res = state.lock()->pathInfoCache.get(hashPart);
+            auto res = state.lock()->pathInfoCache.get(std::string(storePath.to_string()));
             if (res && res->isKnownNow()) {
                 stats.narInfoReadAverted++;
                 if (!res->didExist())
@@ -508,7 +504,7 @@ void Store::queryPathInfo(const StorePath & storePath,
                 stats.narInfoReadAverted++;
                 {
                     auto state_(state.lock());
-                    state_->pathInfoCache.upsert(hashPart,
+                    state_->pathInfoCache.upsert(std::string(storePath.to_string()),
                         res.first == NarInfoDiskCache::oInvalid ? PathInfoCacheValue{} : PathInfoCacheValue{ .value = res.second });
                     if (res.first == NarInfoDiskCache::oInvalid ||
                         !goodStorePath(storePath, res.second->path))
@@ -523,7 +519,7 @@ void Store::queryPathInfo(const StorePath & storePath,
     auto callbackPtr = std::make_shared<decltype(callback)>(std::move(callback));
 
     queryPathInfoUncached(storePath,
-        {[this, storePathS{printStorePath(storePath)}, hashPart, callbackPtr](std::future<std::shared_ptr<const ValidPathInfo>> fut) {
+        {[this, storePath, hashPart, callbackPtr](std::future<std::shared_ptr<const ValidPathInfo>> fut) {
 
             try {
                 auto info = fut.get();
@@ -533,14 +529,12 @@ void Store::queryPathInfo(const StorePath & storePath,
 
                 {
                     auto state_(state.lock());
-                    state_->pathInfoCache.upsert(hashPart, PathInfoCacheValue { .value = info });
+                    state_->pathInfoCache.upsert(std::string(storePath.to_string()), PathInfoCacheValue { .value = info });
                 }
 
-                auto storePath = parseStorePath(storePathS);
-
                 if (!info || !goodStorePath(storePath, info->path)) {
                     stats.narInfoMissing++;
-                    throw InvalidPath("path '%s' is not valid", storePathS);
+                    throw InvalidPath("path '%s' is not valid", printStorePath(storePath));
                 }
 
                 (*callbackPtr)(ref<const ValidPathInfo>(info));

src/libstore/store-api.hh

@@ -232,7 +232,6 @@ protected:
 
     struct State
     {
-        // FIXME: fix key
         LRUCache<std::string, PathInfoCacheValue> pathInfoCache;
     };
 
@@ -561,26 +560,6 @@ public:
     virtual void addIndirectRoot(const Path & path)
     { unsupported("addIndirectRoot"); }
 
-    /* Acquire the global GC lock, then immediately release it.  This
-       function must be called after registering a new permanent root,
-       but before exiting.  Otherwise, it is possible that a running
-       garbage collector doesn't see the new root and deletes the
-       stuff we've just built.  By acquiring the lock briefly, we
-       ensure that either:
-
-       - The collector is already running, and so we block until the
-         collector is finished.  The collector will know about our
-         *temporary* locks, which should include whatever it is we
-         want to register as a permanent lock.
-
-       - The collector isn't running, or it's just started but hasn't
-         acquired the GC lock yet.  In that case we get and release
-         the lock right away, then exit.  The collector scans the
-         permanent root and sees ours.
-
-       In either case the permanent root is seen by the collector. */
-    virtual void syncWithGC() { };
-
     /* Find the roots of the garbage collector.  Each root is a pair
        (link, storepath) where `link' is the path of the symlink
        outside of the Nix store that point to `storePath'. If

src/libstore/uds-remote-store.cc

@@ -56,14 +56,7 @@ ref<RemoteStore::Connection> UDSRemoteStore::openConnection()
     auto conn = make_ref<Connection>();
 
     /* Connect to a daemon that does the privileged work for us. */
-    conn->fd = socket(PF_UNIX, SOCK_STREAM
+    conn->fd = createUnixDomainSocket();
-        #ifdef SOCK_CLOEXEC
-        | SOCK_CLOEXEC
-        #endif
-        , 0);
-    if (!conn->fd)
-        throw SysError("cannot create Unix domain socket");
-    closeOnExec(conn->fd.get());
 
     nix::connect(conn->fd.get(), path ? *path : settings.nixDaemonSocketFile);
 

src/libutil/util.cc

@@ -1670,7 +1670,7 @@ std::unique_ptr<InterruptCallback> createInterruptCallback(std::function<void()>
 }
 
 
-AutoCloseFD createUnixDomainSocket(const Path & path, mode_t mode)
+AutoCloseFD createUnixDomainSocket()
 {
     AutoCloseFD fdSocket = socket(PF_UNIX, SOCK_STREAM
         #ifdef SOCK_CLOEXEC
@@ -1679,8 +1679,14 @@ AutoCloseFD createUnixDomainSocket(const Path & path, mode_t mode)
         , 0);
     if (!fdSocket)
         throw SysError("cannot create Unix domain socket");
-
     closeOnExec(fdSocket.get());
+    return fdSocket;
+}
+
+
+AutoCloseFD createUnixDomainSocket(const Path & path, mode_t mode)
+{
+    auto fdSocket = nix::createUnixDomainSocket();
 
     bind(fdSocket.get(), path);
 
@@ -1709,7 +1715,7 @@ void bind(int fd, const std::string & path)
             std::string base(baseNameOf(path));
             if (base.size() + 1 >= sizeof(addr.sun_path))
                 throw Error("socket path '%s' is too long", base);
-            strcpy(addr.sun_path, base.c_str());
+            memcpy(addr.sun_path, base.c_str(), base.size() + 1);
             if (bind(fd, (struct sockaddr *) &addr, sizeof(addr)) == -1)
                 throw SysError("cannot bind to socket '%s'", path);
             _exit(0);
@@ -1718,7 +1724,7 @@ void bind(int fd, const std::string & path)
         if (status != 0)
             throw Error("cannot bind to socket '%s'", path);
     } else {
-        strcpy(addr.sun_path, path.c_str());
+        memcpy(addr.sun_path, path.c_str(), path.size() + 1);
         if (bind(fd, (struct sockaddr *) &addr, sizeof(addr)) == -1)
             throw SysError("cannot bind to socket '%s'", path);
     }
@@ -1738,7 +1744,7 @@ void connect(int fd, const std::string & path)
             std::string base(baseNameOf(path));
             if (base.size() + 1 >= sizeof(addr.sun_path))
                 throw Error("socket path '%s' is too long", base);
-            strcpy(addr.sun_path, base.c_str());
+            memcpy(addr.sun_path, base.c_str(), base.size() + 1);
             if (connect(fd, (struct sockaddr *) &addr, sizeof(addr)) == -1)
                 throw SysError("cannot connect to socket at '%s'", path);
             _exit(0);
@@ -1747,7 +1753,7 @@ void connect(int fd, const std::string & path)
         if (status != 0)
             throw Error("cannot connect to socket at '%s'", path);
     } else {
-        strcpy(addr.sun_path, path.c_str());
+        memcpy(addr.sun_path, path.c_str(), path.size() + 1);
         if (connect(fd, (struct sockaddr *) &addr, sizeof(addr)) == -1)
             throw SysError("cannot connect to socket at '%s'", path);
     }

src/libutil/util.hh

@@ -511,6 +511,29 @@ std::optional<typename T::mapped_type> get(const T & map, const typename T::key_
 }
 
 
+/* Remove and return the first item from a container. */
+template <class T>
+std::optional<typename T::value_type> remove_begin(T & c)
+{
+    auto i = c.begin();
+    if (i == c.end()) return {};
+    auto v = std::move(*i);
+    c.erase(i);
+    return v;
+}
+
+
+/* Remove and return the first item from a container. */
+template <class T>
+std::optional<typename T::value_type> pop(T & c)
+{
+    if (c.empty()) return {};
+    auto v = std::move(c.front());
+    c.pop();
+    return v;
+}
+
+
 template<typename T>
 class Callback;
 
@@ -571,6 +594,9 @@ extern PathFilter defaultPathFilter;
 /* Common initialisation performed in child processes. */
 void commonChildInit(Pipe & logPipe);
 
+/* Create a Unix domain socket. */
+AutoCloseFD createUnixDomainSocket();
+
 /* Create a Unix domain socket in listen mode. */
 AutoCloseFD createUnixDomainSocket(const Path & path, mode_t mode);
 

tests/ca/build-with-garbage-path.sh

@@ -8,7 +8,7 @@ requireDaemonNewerThan "2.4pre20210621"
 
 # Get the output path of `rootCA`, and put some garbage instead
 outPath="$(nix-build ./content-addressed.nix -A rootCA --no-out-link)"
-nix-store --delete "$outPath"
+nix-store --delete $(nix-store -q --referrers-closure "$outPath")
 touch "$outPath"
 
 # The build should correctly remove the garbage and put the expected path instead

tests/gc-non-blocking.sh

@@ -0,0 +1,33 @@
+# Test whether the collector is non-blocking, i.e. a build can run in
+# parallel with it.
+source common.sh
+
+needLocalStore "the GC test needs a synchronisation point"
+
+clearStore
+
+fifo=$TEST_ROOT/test.fifo
+mkfifo "$fifo"
+
+dummy=$(nix store add-path ./simple.nix)
+
+running=$TEST_ROOT/running
+touch $running
+
+(_NIX_TEST_GC_SYNC=$fifo nix-store --gc -vvvvv; rm $running) &
+pid=$!
+
+sleep 2
+
+outPath=$(nix-build -o "$TEST_ROOT/result" -E "
+  with import ./config.nix;
+  mkDerivation {
+    name = \"non-blocking\";
+    buildCommand = \"set -x; test -e $running; mkdir \$out; echo > $fifo\";
+  }")
+
+wait $pid
+
+(! test -e $running)
+(! test -e $dummy)
+test -e $outPath

tests/gc.sh

@@ -1,5 +1,7 @@
 source common.sh
 
+clearStore
+
 drvPath=$(nix-instantiate dependencies.nix)
 outPath=$(nix-store -rvv "$drvPath")
 
@@ -23,6 +25,12 @@ test -e $inUse
 if nix-store --delete $outPath; then false; fi
 test -e $outPath
 
+for i in $NIX_STORE_DIR/*; do
+    if [[ $i =~ /trash ]]; then continue; fi # compat with old daemon
+    touch $i.lock
+    touch $i.chroot
+done
+
 nix-collect-garbage
 
 # Check that the root and its dependencies haven't been deleted.
@@ -38,3 +46,7 @@ nix-collect-garbage
 
 # Check that the output has been GC'd.
 if test -e $outPath/foobar; then false; fi
+
+# Check that the store is empty.
+rmdir $NIX_STORE_DIR/.links
+rmdir $NIX_STORE_DIR

tests/local.mk

@@ -4,6 +4,7 @@ nix_tests = \
   gc.sh \
   ca/gc.sh \
   gc-concurrent.sh \
+  gc-non-blocking.sh \
   gc-auto.sh \
   referrers.sh user-envs.sh logging.sh nix-build.sh misc.sh fixed.sh \
   gc-runtime.sh check-refs.sh filter-source.sh \

tests/multiple-outputs.sh

@@ -76,7 +76,10 @@ if nix-build multiple-outputs.nix -A cyclic --no-out-link; then
     exit 1
 fi
 
+# Do a GC. This should leave an empty store.
 echo "collecting garbage..."
 rm $TEST_ROOT/result*
 nix-store --gc --keep-derivations --keep-outputs
 nix-store --gc --print-roots
+rm -rf $NIX_STORE_DIR/.links
+rmdir $NIX_STORE_DIR

tests/repair.sh

@@ -30,7 +30,7 @@ nix-store --verify-path $path2
 chmod u+w $path2
 touch $path2/bad
 
-nix-store --delete $(nix-store -qd $path2)
+nix-store --delete $(nix-store -q --referrers-closure $(nix-store -qd $path2))
 
 (! nix-store --verify --check-contents --repair)