Merge pull request #6994 from agbrooks/master

Prevent tempdir from being GC-ed before addToStoreFromDump completes

src/libstore/gc.cc

@@ -619,6 +619,17 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results)
         Path path = storeDir + "/" + 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. */
+        if (baseName.find("tmp-", 0) == 0) {
+            AutoCloseFD 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);
 
         results.paths.insert(path);

src/libstore/local-store.cc

@@ -1382,13 +1382,15 @@ StorePath LocalStore::addToStoreFromDump(Source & source0, std::string_view name
 
     std::unique_ptr<AutoDelete> delTempDir;
     Path tempPath;
+    Path tempDir;
+    AutoCloseFD tempDirFd;
 
     if (!inMemory) {
         /* Drain what we pulled so far, and then keep on pulling */
         StringSource dumpSource { dump };
         ChainSource bothSource { dumpSource, source };
 
-        auto tempDir = createTempDir(realStoreDir, "add");
+        std::tie(tempDir, tempDirFd) = createTempDirInStore();
         delTempDir = std::make_unique<AutoDelete>(tempDir);
         tempPath = tempDir + "/x";
 
@@ -1507,18 +1509,24 @@ StorePath LocalStore::addTextToStore(
 
 
 /* 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 {
         /* 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, "tmp");
-        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)};
 }
 
 

src/libstore/local-store.hh

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