Keep created temp dirs inside store, but protect from GC

Implements the approach suggested by feedback on PR #6994, where
tempdir paths are created in the store (now with an exclusive lock).

As part of this work, the currently-broken and unused
`createTempDirInStore` function is updated to create an exclusive lock
on the temp directory in the store.

The GC now makes a non-blocking attempt to lock any store directories
that "look like" the temp directories created by this function, and if
it can't acquire one, ignores the directory.
This commit is contained in:
Andrew Brooks 2022-09-06 17:48:00 -05:00
parent 1f041ac54f
commit 84fe75a12a
3 changed files with 32 additions and 11 deletions

View file

@ -619,6 +619,18 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results)
Path path = storeDir + "/" + std::string(baseName); Path path = storeDir + "/" + std::string(baseName);
Path realPath = realStoreDir + "/" + std::string(baseName); Path realPath = realStoreDir + "/" + std::string(baseName);
/* There may be temp directories in the store that are still in use
by another process. We need to be sure that we can acquire an
exclusive lock before deleting them. */
AutoCloseFD tmpDirFd;
if (baseName.rfind("add-", 0) == 0) {
tmpDirFd = open(realPath.c_str(), O_RDONLY | O_DIRECTORY);
if (tmpDirFd.get() == -1 || !lockFile(tmpDirFd.get(), ltWrite, false)) {
debug("skipping locked tempdir '%s'", realPath);
return;
}
}
printInfo("deleting '%1%'", path); printInfo("deleting '%1%'", path);
results.paths.insert(path); results.paths.insert(path);

View file

@ -1382,13 +1382,15 @@ StorePath LocalStore::addToStoreFromDump(Source & source0, std::string_view name
std::unique_ptr<AutoDelete> delTempDir; std::unique_ptr<AutoDelete> delTempDir;
Path tempPath; Path tempPath;
Path tempDir;
AutoCloseFD tempDirFd;
if (!inMemory) { if (!inMemory) {
/* Drain what we pulled so far, and then keep on pulling */ /* Drain what we pulled so far, and then keep on pulling */
StringSource dumpSource { dump }; StringSource dumpSource { dump };
ChainSource bothSource { dumpSource, source }; ChainSource bothSource { dumpSource, source };
auto tempDir = createTempDir("", "add"); std::tie(tempDir, tempDirFd) = createTempDirInStore();
delTempDir = std::make_unique<AutoDelete>(tempDir); delTempDir = std::make_unique<AutoDelete>(tempDir);
tempPath = tempDir + "/x"; tempPath = tempDir + "/x";
@ -1431,6 +1433,7 @@ StorePath LocalStore::addToStoreFromDump(Source & source0, std::string_view name
} else { } else {
/* Move the temporary path we restored above. */ /* Move the temporary path we restored above. */
moveFile(tempPath, realPath); moveFile(tempPath, realPath);
tempDirFd.close();
} }
/* For computing the nar hash. In recursive SHA-256 mode, this /* For computing the nar hash. In recursive SHA-256 mode, this
@ -1507,18 +1510,24 @@ StorePath LocalStore::addTextToStore(
/* Create a temporary directory in the store that won't be /* Create a temporary directory in the store that won't be
garbage-collected. */ garbage-collected until the returned FD is closed. */
Path LocalStore::createTempDirInStore() std::pair<Path, AutoCloseFD> LocalStore::createTempDirInStore()
{ {
Path tmpDir; Path tmpDirFn;
AutoCloseFD tmpDirFd;
bool lockedByUs = false;
do { do {
/* There is a slight possibility that `tmpDir' gets deleted by /* There is a slight possibility that `tmpDir' gets deleted by
the GC between createTempDir() and addTempRoot(), so repeat the GC between createTempDir() and when we acquire a lock on it.
until `tmpDir' exists. */ We'll repeat until 'tmpDir' exists and we've locked it. */
tmpDir = createTempDir(realStoreDir); tmpDirFn = createTempDir(realStoreDir, "add");
addTempRoot(parseStorePath(tmpDir)); tmpDirFd = open(tmpDirFn.c_str(), O_RDONLY | O_DIRECTORY);
} while (!pathExists(tmpDir)); if (tmpDirFd.get() < 0) {
return tmpDir; continue;
}
lockedByUs = lockFile(tmpDirFd.get(), ltWrite, true);
} while (!pathExists(tmpDirFn) || !lockedByUs);
return {tmpDirFn, std::move(tmpDirFd)};
} }

View file

@ -256,7 +256,7 @@ private:
void findRuntimeRoots(Roots & roots, bool censor); void findRuntimeRoots(Roots & roots, bool censor);
Path createTempDirInStore(); std::pair<Path, AutoCloseFD> createTempDirInStore();
void checkDerivationOutputs(const StorePath & drvPath, const Derivation & drv); void checkDerivationOutputs(const StorePath & drvPath, const Derivation & drv);