* Defensive programming against POSIX locking idiocy.

* Simplified realiseSlice().

3 files changed, 18 insertions, 25 deletions

diff --git a/src/normalise.cc b/src/normalise.cc
index cc84b6541f39..2eb34d4ee2da 100644
--- a/src/normalise.cc
+++ b/src/normalise.cc
@@ -240,35 +240,10 @@ void realiseSlice(const FSId & id, FSIdSet pending)
     if (fs.type != FState::fsSlice)
         throw Error(format("expected slice in %1%") % (string) id);
     
-    /* Perhaps all paths already contain the right id? */
-
-    bool missing = false;
-    for (SliceElems::const_iterator i = fs.slice.elems.begin();
-         i != fs.slice.elems.end(); i++)
-    {
-        SliceElem elem = *i;
-        string id;
-        if (!nixDB.queryString(noTxn, dbPath2Id, elem.path, id)) {
-            if (pathExists(elem.path))
-                throw Error(format("path `%1%' obstructed") % elem.path);
-            missing = true;
-            break;
-        }
-        if (parseHash(id) != elem.id)
-            throw Error(format("path `%1%' obstructed") % elem.path);
-    }
-
-    if (!missing) {
-        debug(format("already installed"));
-        return;
-    }
-
-    /* For each element, expand its id at its path. */
     for (SliceElems::const_iterator i = fs.slice.elems.begin();
          i != fs.slice.elems.end(); i++)
     {
         SliceElem elem = *i;
-        debug(format("expanding %1% in `%2%'") % (string) elem.id % elem.path);
         expandId(elem.id, elem.path, "/", pending);
     }
 }
diff --git a/src/pathlocks.cc b/src/pathlocks.cc
index ac53dc6437d0..fc05a7b5508c 100644
--- a/src/pathlocks.cc
+++ b/src/pathlocks.cc
@@ -3,6 +3,14 @@
 #include "pathlocks.hh"
 
 
+/* This enables us to check whether are not already holding a lock on
+   a file ourselves.  POSIX locks (fcntl) suck in this respect: if we
+   close a descriptor, the previous lock will be closed as well.  And
+   there is no way to query whether we already have a lock (F_GETLK
+   only works on locks held by other processes). */
+static StringSet lockedPaths; /* !!! not thread-safe */
+
+
 PathLocks::PathLocks(const Strings & _paths)
 {
     /* Note that `fds' is built incrementally so that the destructor
@@ -19,6 +27,9 @@ PathLocks::PathLocks(const Strings & _paths)
         string lockPath = path + ".lock";
 
         debug(format("locking path `%1%'") % path);
+
+        if (lockedPaths.find(lockPath) != lockedPaths.end())
+            throw Error(format("already holding lock on `%1%'") % lockPath);
         
         /* Open/create the lock file. */
         int fd = open(lockPath.c_str(), O_WRONLY | O_CREAT, 0666);
@@ -26,6 +37,7 @@ PathLocks::PathLocks(const Strings & _paths)
             throw SysError(format("opening lock file `%1%'") % lockPath);
 
         fds.push_back(fd);
+        this->paths.push_back(lockPath);
 
         /* Lock it. */
         struct flock lock;
@@ -37,6 +49,8 @@ PathLocks::PathLocks(const Strings & _paths)
         while (fcntl(fd, F_SETLKW, &lock) == -1)
             if (errno != EINTR)
                 throw SysError(format("acquiring lock on `%1%'") % lockPath);
+
+        lockedPaths.insert(lockPath);
     }
 }
 
@@ -45,4 +59,7 @@ PathLocks::~PathLocks()
 {
     for (list<int>::iterator i = fds.begin(); i != fds.end(); i++)
         close(*i);
+
+    for (Strings::iterator i = paths.begin(); i != paths.end(); i++)
+        lockedPaths.erase(*i);
 }
diff --git a/src/pathlocks.hh b/src/pathlocks.hh
index ae3a37cdb214..03a62e65a782 100644
--- a/src/pathlocks.hh
+++ b/src/pathlocks.hh
@@ -8,6 +8,7 @@ class PathLocks
 {
 private:
     list<int> fds;
+    Strings paths;
 
 public:
     PathLocks(const Strings & _paths);