libstore: split tryLockFile lockFile

lockFile is currently interruptible by signals like SIGCHLD. which
happen a lot in daemons. now imagine that daemon enabled automatic
garbage collection. observe that the local store does not actually
check whether its lock operations have succeeded ... get the idea?

Change-Id: Ibfd7ee786c4fee3add72d4456a7e95e73e09c73e
This commit is contained in:
eldritch horrors
2025-02-03 21:56:26 +01:00
parent aa87c8aa93
commit 79f9c39e36
6 changed files with 52 additions and 43 deletions
+3 -3
View File
@@ -137,7 +137,7 @@ static int main_build_remote(AsyncIoRoot & aio, std::string programName, Strings
while (true) {
bestSlotLock.reset();
AutoCloseFD lock = openLockFile(currentLoad + "/main-lock", true);
lockFile(lock.get(), ltWrite, true);
lockFile(lock.get(), ltWrite);
bool rightType = false;
@@ -156,7 +156,7 @@ static int main_build_remote(AsyncIoRoot & aio, std::string programName, Strings
uint64_t load = 0;
for (uint64_t slot = 0; slot < m.maxJobs; ++slot) {
auto slotLock = openSlotLock(m, slot);
if (lockFile(slotLock.get(), ltWrite, false)) {
if (tryLockFile(slotLock.get(), ltWrite)) {
if (!free) {
free = std::move(slotLock);
}
@@ -281,7 +281,7 @@ connected:
auto old = signal(SIGALRM, handleAlarm);
alarm(15 * 60);
if (!lockFile(uploadLock.get(), ltWrite, true))
if (!lockFile(uploadLock.get(), ltWrite))
printError("somebody is hogging the upload lock for '%s', continuing...");
alarm(0);
signal(SIGALRM, old);
+3 -3
View File
@@ -93,7 +93,7 @@ void LocalStore::createTempRootsFile()
*fdTempRoots = openLockFile(fnTempRoots, true);
debug("acquiring write lock on '%s'", fnTempRoots);
lockFile(fdTempRoots->get(), ltWrite, true);
lockFile(fdTempRoots->get(), ltWrite);
/* Check whether the garbage collector didn't get in our
way. */
@@ -214,7 +214,7 @@ void LocalStore::findTempRoots(Roots & tempRoots, bool censor)
/* Try to acquire a write lock without blocking. This can
only succeed if the owning process has died. In that case
we don't care about its temporary roots. */
if (lockFile(fd.get(), ltWrite, false)) {
if (tryLockFile(fd.get(), ltWrite)) {
printInfo("removing stale temporary roots file '%1%'", path);
unlink(path.c_str());
writeFull(fd.get(), "d");
@@ -626,7 +626,7 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results)
exclusive lock before deleting them. */
if (baseName.find("tmp-", 0) == 0) {
AutoCloseFD tmpDirFd{open(realPath.c_str(), O_RDONLY | O_DIRECTORY)};
if (tmpDirFd.get() == -1 || !lockFile(tmpDirFd.get(), ltWrite, false)) {
if (tmpDirFd.get() == -1 || !tryLockFile(tmpDirFd.get(), ltWrite)) {
debug("skipping locked tempdir '%s'", realPath);
return;
}
+9 -9
View File
@@ -97,10 +97,10 @@ void migrateCASchema(SQLite& db, Path schemaPath, AutoCloseFD& lockFd)
curCASchema, nixCASchemaVersion);
}
if (!lockFile(lockFd.get(), ltWrite, false)) {
if (!tryLockFile(lockFd.get(), ltWrite)) {
printInfo("waiting for exclusive access to the Nix store for ca drvs...");
unlockFile(lockFd.get()); // We have acquired a shared lock; release it to prevent deadlocks
lockFile(lockFd.get(), ltWrite, true);
lockFile(lockFd.get(), ltWrite);
}
if (curCASchema == 0) {
@@ -168,7 +168,7 @@ void migrateCASchema(SQLite& db, Path schemaPath, AutoCloseFD& lockFd)
}
writeFile(schemaPath, fmt("%d", nixCASchemaVersion), 0666, true);
lockFile(lockFd.get(), ltRead, true);
lockFile(lockFd.get(), ltRead);
}
}
@@ -278,9 +278,9 @@ LocalStore::LocalStore(LocalStoreConfig config)
globalLock = openLockFile(globalLockPath.c_str(), true);
}
if (!config_.readOnly && !lockFile(globalLock.get(), ltRead, false)) {
if (!config_.readOnly && !tryLockFile(globalLock.get(), ltRead)) {
printInfo("waiting for the big Nix store lock...");
lockFile(globalLock.get(), ltRead, true);
lockFile(globalLock.get(), ltRead);
}
/* Check the current database schema and if necessary do an
@@ -317,10 +317,10 @@ LocalStore::LocalStore(LocalStoreConfig config)
"which is no longer supported. To convert to the new format,\n"
"please use the original Nix version 1.11 first.");
if (!lockFile(globalLock.get(), ltWrite, false)) {
if (!tryLockFile(globalLock.get(), ltWrite)) {
printInfo("waiting for exclusive access to the Nix store...");
unlockFile(globalLock.get()); // We have acquired a shared lock; release it to prevent deadlocks
lockFile(globalLock.get(), ltWrite, true);
lockFile(globalLock.get(), ltWrite);
}
/* Get the schema version again, because another process may
@@ -352,7 +352,7 @@ LocalStore::LocalStore(LocalStoreConfig config)
writeFile(schemaPath, fmt("%1%", nixSchemaVersion), 0666, true);
lockFile(globalLock.get(), ltRead, true);
lockFile(globalLock.get(), ltRead);
}
else openDB(*state, false);
@@ -1489,7 +1489,7 @@ std::pair<Path, AutoCloseFD> LocalStore::createTempDirInStore()
if (tmpDirFd.get() < 0) {
continue;
}
lockedByUs = lockFile(tmpDirFd.get(), ltWrite, true);
lockedByUs = lockFile(tmpDirFd.get(), ltWrite);
} while (!pathExists(tmpDirFn) || !lockedByUs);
return {tmpDirFn, std::move(tmpDirFd)};
}
+2 -2
View File
@@ -90,7 +90,7 @@ struct SimpleUserLock : UserLock
if (!fd)
throw SysError("opening user lock '%s'", fnUserLock);
if (lockFile(fd.get(), ltWrite, false)) {
if (tryLockFile(fd.get(), ltWrite)) {
auto lock = std::make_unique<SimpleUserLock>();
lock->fdUserLock = std::move(fd);
@@ -162,7 +162,7 @@ struct AutoUserLock : UserLock
if (!fd)
throw SysError("opening user lock '%s'", fnUserLock);
if (lockFile(fd.get(), ltWrite, false)) {
if (tryLockFile(fd.get(), ltWrite)) {
auto firstUid = settings.startId + i * maxIdsPerBuild;
+33 -25
View File
@@ -35,29 +35,37 @@ void deleteLockFile(const Path & path, int fd)
file is an optimisation, not a necessity. */
}
bool lockFile(int fd, LockType lockType, bool wait)
static int convertLockType(LockType lockType)
{
int type;
if (lockType == ltRead) type = LOCK_SH;
else if (lockType == ltWrite) type = LOCK_EX;
if (lockType == ltRead) return LOCK_SH;
else if (lockType == ltWrite) return LOCK_EX;
else abort();
}
if (wait) {
while (flock(fd, type) != 0) {
checkInterrupt();
if (errno != EINTR)
throw SysError("acquiring lock");
else
return false;
}
} else {
while (flock(fd, type | LOCK_NB) != 0) {
checkInterrupt();
if (errno == EWOULDBLOCK) return false;
if (errno != EINTR)
throw SysError("acquiring lock");
}
bool lockFile(int fd, LockType lockType)
{
int type = convertLockType(lockType);
while (flock(fd, type) != 0) {
checkInterrupt();
if (errno != EINTR)
throw SysError("acquiring lock");
else
return false;
}
return true;
}
bool tryLockFile(int fd, LockType lockType)
{
int type = convertLockType(lockType);
while (flock(fd, type | LOCK_NB) != 0) {
checkInterrupt();
if (errno == EWOULDBLOCK) return false;
if (errno != EINTR)
throw SysError("acquiring lock");
}
return true;
@@ -111,10 +119,10 @@ bool PathLocks::lockPaths(const PathSet & paths,
fd = openLockFile(lockPath, true);
/* Acquire an exclusive lock. */
if (!lockFile(fd.get(), ltWrite, false)) {
if (!tryLockFile(fd.get(), ltWrite)) {
if (wait) {
if (waitMsg != "") printError(waitMsg);
lockFile(fd.get(), ltWrite, true);
lockFile(fd.get(), ltWrite);
} else {
/* Failed to lock this path; release all other
locks. */
@@ -185,12 +193,12 @@ FdLock::FdLock(int fd, LockType lockType, bool wait, std::string_view waitMsg)
: fd(fd)
{
if (wait) {
if (!lockFile(fd, lockType, false)) {
if (!tryLockFile(fd, lockType)) {
printInfo("%s", waitMsg);
acquired = lockFile(fd, lockType, true);
acquired = lockFile(fd, lockType);
}
} else
acquired = lockFile(fd, lockType, false);
acquired = tryLockFile(fd, lockType);
}
+2 -1
View File
@@ -20,7 +20,8 @@ void deleteLockFile(const Path & path, int fd);
enum LockType { ltRead, ltWrite };
bool lockFile(int fd, LockType lockType, bool wait);
bool lockFile(int fd, LockType lockType);
bool tryLockFile(int fd, LockType lockType);
void unlockFile(int fd);
class PathLocks