From 5c38c863bdb6904f28a929b97e91d283e29aea70 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Thu, 14 Sep 2006 22:30:33 +0000
Subject: [PATCH] * Fix a huge gaping hole in nix-env w.r.t. the garbage
 collector.   Nix-env failed to call addPermRoot(), which is necessary to
 safely   add a new root.  So if nix-env started after and finished before the
   garbage collector, the user environment (plus all other new stuff)   it
 built might be garbage collected, leading to a dangling symlink   chain in
 ~/.nix-profile...

* Be more explicit if we block on the GC lock ("waiting for the big
  garbage collector lock...").

* Don't loop trying to create a new generation.  It's not necessary
  anymore since profiles are locked nowadays.
---
 src/libstore/gc.cc      | 23 ++++++++++++++---------
 src/libstore/gc.hh      |  2 +-
 src/nix-env/profiles.cc | 23 +++++++++++------------
 3 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index 59e71daa0..ed1b1a84a 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -44,7 +44,10 @@ static int openGCLock(LockType lockType)
     if (fdGCLock == -1)
         throw SysError(format("opening global GC lock `%1%'") % fnGCLock);
 
-    lockFile(fdGCLock, lockType, true);
+    if (!lockFile(fdGCLock, lockType, false)) {
+        printMsg(lvlError, format("waiting for the big garbage collector lock..."));
+        lockFile(fdGCLock, lockType, true);
+    }
 
     /* !!! Restrict read permission on the GC root.  Otherwise any
        process that can open the file for reading can DoS the
@@ -74,7 +77,7 @@ void createSymlink(const Path & link, const Path & target, bool careful)
 
 
 Path addPermRoot(const Path & _storePath, const Path & _gcRoot,
-    bool indirect)
+    bool indirect, bool allowOutsideRootsDir)
 {
     Path storePath(canonPath(_storePath));
     Path gcRoot(canonPath(_gcRoot));
@@ -97,14 +100,16 @@ Path addPermRoot(const Path & _storePath, const Path & _gcRoot,
     }
 
     else {
-        Path rootsDir = canonPath((format("%1%/%2%") % nixStateDir % gcRootsDir).str());
+        if (!allowOutsideRootsDir) {
+            Path rootsDir = canonPath((format("%1%/%2%") % nixStateDir % gcRootsDir).str());
     
-        if (string(gcRoot, 0, rootsDir.size() + 1) != rootsDir + "/")
-            throw Error(format(
-                "path `%1%' is not a valid garbage collector root; "
-                "it's not in the directory `%2%'")
-                % gcRoot % rootsDir);
-
+            if (string(gcRoot, 0, rootsDir.size() + 1) != rootsDir + "/")
+                throw Error(format(
+                    "path `%1%' is not a valid garbage collector root; "
+                    "it's not in the directory `%2%'")
+                    % gcRoot % rootsDir);
+        }
+            
         createSymlink(gcRoot, storePath, false);
     }
 
diff --git a/src/libstore/gc.hh b/src/libstore/gc.hh
index 54fc4dec4..3f7242884 100644
--- a/src/libstore/gc.hh
+++ b/src/libstore/gc.hh
@@ -39,7 +39,7 @@ void removeTempRoots();
 
 /* Register a permanent GC root. */
 Path addPermRoot(const Path & storePath, const Path & gcRoot,
-    bool indirect);
+    bool indirect, bool allowOutsideRootsDir = false);
 
 
 }
diff --git a/src/nix-env/profiles.cc b/src/nix-env/profiles.cc
index 0db5bda5e..b99024d07 100644
--- a/src/nix-env/profiles.cc
+++ b/src/nix-env/profiles.cc
@@ -1,5 +1,6 @@
 #include "profiles.hh"
 #include "util.hh"
+#include "gc.hh"
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -82,19 +83,17 @@ Path createGeneration(Path profile, Path outPath)
     Generations gens = findGenerations(profile, dummy);
     unsigned int num = gens.size() > 0 ? gens.front().number : 0;
         
-    /* Create the new generation. */
-    Path outLink;
+    /* Create the new generation.  Note that addPermRoot() blocks if
+       the garbage collector is running to prevent the stuff we've
+       build from moving from the temporary roots (which the GC knows)
+       to the permanent roots (of which the GC would have a stale
+       view).  If we didn't do it this way, the GC might remove the
+       user environment etc. we've just built. */
+    Path generation;
+    makeName(profile, num + 1, generation);
+    addPermRoot(outPath, generation, false, true);
 
-    while (1) {
-        makeName(profile, num, outLink);
-        if (symlink(outPath.c_str(), outLink.c_str()) == 0) break;
-        if (errno != EEXIST)
-            throw SysError(format("creating symlink `%1%'") % outLink);
-        /* Somebody beat us to it, retry with a higher number. */
-        num++;
-    }
-
-    return outLink;
+    return generation;
 }