Merge f124293024 into f9afc1e68c

src/libstore/indirect-root-store.cc

@@ -1,4 +1,5 @@
 #include "nix/store/indirect-root-store.hh"
+#include "nix/util/file-system.hh"
 
 namespace nix {
 
@@ -7,12 +8,32 @@ void IndirectRootStore::makeSymlink(const Path & link, const Path & target)
     /* Create directories up to `gcRoot'. */
     createDirs(dirOf(link));
 
-    /* Create the new symlink. */
+    /* Retry loop for temporary symlink creation to handle race conditions */
-    Path tempLink = fmt("%1%.tmp-%2%-%3%", link, getpid(), rand());
+    while (true) {
-    createSymlink(target, tempLink);
+        Path tempLink = makeTempPath(dirOf(link), baseNameOf(link) + ".tmp");
 
-    /* Atomically replace the old one. */
+        createSymlink(target, tempLink);
-    std::filesystem::rename(tempLink, link);
+
+        /* Atomically replace the old one. */
+        try {
+            std::filesystem::rename(tempLink, link);
+            break; /* Success! */
+        } catch (std::filesystem::filesystem_error & e) {
+            try {
+                std::filesystem::remove(tempLink);
+            } catch (...) {
+                /* Ignore errors removing the temp link */
+            }
+
+            if (e.code() == std::errc::file_exists) {
+                /* Race condition: another process created the same temp link.
+                   Try again with a different name. */
+                continue;
+            }
+
+            throw SysError("failed to create symlink '%1%' -> '%2%'", link, target);
+        }
+    }
 }
 
 Path IndirectRootStore::addPermRoot(const StorePath & storePath, const Path & _gcRoot)

src/libstore/local-binary-cache-store.cc

@@ -58,13 +58,32 @@ protected:
         const std::string & mimeType) override
     {
         auto path2 = config->binaryCacheDir + "/" + path;
-        static std::atomic<int> counter{0};
+
-        Path tmp = fmt("%s.tmp.%d.%d", path2, getpid(), ++counter);
+        /* Retry loop for handling race conditions */
-        AutoDelete del(tmp, false);
+        while (true) {
-        StreamToSourceAdapter source(istream);
+            Path tmp = makeTempPath(dirOf(path2), baseNameOf(path2) + ".tmp");
-        writeFile(tmp, source);
+            AutoDelete del(tmp, false);
-        std::filesystem::rename(tmp, path2);
+
-        del.cancel();
+            StreamToSourceAdapter source(istream);
+            try {
+                writeFile(tmp, source);
+            } catch (Error & e) {
+                e.addTrace({}, "while writing to temporary file '%s' for '%s'", tmp, path2);
+            }
+
+            try {
+                std::filesystem::rename(tmp, path2);
+                del.cancel();
+                break; /* Success! */
+            } catch (std::filesystem::filesystem_error & e) {
+                if (e.code() == std::errc::file_exists) {
+                    /* Race condition: another process created the same file.
+                       Try again with a different name. */
+                    continue;
+                }
+                throw SysError("renaming '%s' to '%s'", tmp, path2);
+            }
+        }
     }
 
     void getFile(const std::string & path, Sink & sink) override

src/libstore/local-store.cc

@@ -24,6 +24,7 @@
 #include <algorithm>
 #include <cstring>
 
+#include <atomic>
 #include <memory>
 #include <new>
 #include <sys/types.h>
@@ -1666,11 +1667,35 @@ void LocalStore::addBuildLog(const StorePath & drvPath, std::string_view log)
 
     createDirs(dirOf(logPath));
 
-    auto tmpFile = fmt("%s.tmp.%d", logPath, getpid());
+    /* Retry loop for temporary log file creation to handle race conditions */
+    while (true) {
+        auto tmpFile = makeTempPath(dirOf(logPath), baseNameOf(logPath) + ".tmp");
 
-    writeFile(tmpFile, compress("bzip2", log));
+        try {
+            writeFile(tmpFile, compress("bzip2", log));
+        } catch (Error & e) {
+            e.addTrace({}, "writing build log to '%s'", tmpFile);
+            throw;
+        }
 
-    std::filesystem::rename(tmpFile, logPath);
+        try {
+            std::filesystem::rename(tmpFile, logPath);
+            break; /* Success! */
+        } catch (std::filesystem::filesystem_error & e) {
+            try {
+                std::filesystem::remove(tmpFile);
+            } catch (...) {
+                /* Ignore errors removing the temp file */
+            }
+
+            if (e.code() == std::errc::file_exists) {
+                /* Another process created the log file. That's fine, we're done. */
+                break;
+            }
+
+            throw SysError("renaming temporary file '%1%' to '%2%'", tmpFile, logPath);
+        }
+    }
 }
 
 std::optional<std::string> LocalStore::getVersion()

src/libstore/optimise-store.cc

@@ -223,38 +223,53 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
        its timestamp back to 0. */
     MakeReadOnly makeReadOnly(mustToggle ? dirOfPath : "");
 
-    std::filesystem::path tempLink = fmt("%1%/.tmp-link-%2%-%3%", config->realStoreDir, getpid(), rand());
+    /* Retry loop for temporary link creation to handle race conditions */
+    while (true) {
+        Path tempLink = makeTempPath(config->realStoreDir.get(), ".tmp-link");
 
-    try {
+        try {
-        std::filesystem::create_hard_link(linkPath, tempLink);
+            std::filesystem::create_hard_link(linkPath, tempLink);
-        inodeHash.insert(st.st_ino);
+            inodeHash.insert(st.st_ino);
-    } catch (std::filesystem::filesystem_error & e) {
+        } catch (std::filesystem::filesystem_error & e) {
-        if (e.code() == std::errc::too_many_links) {
+            if (e.code() == std::errc::too_many_links) {
-            /* Too many links to the same file (>= 32000 on most file
+                /* Too many links to the same file (>= 32000 on most file
-               systems).  This is likely to happen with empty files.
+                   systems).  This is likely to happen with empty files.
-               Just shrug and ignore. */
+                   Just shrug and ignore. */
-            if (st.st_size)
+                if (st.st_size)
-                printInfo("'%1%' has maximum number of links", linkPath);
+                    printInfo("'%1%' has maximum number of links", linkPath);
-            return;
+                return;
+            }
+            throw SysError("creating temporary link '%1%'", tempLink);
         }
-        throw;
-    }
 
-    /* Atomically replace the old file with the new hard link. */
+        /* Atomically replace the old file with the new hard link. */
-    try {
+        try {
-        std::filesystem::rename(tempLink, path);
+            std::filesystem::rename(tempLink, path);
-    } catch (std::filesystem::filesystem_error & e) {
+            break; /* Success! */
-        std::filesystem::remove(tempLink);
+        } catch (std::filesystem::filesystem_error & e) {
-            printError("unable to unlink '%1%'", tempLink);
+            try {
-        if (e.code() == std::errc::too_many_links) {
+                std::filesystem::remove(tempLink);
-            /* Some filesystems generate too many links on the rename,
+            } catch (...) {
-               rather than on the original link.  (Probably it
+                /* Ignore errors removing the temp link */
-               temporarily increases the st_nlink field before
+            }
-               decreasing it again.) */
+
-            debug("'%s' has reached maximum number of links", linkPath);
+            if (e.code() == std::errc::too_many_links) {
-            return;
+                /* Some filesystems generate too many links on the rename,
+                   rather than on the original link.  (Probably it
+                   temporarily increases the st_nlink field before
+                   decreasing it again.) */
+                debug("%s has reached maximum number of links", linkPath);
+                return;
+            }
+
+            if (e.code() == std::errc::file_exists) {
+                /* Race condition: another process created the same temp link.
+                   Try again with a different name. */
+                continue;
+            }
+
+            throw SysError("renaming temporary link '%1%' to '%2%'", tempLink, path);
         }
-        throw;
     }
 
     stats.filesLinked++;