From 2aac7cd0217ce3417b12574ca7f9930090da6c4c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 21 Dec 2011 19:17:45 +0000 Subject: * Another case of lock file permissions being too liberal. --- src/libstore/pathlocks.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore') diff --git a/src/libstore/pathlocks.cc b/src/libstore/pathlocks.cc index d8290815c44c..645f4cd67e7e 100644 --- a/src/libstore/pathlocks.cc +++ b/src/libstore/pathlocks.cc @@ -16,7 +16,7 @@ int openLockFile(const Path & path, bool create) { AutoCloseFD fd; - fd = open(path.c_str(), O_RDWR | (create ? O_CREAT : 0), 0666); + fd = open(path.c_str(), O_RDWR | (create ? O_CREAT : 0), 0600); if (fd == -1 && (create || errno != ENOENT)) throw SysError(format("opening lock file `%1%'") % path); -- cgit 1.4.1 From 58d974336c733416756e5b396928602ea8ed8df2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 22 Dec 2011 14:33:34 +0000 Subject: * Drop unnecessary call to canonPath() (nixStore is already canonical). --- src/libstore/gc.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore') diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index e79d93723b83..a99bb1a81ced 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -634,7 +634,7 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results) try { foreach (vector::iterator, i, entries_) - tryToDelete(state, canonPath(nixStore + "/" + *i)); + tryToDelete(state, nixStore + "/" + *i); } catch (GCLimitReached & e) { } } -- cgit 1.4.1 From b33da599c5c1b06a32a3eeac58f95481d10f821d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 22 Dec 2011 15:55:53 +0000 Subject: * In the garbage collector, delete invalid paths before deleting unreachable paths. This matters when using --max-freed etc.: unreachable paths could become reachable again, so it's nicer to keep them if there is "real" garbage to be deleted. Also, don't use readDirectory() but read the Nix store and delete invalid paths in parallel. This reduces GC latency on very large Nix stores. --- src/libstore/gc.cc | 46 +++++++++++++++++++++++++++++++++++----------- src/libutil/util.cc | 10 +++++++++- src/libutil/util.hh | 1 + 3 files changed, 45 insertions(+), 12 deletions(-) (limited to 'src/libstore') diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index a99bb1a81ced..20f194e6e2a7 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -617,27 +617,51 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results) } else { - printMsg(lvlError, format("reading the Nix store...")); - Paths entries = readDirectory(nixStore); - - /* Randomise the order in which we delete entries to make the - collector less biased towards deleting paths that come - alphabetically first (e.g. /nix/store/000...). This - matters when using --max-freed etc. */ - vector entries_(entries.begin(), entries.end()); - random_shuffle(entries_.begin(), entries_.end()); - if (shouldDelete(state.options.action)) printMsg(lvlError, format("deleting garbage...")); else printMsg(lvlError, format("determining live/dead paths...")); try { + + AutoCloseDir dir = opendir(nixStore.c_str()); + if (!dir) throw SysError(format("opening directory `%1%'") % nixStore); + + /* Read the store and immediately delete all paths that + aren't valid. When using --max-freed etc., deleting + invalid paths is preferred over deleting unreachable + paths, since unreachable paths could become reachable + again. We don't use readDirectory() here so that GCing + can start faster. */ + Paths entries; + struct dirent * dirent; + while (errno = 0, dirent = readdir(dir)) { + checkInterrupt(); + string name = dirent->d_name; + if (name == "." || name == "..") continue; + Path path = nixStore + "/" + name; + if (isValidPath(path)) + entries.push_back(path); + else + tryToDelete(state, path); + } + + dir.close(); + + /* Now delete the unreachable valid paths. Randomise the + order in which we delete entries to make the collector + less biased towards deleting paths that come + alphabetically first (e.g. /nix/store/000...). This + matters when using --max-freed etc. */ + vector entries_(entries.begin(), entries.end()); + random_shuffle(entries_.begin(), entries_.end()); + foreach (vector::iterator, i, entries_) tryToDelete(state, nixStore + "/" + *i); + } catch (GCLimitReached & e) { } - } + } } diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 9adaac40d56e..0352754f592c 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -701,7 +701,7 @@ AutoCloseDir::AutoCloseDir(DIR * dir) AutoCloseDir::~AutoCloseDir() { - if (dir) closedir(dir); + close(); } @@ -717,6 +717,14 @@ AutoCloseDir::operator DIR *() } +void AutoCloseDir::close() +{ + if (dir) { + closedir(dir); + dir = 0; + } +} + ////////////////////////////////////////////////////////////////////// diff --git a/src/libutil/util.hh b/src/libutil/util.hh index f86290f31694..a1cf68e69d1c 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -223,6 +223,7 @@ public: ~AutoCloseDir(); void operator =(DIR * dir); operator DIR *(); + void close(); }; -- cgit 1.4.1 From 524fa8a4f11826fdf22005f3304366856f72ffa5 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 22 Dec 2011 16:27:03 +0000 Subject: * Oops. --- src/libstore/gc.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore') diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 20f194e6e2a7..feaab573ef30 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -657,7 +657,7 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results) random_shuffle(entries_.begin(), entries_.end()); foreach (vector::iterator, i, entries_) - tryToDelete(state, nixStore + "/" + *i); + tryToDelete(state, *i); } catch (GCLimitReached & e) { } -- cgit 1.4.1 From 8c42a8c8ff2986940a41d46b0bdaa1c2ff0f15ad Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sun, 25 Dec 2011 16:38:37 +0000 Subject: * Make sure that lock files are cleaned up properly when building through the build hook. --- src/libstore/build.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'src/libstore') diff --git a/src/libstore/build.cc b/src/libstore/build.cc index a8ef9b23efaf..149cd8b09783 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -1300,6 +1300,13 @@ void DerivationGoal::buildDone() being valid. */ computeClosure(); + /* It is now safe to delete the lock files, since all future + lockers will see that the output paths are valid; they will + not create new lock files with the same names as the old + (unlinked) lock files. */ + outputLocks.setDeletion(true); + outputLocks.unlock(); + } catch (BuildError & e) { printMsg(lvlError, e.msg()); outputLocks.unlock(); @@ -1987,13 +1994,6 @@ void DerivationGoal::computeClosure() infos.push_back(info); } worker.store.registerValidPaths(infos); - - /* It is now safe to delete the lock files, since all future - lockers will see that the output paths are valid; they will not - create new lock files with the same names as the old (unlinked) - lock files. */ - outputLocks.setDeletion(true); - outputLocks.unlock(); } -- cgit 1.4.1