NarCache improvements

- Separate implementation, abstract implements

- `NarCache` work in constant memory

  This is using `RestoreSink` to dedup some write-side IO, but I am not
  so sure that is a good idea. We'll continue reworking it.

- Use pathlocks to avoid download storms

- Use `Descriptor`-based logic, not `pathExists` to avoid symlink races
  if anything is deleted.

src/libstore/binary-cache-store.cc

@@ -562,7 +562,7 @@ ref<SourceAccessor> BinaryCacheStore::getFSAccessor(bool requireValidPath)
 
 std::shared_ptr<SourceAccessor> BinaryCacheStore::getFSAccessor(const StorePath & storePath, bool requireValidPath)
 {
-    return getRemoteFSAccessor(requireValidPath)->accessObject(storePath);
+    return static_cast<ref<SourceAccessor>>(getRemoteFSAccessor(requireValidPath)->accessObject(storePath));
 }
 
 void BinaryCacheStore::addSignatures(const StorePath & storePath, const std::set<Signature> & sigs)

src/libstore/include/nix/store/local-nar-cache.hh

@@ -0,0 +1,20 @@
+#pragma once
+///@file
+
+#include "nix/util/nar-cache.hh"
+
+#include <filesystem>
+#include <memory>
+
+namespace nix {
+
+/**
+ * Create a NAR cache with local disk storage.
+ *
+ * Uses file locks to ensure only one process downloads a NAR at a time.
+ *
+ * @param cacheDir Directory to store cached NAR files
+ */
+std::unique_ptr<NarCache> makeLocalNarCache(std::filesystem::path cacheDir);
+
+} // namespace nix

src/libstore/include/nix/store/meson.build

@@ -50,6 +50,7 @@ headers = [ config_pub_h ] + files(
   'length-prefixed-protocol-helper.hh',
   'local-binary-cache-store.hh',
   'local-fs-store.hh',
+  'local-nar-cache.hh',
   'local-overlay-store.hh',
   'local-store.hh',
   'log-store.hh',

src/libstore/include/nix/store/remote-fs-accessor.hh

@@ -19,7 +19,7 @@ class RemoteFSAccessor : public SourceAccessor
      */
     std::map<std::string, Hash, std::less<>> narHashes;
 
-    NarCache narCache;
+    std::unique_ptr<NarCache> narCache;
 
     bool requireValidPath;
 
@@ -32,7 +32,7 @@ public:
     /**
      * @return nullptr if the store does not contain any object at that path.
      */
-    std::shared_ptr<SourceAccessor> accessObject(const StorePath & path);
+    ref<NarAccessor> accessObject(const StorePath & path);
 
     RemoteFSAccessor(
         ref<Store> store, bool requireValidPath = true, std::optional<std::filesystem::path> cacheDir = {});

src/libstore/local-nar-cache.cc

@@ -0,0 +1,134 @@
+#include "nix/util/nar-cache.hh"
+#include "nix/util/file-system.hh"
+#include "nix/util/file-descriptor.hh"
+#include "nix/util/fs-sink.hh"
+#include "nix/store/pathlocks.hh"
+
+#include <nlohmann/json.hpp>
+#include <optional>
+
+namespace nix {
+
+namespace {
+
+/**
+ * NAR cache with local disk storage (private implementation).
+ *
+ * Uses file locks to ensure only one process downloads a NAR at a time.
+ */
+class LocalNarCache : public NarCache
+{
+    RestoreSink cacheSink;
+
+public:
+
+    LocalNarCache(std::filesystem::path cacheDir)
+        : cacheSink(false)
+    {
+        createDirs(cacheDir);
+        cacheSink.dstPath = std::move(cacheDir);
+    }
+
+    ref<NarAccessor> getOrInsert(const Hash & narHash, std::function<void(Sink &)> populate) override;
+};
+
+} // anonymous namespace
+
+ref<NarAccessor> LocalNarCache::getOrInsert(const Hash & narHash, std::function<void(Sink &)> populate)
+{
+    // Check in-memory cache first
+    if (auto * accessor = get(nars, narHash))
+        return *accessor;
+
+    auto cacheAccessor = [&](ref<NarAccessor> accessor) {
+        nars.emplace(narHash, accessor);
+        return accessor;
+    };
+
+    auto makeCacheFile = [&](const std::string & ext) -> CanonPath {
+        return {narHash.to_string(HashFormat::Nix32, false) + "." + ext};
+    };
+
+    auto cacheFile = makeCacheFile("nar");
+    auto listingFile = makeCacheFile("ls");
+    auto lockFile = makeCacheFile("lock");
+
+    auto cacheFilePath = cacheSink.dstPath / cacheFile.rel();
+    auto lockFilePath = cacheSink.dstPath / lockFile.rel();
+    auto listingFilePath = cacheSink.dstPath / listingFile.rel();
+
+    // Helper to try loading from cache files using FD operations to avoid race conditions
+    auto tryLoadFromCache = [&]() -> std::optional<ref<NarAccessor>> {
+        try {
+            // Try to open cache file - will throw if doesn't exist
+            AutoCloseFD cacheFD = openFileReadonly(cacheFilePath);
+
+            // Try lazy accessor with listing file first
+            try {
+                AutoCloseFD listingFD = openFileReadonly(listingFilePath);
+                auto listingContent = readFile(listingFD.get());
+                return cacheAccessor(makeLazyNarAccessor(
+                    nlohmann::json::parse(listingContent).template get<NarListing>(),
+                    seekableGetNarBytes(cacheFilePath)));
+            } catch (SystemError &) {
+                // Listing file missing or invalid, fall back to full NAR
+            }
+
+            // Fall back to reading full NAR
+            auto narContent = readFile(cacheFD.get());
+            return cacheAccessor(makeNarAccessor(std::move(narContent)));
+        } catch (SystemError &) {
+            // Cache file doesn't exist or can't be opened
+            return std::nullopt;
+        }
+    };
+
+    // Check if already cached (before acquiring lock)
+    if (auto accessor = tryLoadFromCache())
+        return *accessor;
+
+    // Acquire lock to ensure only one process downloads this NAR
+    AutoCloseFD lockFD = openLockFile(lockFilePath, true);
+    FdLock lock(lockFD.get(), ltWrite, true, "waiting for NAR cache lock");
+
+    // Check again after acquiring lock (another process might have just finished)
+    if (auto accessor = tryLoadFromCache())
+        return *accessor;
+
+    // Download and cache the NAR
+    NarListing listing;
+    try {
+        /* FIXME: do this asynchronously. */
+        cacheSink.createRegularFile(cacheFile, [&](CreateRegularFileSink & fileSink) {
+            auto source = sinkToSource([&](Sink & parseSink) {
+                TeeSink teeSink{fileSink, parseSink};
+                populate(teeSink);
+            });
+            listing = parseNarListing(*source);
+        });
+    } catch (...) {
+        ignoreExceptionExceptInterrupt();
+        StringSink narSink;
+        populate(narSink);
+        return cacheAccessor(makeNarAccessor(std::move(narSink.s)));
+    }
+
+    try {
+        cacheSink.createRegularFile(listingFile, [&](CreateRegularFileSink & sink) {
+            auto s = nlohmann::json(listing).dump();
+            StringSource source{s};
+            source.drainInto(sink);
+        });
+    } catch (...) {
+        ignoreExceptionExceptInterrupt();
+    }
+
+    return cacheAccessor(makeLazyNarAccessor(std::move(listing), seekableGetNarBytes(cacheFilePath)));
+}
+
+std::unique_ptr<NarCache> makeLocalNarCache(std::filesystem::path cacheDir)
+{
+    return std::make_unique<LocalNarCache>(std::move(cacheDir));
+}
+
+} // namespace nix

src/libstore/meson.build

@@ -315,6 +315,7 @@ sources = files(
   'legacy-ssh-store.cc',
   'local-binary-cache-store.cc',
   'local-fs-store.cc',
+  'local-nar-cache.cc',
   'local-overlay-store.cc',
   'local-store.cc',
   'log-store.cc',

src/libstore/remote-fs-accessor.cc

@@ -1,11 +1,12 @@
 #include "nix/store/remote-fs-accessor.hh"
+#include "nix/store/local-nar-cache.hh"
 
 namespace nix {
 
 RemoteFSAccessor::RemoteFSAccessor(
     ref<Store> store, bool requireValidPath, std::optional<std::filesystem::path> cacheDir)
     : store(store)
-    , narCache(cacheDir)
+    , narCache(cacheDir ? makeLocalNarCache(*cacheDir) : makeMemoryNarCache())
     , requireValidPath(requireValidPath)
 {
 }
@@ -18,11 +19,11 @@ std::pair<ref<SourceAccessor>, CanonPath> RemoteFSAccessor::fetch(const CanonPat
     return {ref{accessObject(storePath)}, CanonPath{restPath}};
 }
 
-std::shared_ptr<SourceAccessor> RemoteFSAccessor::accessObject(const StorePath & storePath)
+ref<NarAccessor> RemoteFSAccessor::accessObject(const StorePath & storePath)
 {
     // Check if we already have the NAR hash for this store path
     if (auto * narHash = get(narHashes, storePath.hashPart()))
-        return narCache.getOrInsert(*narHash, [&](Sink & sink) { store->narFromPath(storePath, sink); });
+        return narCache->getOrInsert(*narHash, [&](Sink & sink) { store->narFromPath(storePath, sink); });
 
     // Query the path info to get the NAR hash
     auto info = store->queryPathInfo(storePath);
@@ -31,7 +32,7 @@ std::shared_ptr<SourceAccessor> RemoteFSAccessor::accessObject(const StorePath &
     narHashes.emplace(storePath.hashPart(), info->narHash);
 
     // Get or create the NAR accessor
-    return narCache.getOrInsert(info->narHash, [&](Sink & sink) { store->narFromPath(storePath, sink); });
+    return narCache->getOrInsert(info->narHash, [&](Sink & sink) { store->narFromPath(storePath, sink); });
 }
 
 std::optional<SourceAccessor::Stat> RemoteFSAccessor::maybeLstat(const CanonPath & path)

src/libstore/remote-store.cc

@@ -841,7 +841,7 @@ ref<SourceAccessor> RemoteStore::getFSAccessor(bool requireValidPath)
 
 std::shared_ptr<SourceAccessor> RemoteStore::getFSAccessor(const StorePath & path, bool requireValidPath)
 {
-    return getRemoteFSAccessor(requireValidPath)->accessObject(path);
+    return static_cast<ref<SourceAccessor>>(getRemoteFSAccessor(requireValidPath)->accessObject(path));
 }
 
 void RemoteStore::ConnectionHandle::withFramedSink(std::function<void(Sink & sink)> fun)

src/libutil/include/nix/util/nar-cache.hh

@@ -3,45 +3,42 @@
 #include "nix/util/hash.hh"
 #include "nix/util/nar-accessor.hh"
 #include "nix/util/ref.hh"
-#include "nix/util/source-accessor.hh"
 
 #include <filesystem>
 #include <functional>
 #include <map>
-#include <optional>
+#include <memory>
 
 namespace nix {
 
 /**
- * A cache for NAR accessors with optional disk caching.
+ * Abstract cache for NAR accessors.
  */
 class NarCache
 {
-    /**
-     * Optional directory for caching NARs and listings on disk.
-     */
-    std::optional<std::filesystem::path> cacheDir;
-
+protected:
     /**
      * Map from NAR hash to NAR accessor.
      */
-    std::map<Hash, ref<SourceAccessor>> nars;
+    std::map<Hash, ref<NarAccessor>> nars;
 
 public:
 
-    /**
-     * Create a NAR cache with an optional cache directory for disk storage.
-     */
-    NarCache(std::optional<std::filesystem::path> cacheDir = {});
+    virtual ~NarCache() = default;
 
     /**
-     * Lookup or create a NAR accessor, optionally using disk cache.
+     * Lookup or create a NAR accessor.
      *
      * @param narHash The NAR hash to use as cache key
      * @param populate Function called with a Sink to populate the NAR if not cached
      * @return The cached or newly created accessor
      */
-    ref<SourceAccessor> getOrInsert(const Hash & narHash, std::function<void(Sink &)> populate);
+    virtual ref<NarAccessor> getOrInsert(const Hash & narHash, std::function<void(Sink &)> populate) = 0;
 };
 
+/**
+ * Create an in-memory only NAR cache.
+ */
+std::unique_ptr<NarCache> makeMemoryNarCache();
+
 } // namespace nix

src/libutil/nar-cache.cc

@@ -1,84 +1,36 @@
 #include "nix/util/nar-cache.hh"
-#include "nix/util/file-system.hh"
-
-#include <nlohmann/json.hpp>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
 
 namespace nix {
 
-NarCache::NarCache(std::optional<std::filesystem::path> cacheDir_)
-    : cacheDir(std::move(cacheDir_))
-{
-    if (cacheDir)
-        createDirs(*cacheDir);
-}
+namespace {
 
-ref<SourceAccessor> NarCache::getOrInsert(const Hash & narHash, std::function<void(Sink &)> populate)
+/**
+ * In-memory only NAR cache (private implementation).
+ */
+class MemoryNarCache : public NarCache
+{
+public:
+    ref<NarAccessor> getOrInsert(const Hash & narHash, std::function<void(Sink &)> populate) override;
+};
+
+} // anonymous namespace
+
+ref<NarAccessor> MemoryNarCache::getOrInsert(const Hash & narHash, std::function<void(Sink &)> populate)
 {
     // Check in-memory cache first
     if (auto * accessor = get(nars, narHash))
         return *accessor;
 
-    auto cacheAccessor = [&](ref<SourceAccessor> accessor) {
-        nars.emplace(narHash, accessor);
-        return accessor;
-    };
+    StringSink sink;
+    populate(sink);
+    auto accessor = makeNarAccessor(std::move(sink.s));
+    nars.emplace(narHash, accessor);
+    return accessor;
+}
 
-    auto getNar = [&]() {
-        StringSink sink;
-        populate(sink);
-        return std::move(sink.s);
-    };
-
-    if (cacheDir) {
-        auto makeCacheFile = [&](const std::string & ext) {
-            auto res = *cacheDir / narHash.to_string(HashFormat::Nix32, false);
-            res += ".";
-            res += ext;
-            return res;
-        };
-
-        auto cacheFile = makeCacheFile("nar");
-        auto listingFile = makeCacheFile("ls");
-
-        if (nix::pathExists(cacheFile)) {
-            try {
-                return cacheAccessor(makeLazyNarAccessor(
-                    nlohmann::json::parse(nix::readFile(listingFile)).template get<NarListing>(),
-                    seekableGetNarBytes(cacheFile)));
-            } catch (SystemError &) {
-            }
-
-            try {
-                return cacheAccessor(makeNarAccessor(nix::readFile(cacheFile)));
-            } catch (SystemError &) {
-            }
-        }
-
-        auto nar = getNar();
-
-        try {
-            /* FIXME: do this asynchronously. */
-            writeFile(cacheFile, nar);
-        } catch (...) {
-            ignoreExceptionExceptInterrupt();
-        }
-
-        auto narAccessor = makeNarAccessor(std::move(nar));
-
-        try {
-            nlohmann::json j = narAccessor->getListing();
-            writeFile(listingFile, j.dump());
-        } catch (...) {
-            ignoreExceptionExceptInterrupt();
-        }
-
-        return cacheAccessor(narAccessor);
-    }
-
-    return cacheAccessor(makeNarAccessor(getNar()));
+std::unique_ptr<NarCache> makeMemoryNarCache()
+{
+    return std::make_unique<MemoryNarCache>();
 }
 
 } // namespace nix