diff --git a/lix/legacy/build-remote.cc b/lix/legacy/build-remote.cc index f5e64d4da..e7cd2d741 100644 --- a/lix/legacy/build-remote.cc +++ b/lix/legacy/build-remote.cc @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -23,9 +24,6 @@ namespace nix { -static void handleAlarm(int sig) { -} - std::string escapeUri(std::string uri) { std::replace(uri.begin(), uri.end(), '/', '_'); @@ -279,12 +277,8 @@ connected: { Activity act(*logger, lvlTalkative, actUnknown, fmt("waiting for the upload lock to '%s'", storeUri)); - auto old = signal(SIGALRM, handleAlarm); - alarm(15 * 60); - if (!lockFile(uploadLock.get(), ltWrite)) + if (!unsafeLockFileSingleThreaded(uploadLock.get(), ltWrite, std::chrono::minutes(15))) printError("somebody is hogging the upload lock for '%s', continuing..."); - alarm(0); - signal(SIGALRM, old); } auto substitute = settings.buildersUseSubstitutes ? Substitute : NoSubstitute; diff --git a/lix/libstore/gc.cc b/lix/libstore/gc.cc index f084b83c1..9a8fc53c6 100644 --- a/lix/libstore/gc.cc +++ b/lix/libstore/gc.cc @@ -129,7 +129,7 @@ void LocalStore::addTempRoot(const StorePath & path) /* Try to acquire a shared global GC lock (non-blocking). This only succeeds if the garbage collector is not currently running. */ - FdLock gcLock(_fdGCLock.lock()->get(), ltRead, false, ""); + FdLock gcLock(_fdGCLock.lock()->get(), ltRead); if (!gcLock.acquired) { /* We couldn't get a shared global GC lock, so the garbage @@ -583,7 +583,7 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results) here because then in auto-gc mode, another thread could downgrade our exclusive lock. */ auto fdGCLock = openGCLock(); - FdLock gcLock(fdGCLock.get(), ltWrite, true, "waiting for the big garbage collector lock..."); + FdLock gcLock(fdGCLock.get(), ltWrite, "waiting for the big garbage collector lock..."); /* Synchronisation point to test ENOENT handling in addTempRoot(), see tests/gc-non-blocking.sh. */ diff --git a/lix/libstore/local-store.cc b/lix/libstore/local-store.cc index f38af19eb..c88a79305 100644 --- a/lix/libstore/local-store.cc +++ b/lix/libstore/local-store.cc @@ -1489,7 +1489,8 @@ std::pair LocalStore::createTempDirInStore() if (tmpDirFd.get() < 0) { continue; } - lockedByUs = lockFile(tmpDirFd.get(), ltWrite); + lockFile(tmpDirFd.get(), ltWrite); + lockedByUs = true; } while (!pathExists(tmpDirFn) || !lockedByUs); return {tmpDirFn, std::move(tmpDirFd)}; } @@ -1525,7 +1526,7 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair) /* Acquire the global GC lock to get a consistent snapshot of existing and valid paths. */ auto fdGCLock = openGCLock(); - FdLock gcLock(fdGCLock.get(), ltRead, true, "waiting for the big garbage collector lock..."); + FdLock gcLock(fdGCLock.get(), ltRead, "waiting for the big garbage collector lock..."); StorePathSet validPaths; diff --git a/lix/libstore/pathlocks.cc b/lix/libstore/pathlocks.cc index 257155ebf..c5757cd8e 100644 --- a/lix/libstore/pathlocks.cc +++ b/lix/libstore/pathlocks.cc @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -42,10 +43,28 @@ static int convertLockType(LockType lockType) else abort(); } -bool lockFile(int fd, LockType lockType) +void lockFile(int fd, LockType lockType) { int type = convertLockType(lockType); + while (flock(fd, type) != 0) { + checkInterrupt(); + if (errno != EINTR) + throw SysError("acquiring lock"); + } +} + +bool unsafeLockFileSingleThreaded(int fd, LockType lockType, std::chrono::seconds timeout) +{ + int type = convertLockType(lockType); + + auto old = signal(SIGALRM, [](int) {}); + alarm(timeout.count()); + KJ_DEFER({ + alarm(0); + signal(SIGALRM, old); + }); + while (flock(fd, type) != 0) { checkInterrupt(); if (errno != EINTR) @@ -189,16 +208,20 @@ void PathLocks::setDeletion(bool deletePaths) } -FdLock::FdLock(int fd, LockType lockType, bool wait, std::string_view waitMsg) +FdLock::FdLock(int fd, LockType lockType) : fd(fd) { - if (wait) { - if (!tryLockFile(fd, lockType)) { - printInfo("%s", waitMsg); - acquired = lockFile(fd, lockType); - } - } else - acquired = tryLockFile(fd, lockType); + acquired = tryLockFile(fd, lockType); +} + +FdLock::FdLock(int fd, LockType lockType, std::string_view waitMsg) + : fd(fd) +{ + if (!tryLockFile(fd, lockType)) { + printInfo("%s", waitMsg); + lockFile(fd, lockType); + acquired = true; + } } diff --git a/lix/libstore/pathlocks.hh b/lix/libstore/pathlocks.hh index d4b3ef7a2..e8bff3401 100644 --- a/lix/libstore/pathlocks.hh +++ b/lix/libstore/pathlocks.hh @@ -3,6 +3,7 @@ #include "lix/libutil/error.hh" #include "lix/libutil/file-descriptor.hh" +#include namespace nix { @@ -20,7 +21,14 @@ void deleteLockFile(const Path & path, int fd); enum LockType { ltRead, ltWrite }; -bool lockFile(int fd, LockType lockType); +void lockFile(int fd, LockType lockType); +/** + * Same as `lockFile`, but with a timeout. This timeout uses the POSIX `alarm` + * facility and a `SIGALRM` handler. Using this function from multiple threads + * in the same process is not safe: all `SIGALRM` handlers set previously will + * be overwritten while this function is executing and are restored on return. + */ +bool unsafeLockFileSingleThreaded(int fd, LockType lockType, std::chrono::seconds timeout); bool tryLockFile(int fd, LockType lockType); void unlockFile(int fd); @@ -48,7 +56,8 @@ struct FdLock int fd; bool acquired = false; - FdLock(int fd, LockType lockType, bool wait, std::string_view waitMsg); + FdLock(int fd, LockType lockType); + FdLock(int fd, LockType lockType, std::string_view waitMsg); ~FdLock() {