From d27a73b1a95a911077947f39fab42c628c722c0e Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Tue, 5 Dec 2006 00:34:42 +0000
Subject: [PATCH] * In addPermRoot, check that the root that we just registered
 can be   found by the garbage collector.  This addresses NIX-71 and is a  
 particular concern in multi-user stores.

---
 src/libmain/shared.cc |  2 +-
 src/libstore/gc.cc    | 58 ++++++++++++++++++++++++++++++-------------
 2 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc
index e6713e9abf..c1bd1b73a7 100644
--- a/src/libmain/shared.cc
+++ b/src/libmain/shared.cc
@@ -47,7 +47,7 @@ void printGCWarning()
 {
     static bool haveWarned = false;
     warnOnce(haveWarned, 
-        "warning: you did not specify `--add-root'; "
+        "you did not specify `--add-root'; "
         "the result might be removed by the garbage collector");
 }
 
diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index ba59083cad..8eae01066c 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -91,6 +91,9 @@ void LocalStore::addIndirectRoot(const Path & path)
 }
 
 
+static void findRoots(PathSet & roots, bool ignoreUnreadable);
+
+
 Path addPermRoot(const Path & _storePath, const Path & _gcRoot,
     bool indirect, bool allowOutsideRootsDir)
 {
@@ -115,6 +118,18 @@ Path addPermRoot(const Path & _storePath, const Path & _gcRoot,
         }
             
         createSymlink(gcRoot, storePath, false);
+
+        /* Check that the root can be found by the garbage collector. */
+        PathSet roots;
+        findRoots(roots, true);
+        if (roots.find(storePath) == roots.end()) {
+            static bool haveWarned = false;
+            warnOnce(haveWarned, 
+                format(
+                    "the path `%1%' is not reachable from the roots directory of the garbage collector; "
+                    "therefore, `%2%' might be removed by the garbage collector")
+                % gcRoot % storePath);
+        }
     }
 
     /* Grab the global GC root, causing us to block while a GC is in
@@ -292,7 +307,7 @@ static void readTempRoots(PathSet & tempRoots, FDs & fds)
 
 
 static void findRoots(const Path & path, bool recurseSymlinks,
-    PathSet & roots)
+    bool ignoreUnreadable, PathSet & roots)
 {
     struct stat st;
     if (lstat(path.c_str(), &st) == -1)
@@ -303,39 +318,50 @@ static void findRoots(const Path & path, bool recurseSymlinks,
     if (S_ISDIR(st.st_mode)) {
 	Strings names = readDirectory(path);
 	for (Strings::iterator i = names.begin(); i != names.end(); ++i)
-            findRoots(path + "/" + *i, recurseSymlinks, roots);
+            findRoots(path + "/" + *i, recurseSymlinks, ignoreUnreadable, roots);
     }
 
     else if (S_ISLNK(st.st_mode)) {
-        string target = readLink(path);
-        Path target2 = absPath(target, dirOf(path));
+        Path target = absPath(readLink(path), dirOf(path));
 
-        if (isInStore(target2)) {
+        if (isInStore(target)) {
             debug(format("found root `%1%' in `%2%'")
-                % target2 % path);
-            Path target3 = toStorePath(target2);
-            if (store->isValidPath(target3)) 
-                roots.insert(target3);
+                % target % path);
+            Path storePath = toStorePath(target);
+            if (store->isValidPath(storePath)) 
+                roots.insert(storePath);
             else
                 printMsg(lvlInfo, format("skipping invalid root from `%1%' to `%2%'")
-                    % path % target3);
+                    % path % storePath);
         }
 
         else if (recurseSymlinks) {
-            if (pathExists(target2))
-                findRoots(target2, false, roots);
-            else {
-                printMsg(lvlInfo, format("removing stale link from `%1%' to `%2%'") % path % target2);
+            struct stat st2;
+            if (lstat(target.c_str(), &st2) == 0)
+                findRoots(target, false, ignoreUnreadable, roots);
+            else if (ignoreUnreadable && errno == EACCES)
+                /* ignore */ ;
+            else if (errno == ENOENT || errno == ENOTDIR) {
+                printMsg(lvlInfo, format("removing stale link from `%1%' to `%2%'") % path % target);
                 /* Note that we only delete when recursing, i.e., when
                    we are still in the `gcroots' tree.  We never
                    delete stuff outside that tree. */
                 unlink(path.c_str());
             }
+            else 
+                throw SysError(format("statting `%1%'") % target);
         }
     }
 }
 
 
+static void findRoots(PathSet & roots, bool ignoreUnreadable)
+{
+    Path rootsDir = canonPath((format("%1%/%2%") % nixStateDir % gcRootsDir).str());
+    findRoots(rootsDir, true, ignoreUnreadable, roots);
+}
+
+
 static void addAdditionalRoots(PathSet & roots)
 {
     Path rootFinder = getEnv("NIX_ROOT_FINDER",
@@ -410,10 +436,8 @@ void collectGarbage(GCAction action, const PathSet & pathsToDelete,
 
     /* Find the roots.  Since we've grabbed the GC lock, the set of
        permanent roots cannot increase now. */
-    Path rootsDir = canonPath((format("%1%/%2%") % nixStateDir % gcRootsDir).str());
     PathSet roots;
-    if (!ignoreLiveness)
-        findRoots(rootsDir, true, roots);
+    findRoots(roots, false);
 
     /* Add additional roots returned by the program specified by the
        NIX_ROOT_FINDER environment variable.  This is typically used