From e25fad691aa3ccb492c4fb8840289f76151e553e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sat, 2 Dec 2006 16:41:36 +0000 Subject: [PATCH] * Move addTempRoot() to the store API, and add another function syncWithGC() to allow clients to register GC roots without needing write access to the global roots directory or the GC lock. --- src/libstore/build.cc | 4 ++-- src/libstore/gc.cc | 17 ++++++++++++----- src/libstore/gc.hh | 6 ------ src/libstore/local-store.hh | 4 ++++ src/libstore/remote-store.cc | 15 +++++++++++++++ src/libstore/remote-store.hh | 4 ++++ src/libstore/store-api.hh | 25 +++++++++++++++++++++++++ src/libstore/worker-protocol.hh | 2 ++ src/libutil/util.cc | 4 ++-- src/nix-env/main.cc | 4 ++-- src/nix-worker/main.cc | 13 +++++++++++++ 11 files changed, 81 insertions(+), 17 deletions(-) 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); }