Merge pull request NixOS#4269 from obsidiansystems/sync-hash-derivati…

…on-modulo-cache Make drv hash modulo memo table thread-safe
zimbatm · Nov 19, 2020 · bc4df33 · bc4df33
2 parents 79aa7d9 + 2113ae2
commit bc4df33
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 16 deletions.
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
@@ -1132,9 +1132,10 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * *
 
        However, we don't bother doing this for floating CA derivations because
        their "hash modulo" is indeterminate until built. */
-    if (drv.type() != DerivationType::CAFloating)
-        drvHashes.insert_or_assign(drvPath,
-            hashDerivationModulo(*state.store, Derivation(drv), false));
+    if (drv.type() != DerivationType::CAFloating) {
+        auto h = hashDerivationModulo(*state.store, Derivation(drv), false);
+        drvHashes.lock()->insert_or_assign(drvPath, h);
+    }
 
     state.mkAttrs(v, 1 + drv.outputs.size());
     mkString(*state.allocAttr(v, state.sDrvPath), drvPathS, {"=" + drvPathS});

diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc
@@ -451,27 +451,30 @@ DerivationType BasicDerivation::type() const
 }
 
 
-DrvHashes drvHashes;
+Sync<DrvHashes> drvHashes;
 
 /* pathDerivationModulo and hashDerivationModulo are mutually recursive
  */
 
 /* Look up the derivation by value and memoize the
    `hashDerivationModulo` call.
  */
-static const DrvHashModulo & pathDerivationModulo(Store & store, const StorePath & drvPath)
+static const DrvHashModulo pathDerivationModulo(Store & store, const StorePath & drvPath)
 {
-    auto h = drvHashes.find(drvPath);
-    if (h == drvHashes.end()) {
-        // Cache it
-        h = drvHashes.insert_or_assign(
-            drvPath,
-            hashDerivationModulo(
-                store,
-                store.readInvalidDerivation(drvPath),
-                false)).first;
+    {
+        auto hashes = drvHashes.lock();
+        auto h = hashes->find(drvPath);
+        if (h != hashes->end()) {
+            return h->second;
+        }
     }
-    return h->second;
+    auto h = hashDerivationModulo(
+        store,
+        store.readInvalidDerivation(drvPath),
+        false);
+    // Cache it
+    drvHashes.lock()->insert_or_assign(drvPath, h);
+    return h;
 }
 
 /* See the header for interface details. These are the implementation details.

diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh
@@ -210,7 +210,8 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m
 /* Memoisation of hashDerivationModulo(). */
 typedef std::map<StorePath, DrvHashModulo> DrvHashes;
 
-extern DrvHashes drvHashes; // FIXME: global, not thread-safe
+// FIXME: global, though at least thread-safe.
+extern Sync<DrvHashes> drvHashes;
 
 /* Memoisation of `readDerivation(..).resove()`. */
 typedef std::map<