about summary refs log tree commit diff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/build-remote/build-remote.cc2
-rw-r--r--src/libstore/build.cc30
-rw-r--r--src/libstore/local-store.cc3
-rw-r--r--src/libstore/local-store.hh3
-rw-r--r--src/libstore/pathlocks.cc6
-rw-r--r--src/libstore/pathlocks.hh6
-rw-r--r--src/nix/build.cc2
-rw-r--r--src/nix/command.hh2
-rw-r--r--src/nix/installables.cc4
9 files changed, 31 insertions, 27 deletions
diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc
index df579729af29..c6e75e8cc704 100644
--- a/src/build-remote/build-remote.cc
+++ b/src/build-remote/build-remote.cc
@@ -241,7 +241,7 @@ connected:
 
         if (!missing.empty()) {
             Activity act(*logger, lvlTalkative, actUnknown, fmt("copying outputs from '%s'", storeUri));
-            setenv("NIX_HELD_LOCKS", concatStringsSep(" ", missing).c_str(), 1); /* FIXME: ugly */
+            store->locksHeld.insert(missing.begin(), missing.end()); /* FIXME: ugly */
             copyPaths(ref<Store>(sshStore), store, missing, NoRepair, NoCheckSigs, substitute);
         }
 
diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index 392b494e65eb..cc69ff1c74bf 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -1335,19 +1335,6 @@ void DerivationGoal::tryToBuild()
 {
     trace("trying to build");
 
-    /* Check for the possibility that some other goal in this process
-       has locked the output since we checked in haveDerivation().
-       (It can't happen between here and the lockPaths() call below
-       because we're not allowing multi-threading.)  If so, put this
-       goal to sleep until another goal finishes, then try again. */
-    for (auto & i : drv->outputs)
-        if (pathIsLockedByMe(worker.store.toRealPath(i.second.path))) {
-            debug(format("putting derivation '%1%' to sleep because '%2%' is locked by another goal")
-                % drvPath % i.second.path);
-            worker.waitForAnyGoal(shared_from_this());
-            return;
-        }
-
     /* Obtain locks on all output paths.  The locks are automatically
        released when we exit this function or Nix crashes.  If we
        can't acquire the lock, then continue; hopefully some other
@@ -3739,6 +3726,17 @@ void SubstitutionGoal::tryToRun()
         return;
     }
 
+    /* If the store path is already locked (probably by a
+       DerivationGoal), then put this goal to sleep. Note: we don't
+       acquire a lock here since that breaks addToStore(), so below we
+       handle an AlreadyLocked exception from addToStore(). The check
+       here is just an optimisation to prevent having to redo a
+       download due to a locked path. */
+    if (pathIsLockedByMe(worker.store.toRealPath(storePath))) {
+        worker.waitForAWhile(shared_from_this());
+        return;
+    }
+
     maintainRunningSubstitutions = std::make_unique<MaintainCount<uint64_t>>(worker.runningSubstitutions);
     worker.updateProgress();
 
@@ -3778,6 +3776,12 @@ void SubstitutionGoal::finished()
 
     try {
         promise.get_future().get();
+    } catch (AlreadyLocked & e) {
+        /* Probably a DerivationGoal is already building this store
+           path. Sleep for a while and try again. */
+        state = &SubstitutionGoal::init;
+        worker.waitForAWhile(shared_from_this());
+        return;
     } catch (Error & e) {
         printError(e.msg());
 
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index 7afecc1cfc62..4afe51ea91ec 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -992,8 +992,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, const ref<std::string> &
         /* Lock the output path.  But don't lock if we're being called
            from a build hook (whose parent process already acquired a
            lock on this path). */
-        Strings locksHeld = tokenizeString<Strings>(getEnv("NIX_HELD_LOCKS"));
-        if (find(locksHeld.begin(), locksHeld.end(), info.path) == locksHeld.end())
+        if (!locksHeld.count(info.path))
             outputLock.lockPaths({realPath});
 
         if (repair || !isValidPath(info.path)) {
diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index 30bef3a799d4..bbd50e1c1451 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -104,6 +104,9 @@ private:
 
 public:
 
+    // Hack for build-remote.cc.
+    PathSet locksHeld = tokenizeString<PathSet>(getEnv("NIX_HELD_LOCKS"));
+
     /* Initialise the local store, upgrading the schema if
        necessary. */
     LocalStore(const Params & params);
diff --git a/src/libstore/pathlocks.cc b/src/libstore/pathlocks.cc
index 587f29598851..08d1efdbeb01 100644
--- a/src/libstore/pathlocks.cc
+++ b/src/libstore/pathlocks.cc
@@ -113,8 +113,10 @@ bool PathLocks::lockPaths(const PathSet & _paths,
 
         {
             auto lockedPaths(lockedPaths_.lock());
-            if (lockedPaths->count(lockPath))
-                throw Error("deadlock: trying to re-acquire self-held lock '%s'", lockPath);
+            if (lockedPaths->count(lockPath)) {
+                if (!wait) return false;
+                throw AlreadyLocked("deadlock: trying to re-acquire self-held lock '%s'", lockPath);
+            }
             lockedPaths->insert(lockPath);
         }
 
diff --git a/src/libstore/pathlocks.hh b/src/libstore/pathlocks.hh
index 2a7de611446e..db51f950a320 100644
--- a/src/libstore/pathlocks.hh
+++ b/src/libstore/pathlocks.hh
@@ -2,10 +2,8 @@
 
 #include "util.hh"
 
-
 namespace nix {
 
-
 /* Open (possibly create) a lock file and return the file descriptor.
    -1 is returned if create is false and the lock could not be opened
    because it doesn't exist.  Any other error throws an exception. */
@@ -18,6 +16,7 @@ enum LockType { ltRead, ltWrite, ltNone };
 
 bool lockFile(int fd, LockType lockType, bool wait);
 
+MakeError(AlreadyLocked, Error);
 
 class PathLocks
 {
@@ -38,9 +37,6 @@ public:
     void setDeletion(bool deletePaths);
 };
 
-
-// FIXME: not thread-safe!
 bool pathIsLockedByMe(const Path & path);
 
-
 }
diff --git a/src/nix/build.cc b/src/nix/build.cc
index f7c99f12dbbf..b4f21b32d78f 100644
--- a/src/nix/build.cc
+++ b/src/nix/build.cc
@@ -50,7 +50,7 @@ struct CmdBuild : MixDryRun, InstallablesCommand
 
     void run(ref<Store> store) override
     {
-        auto buildables = toBuildables(store, dryRun ? DryRun : Build, installables);
+        auto buildables = build(store, dryRun ? DryRun : Build, installables);
 
         for (size_t i = 0; i < buildables.size(); ++i) {
             auto & b(buildables[i]);
diff --git a/src/nix/command.hh b/src/nix/command.hh
index a7863c49f37a..97a6fee7fd27 100644
--- a/src/nix/command.hh
+++ b/src/nix/command.hh
@@ -198,7 +198,7 @@ std::shared_ptr<Installable> parseInstallable(
     SourceExprCommand & cmd, ref<Store> store, const std::string & installable,
     bool useDefaultInstallables);
 
-Buildables toBuildables(ref<Store> store, RealiseMode mode,
+Buildables build(ref<Store> store, RealiseMode mode,
     std::vector<std::shared_ptr<Installable>> installables);
 
 PathSet toStorePaths(ref<Store> store, RealiseMode mode,
diff --git a/src/nix/installables.cc b/src/nix/installables.cc
index c3b06c22eba8..a3fdd8a2808d 100644
--- a/src/nix/installables.cc
+++ b/src/nix/installables.cc
@@ -253,7 +253,7 @@ std::shared_ptr<Installable> parseInstallable(
     return installables.front();
 }
 
-Buildables toBuildables(ref<Store> store, RealiseMode mode,
+Buildables build(ref<Store> store, RealiseMode mode,
     std::vector<std::shared_ptr<Installable>> installables)
 {
     if (mode != Build)
@@ -291,7 +291,7 @@ PathSet toStorePaths(ref<Store> store, RealiseMode mode,
 {
     PathSet outPaths;
 
-    for (auto & b : toBuildables(store, mode, installables))
+    for (auto & b : build(store, mode, installables))
         for (auto & output : b.outputs)
             outPaths.insert(output.second);
generated by cgit-pink 1.4.1 (git 2.44.2) at 2024-10-21T23ยท37+0000