* Make lock removal safe by signalling to blocked processes that the

lock they are waiting on has become stale (we do this by writing a
  meaningless token to the unlinked file).
This commit is contained in:
Eelco Dolstra 2005-01-27 12:19:25 +00:00
parent a24b78e9f1
commit 59682e6188
4 changed files with 52 additions and 21 deletions

View file

@ -61,7 +61,7 @@ PathLocks::PathLocks(const PathSet & paths)
void PathLocks::lockPaths(const PathSet & _paths) void PathLocks::lockPaths(const PathSet & _paths)
{ {
/* May be called only once! */ /* May be called only once! */
assert(this->paths.empty()); assert(fds.empty());
/* Note that `fds' is built incrementally so that the destructor /* Note that `fds' is built incrementally so that the destructor
will only release those locks that we have already acquired. */ will only release those locks that we have already acquired. */
@ -84,19 +84,37 @@ void PathLocks::lockPaths(const PathSet & _paths)
continue; continue;
} }
AutoCloseFD fd;
while (1) {
/* Open/create the lock file. */ /* Open/create the lock file. */
int fd = open(lockPath.c_str(), O_WRONLY | O_CREAT, 0666); fd = open(lockPath.c_str(), O_WRONLY | O_CREAT, 0666);
if (fd == -1) if (fd == -1)
throw SysError(format("opening lock file `%1%'") % lockPath); throw SysError(format("opening lock file `%1%'") % lockPath);
fds.push_back(fd);
this->paths.push_back(lockPath);
/* Acquire an exclusive lock. */ /* Acquire an exclusive lock. */
lockFile(fd, ltWrite, true); lockFile(fd, ltWrite, true);
debug(format("lock acquired on `%1%'") % lockPath); debug(format("lock acquired on `%1%'") % lockPath);
/* Check that the lock file hasn't become stale (i.e.,
hasn't been unlinked). */
struct stat st;
if (fstat(fd, &st) == -1)
throw SysError(format("statting lock file `%1%'") % lockPath);
if (st.st_size != 0)
/* 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.borrow(), lockPath));
lockedPaths.insert(lockPath); lockedPaths.insert(lockPath);
} }
} }
@ -104,19 +122,22 @@ void PathLocks::lockPaths(const PathSet & _paths)
PathLocks::~PathLocks() PathLocks::~PathLocks()
{ {
for (list<int>::iterator i = fds.begin(); i != fds.end(); i++) for (list<FDPair>::iterator i = fds.begin(); i != fds.end(); i++) {
if (close(*i) != 0) throw SysError("closing fd");
for (Paths::iterator i = paths.begin(); i != paths.end(); i++) {
checkInterrupt();
if (deletePaths) { if (deletePaths) {
/* This is not safe in general! */ /* Write a (meaningless) token to the file to indicate to
unlink(i->c_str()); other processes waiting on this lock that the lock is
stale (deleted). */
if (write(i->first, "d", 1) == 1) {
unlink(i->second.c_str());
}
/* Note that the result of unlink() is ignored; removing /* Note that the result of unlink() is ignored; removing
the lock file is an optimisation, not a necessity. */ the lock file is an optimisation, not a necessity. */
} }
lockedPaths.erase(*i); lockedPaths.erase(i->second);
debug(format("lock released on `%1%'") % *i); if (close(i->first) == -1)
printMsg(lvlError,
format("error (ignored): cannot close lock file on `%1%'") % i->second);
debug(format("lock released on `%1%'") % i->second);
} }
} }

View file

@ -12,8 +12,8 @@ bool lockFile(int fd, LockType lockType, bool wait);
class PathLocks class PathLocks
{ {
private: private:
list<int> fds; typedef pair<int, Path> FDPair;
Paths paths; list<FDPair> fds;
bool deletePaths; bool deletePaths;
public: public:

View file

@ -415,6 +415,15 @@ bool AutoCloseFD::isOpen()
} }
/* Pass responsibility for closing this fd to the caller. */
int AutoCloseFD::borrow()
{
int oldFD = fd;
fd = -1;
return oldFD;
}
void Pipe::create() void Pipe::create()
{ {
int fds[2]; int fds[2];

View file

@ -184,6 +184,7 @@ public:
operator int(); operator int();
void close(); void close();
bool isOpen(); bool isOpen();
int borrow();
}; };