From 9c1db3cd8b648899c14f486ac720f76cfb62f97a Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Mon, 3 Feb 2025 21:56:26 +0100 Subject: [PATCH] libstore: extract unlockFile from lockFile propagating the "unlock" lock type through this high-level api is nonsense. it doesn't make sense to treat locking and unlocking as similar operations; unlocking *must* not not interruptible by our checkInterrupt machinery or it will just leave locks lying around for a potentially very long time. unlock operations should not be taking long enough to *want* them interrupted anyway. even on nfs this makes very little sense because nfs waits *uninterruptibly*. Change-Id: I10d605c8fe6c651bee64466eee1f8e20251d39f4 (cherry picked from commit aa87c8aa93d48c4610edcc2b81ac28174f4b1e37) --- lix/libstore/local-store.cc | 4 ++-- lix/libstore/pathlocks.cc | 14 +++++++++++--- lix/libstore/pathlocks.hh | 5 +++-- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/lix/libstore/local-store.cc b/lix/libstore/local-store.cc index 9f2828920..3e9f9a0d3 100644 --- a/lix/libstore/local-store.cc +++ b/lix/libstore/local-store.cc @@ -101,7 +101,7 @@ void migrateCASchema(SQLite& db, Path schemaPath, AutoCloseFD& lockFd) if (!lockFile(lockFd.get(), ltWrite, false)) { printInfo("waiting for exclusive access to the Nix store for ca drvs..."); - lockFile(lockFd.get(), ltNone, false); // We have acquired a shared lock; release it to prevent deadlocks + unlockFile(lockFd.get()); // We have acquired a shared lock; release it to prevent deadlocks lockFile(lockFd.get(), ltWrite, true); } @@ -321,7 +321,7 @@ LocalStore::LocalStore(LocalStoreConfig config) if (!lockFile(globalLock.get(), ltWrite, false)) { printInfo("waiting for exclusive access to the Nix store..."); - lockFile(globalLock.get(), ltNone, false); // We have acquired a shared lock; release it to prevent deadlocks + unlockFile(globalLock.get()); // We have acquired a shared lock; release it to prevent deadlocks lockFile(globalLock.get(), ltWrite, true); } diff --git a/lix/libstore/pathlocks.cc b/lix/libstore/pathlocks.cc index 8c990ef4a..38d179f45 100644 --- a/lix/libstore/pathlocks.cc +++ b/lix/libstore/pathlocks.cc @@ -41,14 +41,13 @@ bool lockFile(int fd, LockType lockType, bool wait) int type; if (lockType == ltRead) type = LOCK_SH; else if (lockType == ltWrite) type = LOCK_EX; - else if (lockType == ltNone) type = LOCK_UN; else abort(); if (wait) { while (flock(fd, type) != 0) { checkInterrupt(); if (errno != EINTR) - throw SysError("acquiring/releasing lock"); + throw SysError("acquiring lock"); else return false; } @@ -57,13 +56,22 @@ bool lockFile(int fd, LockType lockType, bool wait) checkInterrupt(); if (errno == EWOULDBLOCK) return false; if (errno != EINTR) - throw SysError("acquiring/releasing lock"); + throw SysError("acquiring lock"); } } return true; } +void unlockFile(int fd) +{ + while (flock(fd, LOCK_UN) != 0) { + if (errno != EINTR) { + throw SysError("releasing lock"); + } + } +} + PathLocks::PathLocks() : deletePaths(false) diff --git a/lix/libstore/pathlocks.hh b/lix/libstore/pathlocks.hh index 3f46c873a..3309cd687 100644 --- a/lix/libstore/pathlocks.hh +++ b/lix/libstore/pathlocks.hh @@ -18,9 +18,10 @@ AutoCloseFD openLockFile(const Path & path, bool create); */ void deleteLockFile(const Path & path, int fd); -enum LockType { ltRead, ltWrite, ltNone }; +enum LockType { ltRead, ltWrite }; bool lockFile(int fd, LockType lockType, bool wait); +void unlockFile(int fd); class PathLocks { @@ -52,7 +53,7 @@ struct FdLock { try { if (acquired) - lockFile(fd, ltNone, false); + unlockFile(fd); } catch (SysError &) { ignoreExceptionInDestructor(); }