Merge pull request #14038 from NixOS/thread-safe-dummy

libstore: Make writable dummy store thread-safe

Author: Ericson2314

src/libstore/dummy-store.cc

@@ -4,6 +4,8 @@
 #include "nix/util/memory-source-accessor.hh"
 #include "nix/store/dummy-store.hh"
 
+#include <boost/unordered/concurrent_flat_map.hpp>
+
 namespace nix {
 
 std::string DummyStoreConfig::doc()
@@ -13,6 +15,99 @@ std::string DummyStoreConfig::doc()
         ;
 }
 
+namespace {
+
+class WholeStoreViewAccessor : public SourceAccessor
+{
+    using BaseName = std::string;
+
+    /**
+     * Map from store path basenames to corresponding accessors.
+     */
+    boost::concurrent_flat_map<BaseName, ref<MemorySourceAccessor>> subdirs;
+
+    /**
+     * Helper accessor for accessing just the CanonPath::root.
+     */
+    MemorySourceAccessor rootPathAccessor;
+
+    /**
+     * Helper empty accessor.
+     */
+    MemorySourceAccessor emptyAccessor;
+
+    auto
+    callWithAccessorForPath(CanonPath path, std::invocable<MemorySourceAccessor &, const CanonPath &> auto callback)
+    {
+        if (path.isRoot())
+            return callback(rootPathAccessor, path);
+
+        BaseName baseName(*path.begin());
+        MemorySourceAccessor * res = nullptr;
+
+        subdirs.cvisit(baseName, [&](const auto & kv) {
+            path = path.removePrefix(CanonPath{baseName});
+            res = &*kv.second;
+        });
+
+        if (!res)
+            res = &emptyAccessor;
+
+        return callback(*res, path);
+    }
+
+public:
+    WholeStoreViewAccessor()
+    {
+        MemorySink sink{rootPathAccessor};
+        sink.createDirectory(CanonPath::root);
+    }
+
+    void addObject(std::string_view baseName, ref<MemorySourceAccessor> accessor)
+    {
+        subdirs.emplace(baseName, std::move(accessor));
+    }
+
+    std::string readFile(const CanonPath & path) override
+    {
+        return callWithAccessorForPath(
+            path, [](SourceAccessor & accessor, const CanonPath & path) { return accessor.readFile(path); });
+    }
+
+    void readFile(const CanonPath & path, Sink & sink, std::function<void(uint64_t)> sizeCallback) override
+    {
+        return callWithAccessorForPath(path, [&](SourceAccessor & accessor, const CanonPath & path) {
+            return accessor.readFile(path, sink, sizeCallback);
+        });
+    }
+
+    bool pathExists(const CanonPath & path) override
+    {
+        return callWithAccessorForPath(
+            path, [](SourceAccessor & accessor, const CanonPath & path) { return accessor.pathExists(path); });
+    }
+
+    std::optional<Stat> maybeLstat(const CanonPath & path) override
+    {
+        return callWithAccessorForPath(
+            path, [](SourceAccessor & accessor, const CanonPath & path) { return accessor.maybeLstat(path); });
+    }
+
+    DirEntries readDirectory(const CanonPath & path) override
+    {
+        return callWithAccessorForPath(
+            path, [](SourceAccessor & accessor, const CanonPath & path) { return accessor.readDirectory(path); });
+    }
+
+    std::string readLink(const CanonPath & path) override
+    {
+        return callWithAccessorForPath(
+            path, [](SourceAccessor & accessor, const CanonPath & path) { return accessor.readLink(path); });
+    }
+};
+
+} // namespace
+
 struct DummyStore : virtual Store
 {
     using Config = DummyStoreConfig;
@@ -29,7 +124,7 @@ struct DummyStore : virtual Store
      * This is map conceptually owns the file system objects for each
      * store object.
      */
-    std::map<StorePath, PathInfoAndContents> contents;
+    boost::concurrent_flat_map<StorePath, PathInfoAndContents> contents;
 
     /**
      * This view conceptually just borrows the file systems objects of
@@ -38,23 +133,23 @@ struct DummyStore : virtual Store
      *
      * This is needed just in order to implement `Store::getFSAccessor`.
      */
-    ref<MemorySourceAccessor> wholeStoreView = make_ref<MemorySourceAccessor>();
+    ref<WholeStoreViewAccessor> wholeStoreView = make_ref<WholeStoreViewAccessor>();
 
     DummyStore(ref<const Config> config)
         : Store{*config}
         , config(config)
     {
         wholeStoreView->setPathDisplay(config->storeDir);
-        MemorySink sink{*wholeStoreView};
-        sink.createDirectory(CanonPath::root);
     }
 
     void queryPathInfoUncached(
         const StorePath & path, Callback<std::shared_ptr<const ValidPathInfo>> callback) noexcept override
     {
-        if (auto it = contents.find(path); it != contents.end())
-            callback(std::make_shared<ValidPathInfo>(StorePath{path}, it->second.info));
-        else
+        bool visited = contents.cvisit(path, [&](const auto & kv) {
+            callback(std::make_shared<ValidPathInfo>(StorePath{kv.first}, kv.second.info));
+        });
+
+        if (!visited)
             callback(nullptr);
     }
 
@@ -87,19 +182,14 @@ struct DummyStore : virtual Store
         parseDump(tempSink, source);
         auto path = info.path;
 
-        auto [it, _] = contents.insert({
-            path,
-            {
-                std::move(info),
-                make_ref<MemorySourceAccessor>(std::move(*temp)),
-            },
-        });
-
-        auto & pathAndContents = it->second;
-
-        bool inserted = wholeStoreView->open(CanonPath(path.to_string()), pathAndContents.contents->root);
-        if (!inserted)
-            unreachable();
+        auto accessor = make_ref<MemorySourceAccessor>(std::move(*temp));
+        contents.insert(
+            {path,
+             PathInfoAndContents{
+                 std::move(info),
+                 accessor,
+             }});
+        wholeStoreView->addObject(path.to_string(), accessor);
     }
 
     StorePath addToStoreFromDump(
@@ -156,33 +246,28 @@ struct DummyStore : virtual Store
         info.narSize = narHash.second.value();
 
         auto path = info.path;
-
-        auto [it, _] = contents.insert({
-            path,
-            {
-                std::move(info),
-                make_ref<MemorySourceAccessor>(std::move(*temp)),
-            },
-        });
-
-        auto & pathAndContents = it->second;
-
-        bool inserted = wholeStoreView->open(CanonPath(path.to_string()), pathAndContents.contents->root);
-        if (!inserted)
-            unreachable();
+        auto accessor = make_ref<MemorySourceAccessor>(std::move(*temp));
+        contents.insert(
+            {path,
+             PathInfoAndContents{
+                 std::move(info),
+                 accessor,
+             }});
+        wholeStoreView->addObject(path.to_string(), accessor);
 
         return path;
     }
 
     void narFromPath(const StorePath & path, Sink & sink) override
     {
-        auto object = contents.find(path);
-        if (object == contents.end())
-            throw Error("path '%s' is not valid", printStorePath(path));
+        bool visited = contents.cvisit(path, [&](const auto & kv) {
+            const auto & [info, accessor] = kv.second;
+            SourcePath sourcePath(accessor);
+            dumpPath(sourcePath, sink, FileSerialisationMethod::NixArchive);
+        });
 
-        const auto & [info, accessor] = object->second;
-        SourcePath sourcePath(accessor);
-        dumpPath(sourcePath, sink, FileSerialisationMethod::NixArchive);
+        if (!visited)
+            throw Error("path '%s' is not valid", printStorePath(path));
     }
 
     void

src/libstore/include/nix/store/path.hh

@@ -108,4 +108,13 @@ struct hash<nix::StorePath>
 
 } // namespace std
 
+namespace nix {
+
+inline std::size_t hash_value(const StorePath & path)
+{
+    return std::hash<StorePath>{}(path);
+}
+
+} // namespace nix
+
 JSON_IMPL(nix::StorePath)

src/libutil-tests/git.cc

@@ -233,30 +233,30 @@ TEST_F(GitTest, both_roundrip)
         .contents{
             {
                 "foo",
-                make_ref<File>(File::Regular{
+                File::Regular{
                     .contents = "hello\n\0\n\tworld!",
-                }),
+                },
             },
             {
                 "bar",
-                make_ref<File>(File::Directory{
+                File::Directory{
                     .contents =
                         {
                             {
                                 "baz",
-                                make_ref<File>(File::Regular{
+                                File::Regular{
                                     .executable = true,
                                     .contents = "good day,\n\0\n\tworld!",
-                                }),
+                                },
                             },
                             {
                                 "quux",
-                                make_ref<File>(File::Symlink{
+                                File::Symlink{
                                     .target = "/over/there",
-                                }),
+                                },
                             },
                         },
-                }),
+                },
             },
         },
     };

src/libutil/include/nix/util/memory-source-accessor.hh

@@ -35,7 +35,7 @@ struct MemorySourceAccessor : virtual SourceAccessor
         {
             using Name = std::string;
 
-            std::map<Name, ref<File>, std::less<>> contents;
+            std::map<Name, File, std::less<>> contents;
 
             bool operator==(const Directory &) const noexcept;
             // TODO libc++ 16 (used by darwin) missing `std::map::operator <=>`, can't do yet.
@@ -89,21 +89,13 @@ struct MemorySourceAccessor : virtual SourceAccessor
     SourcePath addFile(CanonPath path, std::string && contents);
 };
 
-inline bool
-MemorySourceAccessor::File::Directory::operator==(const MemorySourceAccessor::File::Directory & other) const noexcept
-{
-    return std::ranges::equal(contents, other.contents, [](const auto & lhs, const auto & rhs) -> bool {
-        return lhs.first == rhs.first && *lhs.second == *rhs.second;
-    });
-};
+inline bool MemorySourceAccessor::File::Directory::operator==(
+    const MemorySourceAccessor::File::Directory &) const noexcept = default;
 
 inline bool
 MemorySourceAccessor::File::Directory::operator<(const MemorySourceAccessor::File::Directory & other) const noexcept
 {
-    return std::ranges::lexicographical_compare(
-        contents, other.contents, [](const auto & lhs, const auto & rhs) -> bool {
-            return lhs.first < rhs.first && *lhs.second < *rhs.second;
-        });
+    return contents < other.contents;
 }
 
 inline bool MemorySourceAccessor::File::operator==(const MemorySourceAccessor::File &) const noexcept = default;

src/libutil/memory-source-accessor.cc

@@ -39,11 +39,11 @@ MemorySourceAccessor::File * MemorySourceAccessor::open(const CanonPath & path,
                     i,
                     {
                         std::string{name},
-                        make_ref<File>(File::Directory{}),
+                        File::Directory{},
                     });
             }
         }
-        cur = &*i->second;
+        cur = &i->second;
     }
 
     if (newF && create)
@@ -107,7 +107,7 @@ MemorySourceAccessor::DirEntries MemorySourceAccessor::readDirectory(const Canon
     if (auto * d = std::get_if<File::Directory>(&f->raw)) {
         DirEntries res;
         for (auto & [name, file] : d->contents)
-            res.insert_or_assign(name, file->lstat().type);
+            res.insert_or_assign(name, file.lstat().type);
         return res;
     } else
         throw Error("file '%s' is not a directory", path);