From 1328aa33077fd1cf84869e366c82b8ea1d1abb5d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 31 Jan 2005 10:27:25 +0000 Subject: [PATCH] * Start of concurrent garbage collection. Processes write temporary roots to a per-process temporary file in /nix/var/nix/temproots while holding a write lock on that file. The garbage collector acquires read locks on all those files, thus blocking further progress in other Nix processes, and reads the sets of temporary roots. --- Makefile.am | 1 + scripts/nix-collect-garbage.in | 6 +- src/libstore/build.cc | 8 +- src/libstore/gc.cc | 247 ++++++++++++++++++++------------- src/libstore/gc.hh | 6 + src/libstore/store.cc | 5 + src/libutil/util.cc | 13 +- src/libutil/util.hh | 3 +- tests/Makefile.am | 7 +- tests/gc-concurrent.sh | 4 +- tests/init.sh | 1 + 11 files changed, 193 insertions(+), 108 deletions(-) diff --git a/Makefile.am b/Makefile.am index 58440ba85..7e386f84c 100644 --- a/Makefile.am +++ b/Makefile.am @@ -24,6 +24,7 @@ init-state: $(INSTALL) $(INIT_FLAGS) -d $(DESTDIR)$(localstatedir)/log/nix $(INSTALL) $(INIT_FLAGS) -d $(DESTDIR)$(localstatedir)/nix/profiles $(INSTALL) $(INIT_FLAGS) -d $(DESTDIR)$(localstatedir)/nix/gcroots + $(INSTALL) $(INIT_FLAGS) -d $(DESTDIR)$(localstatedir)/nix/temproots $(INSTALL) $(INIT_FLAGS) $(GROUP_WRITABLE) -d $(DESTDIR)$(localstatedir)/nix/gcroots/tmp $(INSTALL) $(INIT_FLAGS) $(GROUP_WRITABLE) -d $(DESTDIR)$(localstatedir)/nix/gcroots/channels rm -f $(DESTDIR)$(localstatedir)/nix/gcroots/profiles diff --git a/scripts/nix-collect-garbage.in b/scripts/nix-collect-garbage.in index f11ed2cb6..c92737f25 100644 --- a/scripts/nix-collect-garbage.in +++ b/scripts/nix-collect-garbage.in @@ -9,6 +9,7 @@ my $storeDir = "@storedir@"; my %alive; my $gcOper = "--delete"; +my $extraArgs = ""; my @roots = (); @@ -19,6 +20,9 @@ for (my $i = 0; $i < scalar @ARGV; $i++) { if ($arg eq "--delete" || $arg eq "--print-live" || $arg eq "--print-dead") { $gcOper = $arg; } + elsif ($arg =~ /^-v+$/) { + $extraArgs = "$extraArgs $arg"; + } else { die "unknown argument `$arg'" }; } @@ -66,7 +70,7 @@ findRoots 1, $rootsDir; # Run the collector with the roots we found. -my $pid = open2(">&1", \*WRITE, "@bindir@/nix-store --gc $gcOper") +my $pid = open2(">&1", \*WRITE, "@bindir@/nix-store --gc $gcOper $extraArgs") or die "cannot run `nix-store --gc'"; foreach my $root (@roots) { diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 52bd08bb1..dbfde447e 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -13,6 +13,7 @@ #include "references.hh" #include "pathlocks.hh" #include "globals.hh" +#include "gc.hh" /* !!! TODO derivationFromPath shouldn't be used here */ @@ -59,7 +60,6 @@ protected: /* Whether amDone() has been called. */ bool done; - Goal(Worker & worker) : worker(worker) { done = false; @@ -442,6 +442,10 @@ void DerivationGoal::haveStoreExpr() /* Get the derivation. */ drv = derivationFromPath(drvPath); + for (DerivationOutputs::iterator i = drv.outputs.begin(); + i != drv.outputs.end(); ++i) + addTempRoot(i->second.path); + /* Check what outputs paths are not already valid. */ PathSet invalidOutputs = checkPathValidity(false); @@ -1308,6 +1312,8 @@ void SubstitutionGoal::init() { trace("init"); + addTempRoot(storePath); + /* If the path already exists we're done. */ if (isValidPath(storePath)) { amDone(); diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index ba6e6bb9d..4c6a944b8 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -1,20 +1,146 @@ #include "globals.hh" #include "gc.hh" #include "build.hh" +#include "pathlocks.hh" + +#include #include #include +#include +#include #include +static string tempRootsDir = "temproots"; + +/* The file to which we write our temporary roots. */ +Path fnTempRoots; +static AutoCloseFD fdTempRoots; + + +void addTempRoot(const Path & path) +{ + /* Create the temporary roots file for this process. */ + if (fdTempRoots == -1) { + + while (1) { + fnTempRoots = (format("%1%/%2%/%3%") + % nixStateDir % tempRootsDir % getpid()).str(); + + fdTempRoots = open(fnTempRoots.c_str(), O_RDWR | O_CREAT | O_TRUNC, 0600); + if (fdTempRoots == -1) + throw SysError(format("opening temporary roots file `%1%'") % fnTempRoots); + + debug(format("acquiring read lock on `%1%'") % fnTempRoots); + lockFile(fdTempRoots, ltRead, true); + + /* Check whether the garbage collector didn't get in our + way. */ + struct stat st; + if (fstat(fdTempRoots, &st) == -1) + throw SysError(format("statting `%1%'") % fnTempRoots); + if (st.st_size == 0) break; + + /* The garbage collector deleted this file before we could + get a lock. (It won't delete the file after we get a + lock.) Try again. */ + } + + } + + /* Upgrade the lock to a write lock. This will cause us to block + if the garbage collector is holding our lock. */ + debug(format("acquiring write lock on `%1%'") % fnTempRoots); + lockFile(fdTempRoots, ltWrite, true); + + string s = path + '\0'; + writeFull(fdTempRoots, (const unsigned char *) s.c_str(), s.size()); + + /* Downgrade to a read lock. */ + debug(format("downgrading to read lock on `%1%'") % fnTempRoots); + lockFile(fdTempRoots, ltRead, true); +} + + +typedef shared_ptr FDPtr; +typedef list FDs; + + +static void readTempRoots(PathSet & tempRoots, FDs & fds) +{ + /* Read the `temproots' directory for per-process temporary root + files. */ + Strings tempRootFiles = readDirectory( + (format("%1%/%2%") % nixStateDir % tempRootsDir).str()); + + for (Strings::iterator i = tempRootFiles.begin(); + i != tempRootFiles.end(); ++i) + { + Path path = (format("%1%/%2%/%3%") % nixStateDir % tempRootsDir % *i).str(); + + debug(format("reading temporary root file `%1%'") % path); + + FDPtr fd(new AutoCloseFD(open(path.c_str(), O_RDWR, 0666))); + if (*fd == -1) { + /* It's okay if the file has disappeared. */ + if (errno == ENOENT) continue; + throw SysError(format("opening temporary roots file `%1%'") % path); + } + + /* 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, ltWrite, false)) { + printMsg(lvlError, format("removing stale temporary roots file `%1%'") + % path); + /* !!! write token, unlink */ + 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, ltRead, true); + + /* Read the entire file. */ + struct stat st; + if (fstat(*fd, &st) == -1) + throw SysError(format("statting `%1%'") % path); + unsigned char buf[st.st_size]; /* !!! stack space */ + readFull(*fd, buf, st.st_size); + debug(format("FILE SIZE %1%") % st.st_size); + + /* Extract the roots. */ + string contents((char *) buf, st.st_size); + unsigned int pos = 0, end; + + while ((end = contents.find((char) 0, pos)) != string::npos) { + Path root(contents, pos, end - pos); + debug(format("got temporary root `%1%'") % root); + assertStorePath(root); + tempRoots.insert(root); + pos = end + 1; + } + + fds.push_back(fd); /* keep open */ + } +} + + void collectGarbage(const PathSet & roots, GCAction action, PathSet & result) { result.clear(); - /* !!! TODO: Acquire an exclusive lock on the gcroots directory. - This prevents the set of live paths from increasing after this - point. */ + /* !!! TODO: Acquire the global GC root. This prevents + a) New roots from being added. + b) Processes from creating new temporary root files. */ + + /* !!! Restrict read permission on the GC root. Otherwise any + process that can open the file for reading can DoS the + collector. */ /* Determine the live paths which is just the closure of the roots under the `references' relation. */ @@ -27,6 +153,16 @@ void collectGarbage(const PathSet & roots, GCAction action, return; } + /* 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. */ + PathSet tempRoots; + FDs fds; + readTempRoots(tempRoots, fds); + + for (FDs::iterator i = fds.begin(); i != fds.end(); ++i) + debug(format("FD %1%") % (int) **i); + /* !!! TODO: Try to acquire (without blocking) exclusive locks on the files in the `pending' directory. Delete all files for which we managed to acquire such a lock (since if we could get @@ -50,6 +186,11 @@ void collectGarbage(const PathSet & roots, GCAction action, continue; } + if (tempRoots.find(path) != tempRoots.end()) { + debug(format("temporary root `%1%'") % path); + continue; + } + debug(format("dead path `%1%'") % path); result.insert(path); @@ -57,100 +198,10 @@ void collectGarbage(const PathSet & roots, GCAction action, printMsg(lvlInfo, format("deleting `%1%'") % path); deleteFromStore(path); } - + + /* Only delete lock files if the path is belongs to doesn't + exist and isn't a temporary root and we can acquire an + exclusive lock on it. */ + /* !!! */ } } - - - -#if 0 -void followLivePaths(Path nePath, PathSet & live) -{ - /* Just to be sure, canonicalise the path. It is important to do - this here and in findDeadPath() to ensure that a live path is - not mistaken for a dead path due to some non-canonical - representation. */ - nePath = canonPath(nePath); - - if (live.find(nePath) != live.end()) return; - live.insert(nePath); - - startNest(nest, lvlDebug, format("following `%1%'") % nePath); - assertStorePath(nePath); - - if (isValidPath(nePath)) { - - /* !!! should make sure that no substitutes are used */ - StoreExpr ne = storeExprFromPath(nePath); - - /* !!! painfully similar to requisitesWorker() */ - if (ne.type == StoreExpr::neClosure) - for (ClosureElems::iterator i = ne.closure.elems.begin(); - i != ne.closure.elems.end(); ++i) - { - Path p = canonPath(i->first); - if (live.find(p) == live.end()) { - debug(format("found live `%1%'") % p); - assertStorePath(p); - live.insert(p); - } - } - - else if (ne.type == StoreExpr::neDerivation) - for (PathSet::iterator i = ne.derivation.inputs.begin(); - i != ne.derivation.inputs.end(); ++i) - followLivePaths(*i, live); - - else abort(); - - } - - Path nfPath; - if (querySuccessor(nePath, nfPath)) - followLivePaths(nfPath, live); -} - - -PathSet findLivePaths(const Paths & roots) -{ - PathSet live; - - startNest(nest, lvlDebug, "finding live paths"); - - for (Paths::const_iterator i = roots.begin(); i != roots.end(); ++i) - followLivePaths(*i, live); - - return live; -} - - -PathSet findDeadPaths(const PathSet & live, time_t minAge) -{ - PathSet dead; - - startNest(nest, lvlDebug, "finding dead paths"); - - time_t now = time(0); - - Strings storeNames = readDirectory(nixStore); - - for (Strings::iterator i = storeNames.begin(); i != storeNames.end(); ++i) { - Path p = canonPath(nixStore + "/" + *i); - - if (minAge > 0) { - struct stat st; - if (lstat(p.c_str(), &st) != 0) - throw SysError(format("obtaining information about `%1%'") % p); - if (st.st_atime + minAge >= now) continue; - } - - if (live.find(p) == live.end()) { - debug(format("dead path `%1%'") % p); - dead.insert(p); - } else - debug(format("live path `%1%'") % p); - } - - return dead; -} -#endif diff --git a/src/libstore/gc.hh b/src/libstore/gc.hh index 2ea851abc..838188ade 100644 --- a/src/libstore/gc.hh +++ b/src/libstore/gc.hh @@ -3,6 +3,7 @@ #include "util.hh" + /* Garbage collector operation. */ typedef enum { gcReturnLive, gcReturnDead, gcDeleteDead } GCAction; @@ -14,4 +15,9 @@ typedef enum { gcReturnLive, gcReturnDead, gcDeleteDead } GCAction; void collectGarbage(const PathSet & roots, GCAction action, PathSet & result); +/* Register a temporary GC root. This root will automatically + disappear when this process exits. */ +void addTempRoot(const Path & path); + + #endif /* !__GC_H */ diff --git a/src/libstore/store.cc b/src/libstore/store.cc index e676216c9..7c0faaf6c 100644 --- a/src/libstore/store.cc +++ b/src/libstore/store.cc @@ -12,6 +12,7 @@ #include "db.hh" #include "archive.hh" #include "pathlocks.hh" +#include "gc.hh" /* Nix database. */ @@ -468,6 +469,8 @@ Path addToStore(const Path & _srcPath) string baseName = baseNameOf(srcPath); Path dstPath = makeStorePath("source", h, baseName); + addTempRoot(dstPath); + if (!readOnlyMode && !isValidPath(dstPath)) { /* The first check above is an optimisation to prevent @@ -512,6 +515,8 @@ Path addTextToStore(const string & suffix, const string & s, Path dstPath = makeStorePath("text", hash, suffix); + addTempRoot(dstPath); + if (!readOnlyMode && !isValidPath(dstPath)) { PathSet lockPaths; diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 0af6ee149..611567c12 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -234,8 +234,8 @@ Path createTempDir() void writeStringToFile(const Path & path, const string & s) { - AutoCloseFD fd = open(path.c_str(), - O_CREAT | O_EXCL | O_WRONLY, 0666); + AutoCloseFD fd(open(path.c_str(), + O_CREAT | O_EXCL | O_WRONLY, 0666)); if (fd == -1) throw SysError(format("creating file `%1%'") % path); writeFull(fd, (unsigned char *) s.c_str(), s.size()); @@ -375,6 +375,12 @@ AutoCloseFD::AutoCloseFD(int fd) } +AutoCloseFD::AutoCloseFD(const AutoCloseFD & fd) +{ + abort(); +} + + AutoCloseFD::~AutoCloseFD() { try { @@ -392,7 +398,7 @@ void AutoCloseFD::operator =(int fd) } -AutoCloseFD::operator int() +AutoCloseFD::operator int() const { return fd; } @@ -401,6 +407,7 @@ AutoCloseFD::operator int() void AutoCloseFD::close() { if (fd != -1) { + debug(format("closing fd %1%") % fd); if (::close(fd) == -1) /* This should never happen. */ throw SysError("closing file descriptor"); diff --git a/src/libutil/util.hh b/src/libutil/util.hh index d947c3425..104e3f265 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -179,9 +179,10 @@ class AutoCloseFD public: AutoCloseFD(); AutoCloseFD(int fd); + AutoCloseFD(const AutoCloseFD & fd); ~AutoCloseFD(); void operator =(int fd); - operator int(); + operator int() const; void close(); bool isOpen(); int borrow(); diff --git a/tests/Makefile.am b/tests/Makefile.am index 53ecfe1fb..7c823a046 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -36,9 +36,10 @@ nix-pull.sh: dependencies.nix gc.sh: dependencies.nix gc-concurrent.sh: gc-concurrent.nix -TESTS = init.sh hash.sh lang.sh simple.sh dependencies.sh locking.sh parallel.sh \ - build-hook.sh substitutes.sh substitutes2.sh fallback.sh nix-push.sh gc.sh \ - gc-concurrent.sh verify.sh nix-pull.sh +#TESTS = init.sh hash.sh lang.sh simple.sh dependencies.sh locking.sh parallel.sh \ +# build-hook.sh substitutes.sh substitutes2.sh fallback.sh nix-push.sh gc.sh \ +# gc-concurrent.sh verify.sh nix-pull.sh +TESTS = init.sh gc-concurrent.sh XFAIL_TESTS = diff --git a/tests/gc-concurrent.sh b/tests/gc-concurrent.sh index d85c60982..c41475fb6 100644 --- a/tests/gc-concurrent.sh +++ b/tests/gc-concurrent.sh @@ -1,6 +1,8 @@ storeExpr=$($TOP/src/nix-instantiate/nix-instantiate gc-concurrent.nix) outPath=$($TOP/src/nix-store/nix-store -q $storeExpr) +ls -l test-tmp/state/temproots + # Start a build in the background. $TOP/src/nix-store/nix-store -rvv "$storeExpr" & @@ -8,7 +10,7 @@ pid=$! # Run the garbage collector while the build is running. sleep 2 -$NIX_BIN_DIR/nix-collect-garbage +$NIX_BIN_DIR/nix-collect-garbage -vvvvv # Wait for the build to finish. echo waiting for pid $pid to finish... diff --git a/tests/init.sh b/tests/init.sh index e80a847c3..63cce81bd 100644 --- a/tests/init.sh +++ b/tests/init.sh @@ -26,6 +26,7 @@ ln -s $TOP/scripts/readmanifest.pm $NIX_BIN_DIR/nix/ mkdir -p "$NIX_LOCALSTATE_DIR"/nix/manifests mkdir -p "$NIX_LOCALSTATE_DIR"/nix/gcroots mkdir -p "$NIX_LOCALSTATE_DIR"/log/nix +mkdir -p "$NIX_LOCALSTATE_DIR"/temproots mkdir $NIX_DATA_DIR/nix cp -prd $TOP/corepkgs $NIX_DATA_DIR/nix/