diff --git a/src/libstore/build.cc b/src/libstore/build.cc index ab1011981..71560b2d0 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -642,7 +642,7 @@ void DerivationGoal::haveStoreExpr() for (DerivationOutputs::iterator i = drv.outputs.begin(); i != drv.outputs.end(); ++i) - addTempRoot(i->second.path); + store->addTempRoot(i->second.path); /* Check what outputs paths are not already valid. */ PathSet invalidOutputs = checkPathValidity(false); @@ -1714,7 +1714,7 @@ void SubstitutionGoal::init() { trace("init"); - addTempRoot(storePath); + store->addTempRoot(storePath); /* If the path already exists we're done. */ if (store->isValidPath(storePath)) { diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 3e4150d89..56e64369a 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -76,6 +76,12 @@ void createSymlink(const Path & link, const Path & target, bool careful) } +void LocalStore::syncWithGC() +{ + AutoCloseFD fdGCLock = openGCLock(ltRead); +} + + Path addPermRoot(const Path & _storePath, const Path & _gcRoot, bool indirect, bool allowOutsideRootsDir) { @@ -83,10 +89,6 @@ Path addPermRoot(const Path & _storePath, const Path & _gcRoot, Path gcRoot(canonPath(_gcRoot)); assertStorePath(storePath); - /* Grab the global GC root. This prevents the set of permanent - roots from increasing while a GC is in progress. */ - AutoCloseFD fdGCLock = openGCLock(ltRead); - if (indirect) { string hash = printHash32(hashString(htSHA1, gcRoot)); Path realRoot = canonPath((format("%1%/%2%/auto/%3%") @@ -110,6 +112,11 @@ Path addPermRoot(const Path & _storePath, const Path & _gcRoot, createSymlink(gcRoot, storePath, false); } + /* 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. */ + store->syncWithGC(); + return gcRoot; } @@ -119,7 +126,7 @@ static Path fnTempRoots; static AutoCloseFD fdTempRoots; -void addTempRoot(const Path & path) +void LocalStore::addTempRoot(const Path & path) { /* Create the temporary roots file for this process. */ if (fdTempRoots == -1) { diff --git a/src/libstore/gc.hh b/src/libstore/gc.hh index 3f7242884..d4f40afa2 100644 --- a/src/libstore/gc.hh +++ b/src/libstore/gc.hh @@ -26,12 +26,6 @@ typedef enum { void collectGarbage(GCAction action, const PathSet & pathsToDelete, bool ignoreLiveness, PathSet & result, unsigned long long & bytesFreed); -/* Register a temporary GC root. This root will automatically - disappear when this process exits. WARNING: this function should - not be called inside a BDB transaction, otherwise we can - deadlock. */ -void addTempRoot(const Path & path); - /* Remove the temporary roots file for this process. Any temporary root becomes garbage after this point unless it has been registered as a (permanent) root. */ diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 3a7b22048..389be33a3 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -58,6 +58,10 @@ public: void buildDerivations(const PathSet & drvPaths); void ensurePath(const Path & path); + + void addTempRoot(const Path & path); + + void syncWithGC(); }; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 2391fd9e8..9b9d74f7e 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -170,4 +170,19 @@ void RemoteStore::ensurePath(const Path & path) } +void RemoteStore::addTempRoot(const Path & path) +{ + writeInt(wopAddTempRoot, to); + writeString(path, to); + readInt(from); +} + + +void RemoteStore::syncWithGC() +{ + writeInt(wopSyncWithGC, to); + readInt(from); +} + + } diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 62feee0ea..b11191c09 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -47,6 +47,10 @@ public: void ensurePath(const Path & path); + void addTempRoot(const Path & path); + + void syncWithGC(); + private: Pipe toChild; Pipe fromChild; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 6fbe97931..67d230ca7 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -86,6 +86,31 @@ public: may be made valid by running a substitute (if defined for the path). */ virtual void ensurePath(const Path & path) = 0; + + /* Add a store path as a temporary root of the garbage collector. + The root disappears as soon as we exit. */ + virtual void addTempRoot(const Path & path) = 0; + + /* 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 our's. + + In either case the permanent root is seen by the collector. */ + virtual void syncWithGC() = 0; + }; diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index 65f5fc100..2700b6719 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -18,6 +18,8 @@ typedef enum { wopAddTextToStore, wopBuildDerivations, wopEnsurePath, + wopAddTempRoot, + wopSyncWithGC } WorkerOp; diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 5623b0131..6d96310da 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -122,7 +122,7 @@ Path dirOf(const Path & path) { Path::size_type pos = path.rfind('/'); if (pos == string::npos) - throw Error(format("invalid file name: %1%") % path); + throw Error(format("invalid file name `%1%'") % path); return pos == 0 ? "/" : Path(path, 0, pos); } @@ -131,7 +131,7 @@ string baseNameOf(const Path & path) { Path::size_type pos = path.rfind('/'); if (pos == string::npos) - throw Error(format("invalid file name %1% ") % path); + throw Error(format("invalid file name `%1%'") % path); return string(path, pos + 1); } diff --git a/src/nix-env/main.cc b/src/nix-env/main.cc index 4f0b5eca9..f2988485c 100644 --- a/src/nix-env/main.cc +++ b/src/nix-env/main.cc @@ -182,7 +182,7 @@ static void createUserEnv(EvalState & state, const DrvInfos & elems, /* This is only necessary when installing store paths, e.g., `nix-env -i /nix/store/abcd...-foo'. */ - addTempRoot(i->queryOutPath(state)); + store->addTempRoot(i->queryOutPath(state)); store->ensurePath(i->queryOutPath(state)); references.insert(i->queryOutPath(state)); @@ -940,7 +940,7 @@ static void opSwitchProfile(Globals & globals, if (opArgs.size() != 1) throw UsageError(format("exactly one argument expected")); - Path profile = opArgs.front(); + Path profile = absPath(opArgs.front()); Path profileLink = getHomeDir() + "/.nix-profile"; switchLink(profileLink, profile); diff --git a/src/nix-worker/main.cc b/src/nix-worker/main.cc index 95077e653..fef2c2958 100644 --- a/src/nix-worker/main.cc +++ b/src/nix-worker/main.cc @@ -127,6 +127,19 @@ void processConnection(Source & from, Sink & to) break; } + case wopAddTempRoot: { + Path path = readStorePath(from); + store->addTempRoot(path); + writeInt(1, to); + break; + } + + case wopSyncWithGC: { + store->syncWithGC(); + writeInt(1, to); + break; + } + default: throw Error(format("invalid operation %1%") % op); }