diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index c89f7a8a1e..cff5037845 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -12,17 +12,30 @@ #include -/* Acquire the global GC lock. */ -static AutoCloseFD openGCLock(LockType lockType) +static string gcLockName = "gc.lock"; + + +/* Acquire the global GC lock. This is used to prevent new Nix + processes from starting after the temporary root files have been + read. To be precise: when they try to create a new temporary root + file, they will block until the garbage collector has finished / + yielded the GC lock. */ +static int openGCLock(LockType lockType) { -#if 0 - Path fnGCLock = (format("%1%/%2%/%3%") - % nixStateDir % tempRootsDir % getpid()).str(); + Path fnGCLock = (format("%1%/%2%") + % nixStateDir % gcLockName).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); -#endif + AutoCloseFD fdGCLock = open(fnGCLock.c_str(), O_RDWR | O_CREAT, 0600); + if (fdGCLock == -1) + throw SysError(format("opening global GC lock `%1%'") % fnGCLock); + + lockFile(fdGCLock, lockType, true); + + /* !!! Restrict read permission on the GC root. Otherwise any + process that can open the file for reading can DoS the + collector. */ + + return fdGCLock.borrow(); } @@ -41,11 +54,15 @@ void addTempRoot(const Path & path) while (1) { fnTempRoots = (format("%1%/%2%/%3%") % nixStateDir % tempRootsDir % getpid()).str(); - + + AutoCloseFD fdGCLock = openGCLock(ltRead); + fdTempRoots = open(fnTempRoots.c_str(), O_RDWR | O_CREAT | O_TRUNC, 0600); if (fdTempRoots == -1) throw SysError(format("opening temporary roots file `%1%'") % fnTempRoots); + fdGCLock.close(); + debug(format("acquiring read lock on `%1%'") % fnTempRoots); lockFile(fdTempRoots, ltRead, true); @@ -188,14 +205,11 @@ void collectGarbage(const PathSet & roots, GCAction action, { result.clear(); - /* !!! TODO: Acquire the global GC root. This prevents + /* Acquire the global GC root. This prevents a) New roots from being added. b) Processes from creating new temporary root files. */ + AutoCloseFD fdGCLock = openGCLock(ltWrite); - /* !!! 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. */ PathSet livePaths; diff --git a/tests/Makefile.am b/tests/Makefile.am index e1c1585007..c88379e47f 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -36,10 +36,10 @@ nix-pull.sh: dependencies.nix gc.sh: dependencies.nix gc-concurrent.sh: gc-concurrent.nix gc-concurrent2.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 gc-concurrent.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 fd329b457e..c85a03e1ca 100644 --- a/tests/gc-concurrent.sh +++ b/tests/gc-concurrent.sh @@ -6,6 +6,8 @@ outPath2=$($TOP/src/nix-store/nix-store -q $storeExpr2) ls -l test-tmp/state/temproots +ln -s $storeExpr2 "$NIX_LOCALSTATE_DIR"/nix/gcroots/foo2 + # Start build #1 in the background. It starts immediately. $TOP/src/nix-store/nix-store -rvv "$storeExpr1" & pid1=$! @@ -31,4 +33,5 @@ wait $pid2 cat $outPath1/foobar cat $outPath1/input-2/bar +# Build #2 should have failed because its derivation got garbage collected. cat $outPath2/foobar