From 32fa82a56a2c4a937c91f72be16339b4ddf7cdf1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 31 Jan 2005 22:23:49 +0000 Subject: [PATCH] * Acquire a global GC lock to prevent new temporary root files from being created after the garbage collector has read the temproots directory. This blocks the creation of new processes, but the garbage collector could periodically release the GC lock to allow them to run. --- src/libstore/gc.cc | 44 ++++++++++++++++++++++++++++-------------- tests/Makefile.am | 8 ++++---- tests/gc-concurrent.sh | 3 +++ 3 files changed, 36 insertions(+), 19 deletions(-) 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