Probably fix a segfault in PathLocks

This commit is contained in:
Eelco Dolstra 2016-12-09 13:26:43 +01:00
parent b30d1e7ada
commit 47f587700d
No known key found for this signature in database
GPG key ID: 8170B4726D7198DE

View file

@ -1,5 +1,6 @@
#include "pathlocks.hh" #include "pathlocks.hh"
#include "util.hh" #include "util.hh"
#include "sync.hh"
#include <cerrno> #include <cerrno>
#include <cstdlib> #include <cstdlib>
@ -72,7 +73,7 @@ bool lockFile(int fd, LockType lockType, bool wait)
close a descriptor, the previous lock will be closed as well. And 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 there is no way to query whether we already have a lock (F_GETLK
only works on locks held by other processes). */ only works on locks held by other processes). */
static StringSet lockedPaths; /* !!! not thread-safe */ static Sync<StringSet> lockedPaths_;
PathLocks::PathLocks() PathLocks::PathLocks()
@ -108,49 +109,60 @@ bool PathLocks::lockPaths(const PathSet & _paths,
debug(format("locking path %1%") % path); debug(format("locking path %1%") % path);
if (lockedPaths.find(lockPath) != lockedPaths.end()) {
throw Error("deadlock: trying to re-acquire self-held lock"); auto lockedPaths(lockedPaths_.lock());
if (lockedPaths->count(lockPath))
AutoCloseFD fd; throw Error("deadlock: trying to re-acquire self-held lock %s", lockPath);
lockedPaths->insert(lockPath);
while (1) { }
/* Open/create the lock file. */ try {
fd = openLockFile(lockPath, true);
AutoCloseFD fd;
/* Acquire an exclusive lock. */
if (!lockFile(fd.get(), ltWrite, false)) { while (1) {
if (wait) {
if (waitMsg != "") printError(waitMsg); /* Open/create the lock file. */
lockFile(fd.get(), ltWrite, true); fd = openLockFile(lockPath, true);
} else {
/* Failed to lock this path; release all other /* Acquire an exclusive lock. */
locks. */ if (!lockFile(fd.get(), ltWrite, false)) {
unlock(); if (wait) {
return false; if (waitMsg != "") printError(waitMsg);
} lockFile(fd.get(), ltWrite, true);
} } else {
/* Failed to lock this path; release all other
debug(format("lock acquired on %1%") % lockPath); locks. */
unlock();
/* Check that the lock file hasn't become stale (i.e., return false;
hasn't been unlinked). */ }
struct stat st; }
if (fstat(fd.get(), &st) == -1)
throw SysError(format("statting lock file %1%") % lockPath); debug(format("lock acquired on %1%") % lockPath);
if (st.st_size != 0)
/* This lock file has been unlinked, so we're holding /* Check that the lock file hasn't become stale (i.e.,
a lock on a deleted file. This means that other hasn't been unlinked). */
processes may create and acquire a lock on struct stat st;
`lockPath', and proceed. So we must retry. */ if (fstat(fd.get(), &st) == -1)
debug(format("open lock file %1% has become stale") % lockPath); throw SysError(format("statting lock file %1%") % lockPath);
else if (st.st_size != 0)
break; /* This lock file has been unlinked, so we're holding
a lock on a deleted file. This means that other
processes may create and acquire a lock on
`lockPath', and proceed. So we must retry. */
debug(format("open lock file %1% has become stale") % lockPath);
else
break;
}
/* Use borrow so that the descriptor isn't closed. */
fds.push_back(FDPair(fd.release(), lockPath));
} catch (...) {
lockedPaths_.lock()->erase(lockPath);
throw;
} }
/* Use borrow so that the descriptor isn't closed. */
fds.push_back(FDPair(fd.release(), lockPath));
lockedPaths.insert(lockPath);
} }
return true; return true;
@ -172,7 +184,8 @@ void PathLocks::unlock()
for (auto & i : fds) { for (auto & i : fds) {
if (deletePaths) deleteLockFile(i.second, i.first); if (deletePaths) deleteLockFile(i.second, i.first);
lockedPaths.erase(i.second); lockedPaths_.lock()->erase(i.second);
if (close(i.first) == -1) if (close(i.first) == -1)
printError( printError(
format("error (ignored): cannot close lock file on %1%") % i.second); format("error (ignored): cannot close lock file on %1%") % i.second);
@ -193,7 +206,7 @@ void PathLocks::setDeletion(bool deletePaths)
bool pathIsLockedByMe(const Path & path) bool pathIsLockedByMe(const Path & path)
{ {
Path lockPath = path + ".lock"; Path lockPath = path + ".lock";
return lockedPaths.find(lockPath) != lockedPaths.end(); return lockedPaths_.lock()->count(lockPath);
} }