From d99d04e6442dcc39a24cebac01af117ce00a5006 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 1 Aug 2003 15:06:23 +0000 Subject: [PATCH] * Defensive programming against POSIX locking idiocy. * Simplified realiseSlice(). --- src/normalise.cc | 25 ------------------------- src/pathlocks.cc | 17 +++++++++++++++++ src/pathlocks.hh | 1 + 3 files changed, 18 insertions(+), 25 deletions(-) diff --git a/src/normalise.cc b/src/normalise.cc index cc84b6541..2eb34d4ee 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 ac53dc643..fc05a7b55 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::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 ae3a37cdb..03a62e65a 100644 --- a/src/pathlocks.hh +++ b/src/pathlocks.hh @@ -8,6 +8,7 @@ class PathLocks { private: list fds; + Strings paths; public: PathLocks(const Strings & _paths);