guiwf/lix
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

libstore: rework path locking infra entirely

Browse Source 
we now allow for single locked paths rather than only sets, the lock
holder classes are finally resource-safe, and not locking files from
within constructors means we can (in theory!) make path locks async.

Change-Id: I1e1807299d370c07b15c332d5b2ff77b64456e7d
This commit is contained in:
eldritch horrors
2025-02-08 13:39:49 +01:00
parent 72326c4044
commit ec44c8bd6c
11 changed files with 120 additions and 112 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lix/legacy/nix-env.cc
+1 -2
View File
@@ -1359,8 +1359,7 @@ static void opListGenerations(Globals & globals, Strings opFlags, Strings opArgs
if (opArgs.size() != 0)
throw UsageError("no arguments expected");
PathLocks lock;
lockProfile(lock, globals.profile);
PathLock lock = lockProfile(globals.profile);
auto [gens, curGen] = findGenerations(globals.profile);
lix/legacy/user-env.cc
+1 -2
View File
@@ -133,8 +133,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
auto store2 = state.ctx.store.dynamic_pointer_cast<LocalFSStore>();
if (store2) {
PathLocks lock;
lockProfile(lock, profile);
PathLock lock = lockProfile(profile);
Path lockTokenCur = optimisticLockProfile(profile);
if (lockToken != lockTokenCur) {
lix/libfetchers/git.cc
+1 -1
View File
@@ -558,7 +558,7 @@ struct GitInputScheme : InputScheme
gitDir = ".";
createDirs(dirOf(cacheDir));
PathLocks cacheDirLock({cacheDir + ".lock"});
PathLock cacheDirLock = lockPath(cacheDir + ".lock");
if (!pathExists(cacheDir)) {
runProgram("git", true, { "-c", "init.defaultBranch=" + gitInitialBranch, "init", "--bare", repoDir });
lix/libstore/build/derivation-goal.cc
+6 -5
View File
@@ -734,7 +734,8 @@ retry:
}
}
if (!outputLocks.tryLockPaths(lockFiles)) {
outputLocks = tryLockPaths(lockFiles);
if (!outputLocks) {
if (!actLock)
actLock = std::make_unique<Activity>(*logger, lvlWarn, actBuildWaiting,
fmt("waiting for lock on %s", Magenta(showPaths(lockFiles))));
@@ -807,7 +808,7 @@ retry:
if (!actLock)
actLock = std::make_unique<Activity>(*logger, lvlTalkative, actBuildWaiting,
fmt("waiting for a machine to build '%s'", Magenta(worker.store.printStorePath(drvPath))));
outputLocks.unlock();
outputLocks.reset();
co_await waitForAWhile();
goto retry;
}
@@ -1086,11 +1087,11 @@ try {
lockers will see that the output paths are valid; they will
not create new lock files with the same names as the old
(unlinked) lock files. */
outputLocks.unlock();
outputLocks.reset();
co_return done(BuildResult::Built, std::move(builtOutputs));
} catch (BuildError & e) {
outputLocks.unlock();
outputLocks.reset();
BuildResult::Status st = BuildResult::MiscFailure;
@@ -1712,7 +1713,7 @@ Goal::WorkResult DerivationGoal::done(
{
isDone = true;
outputLocks.unlock();
outputLocks.reset();
buildResult.status = status;
if (ex)
buildResult.errorMsg = fmt("%s", Uncolored(ex->info().msg));
lix/libstore/build/derivation-goal.hh
+1 -1
View File
@@ -174,7 +174,7 @@ struct DerivationGoal : public Goal
/**
* Locks on (fixed) output paths.
*/
PathLocks outputLocks;
std::optional<PathLocks> outputLocks;
/**
* All input paths (that is, the union of FS closures of the
lix/libstore/build/local-derivation-goal.cc
+4 -4
View File
@@ -160,7 +160,7 @@ retry:
#endif
if (!slotToken.valid()) {
outputLocks.unlock();
outputLocks.reset();
if (worker.localBuilds.capacity() > 0) {
slotToken = co_await worker.localBuilds.acquire();
co_return co_await tryToBuild();
@@ -262,7 +262,7 @@ retry:
}
} catch (BuildError & e) {
outputLocks.unlock();
outputLocks.reset();
buildUser.reset();
auto report = done(BuildResult::InputRejected, {}, std::move(e));
report.permanentFailure = true;
@@ -2295,13 +2295,13 @@ try {
/* Lock final output path, if not already locked. This happens with
floating CA derivations and hash-mismatching fixed-output
derivations. */
PathLocks dynamicOutputLock;
std::optional<PathLock> dynamicOutputLock;
auto optFixedPath = output->path(worker.store, drv->name, outputName);
if (!optFixedPath ||
worker.store.printStorePath(*optFixedPath) != finalDestPath)
{
assert(newInfo.ca);
dynamicOutputLock.lockPaths({worker.store.toRealPath(finalDestPath)});
dynamicOutputLock = lockPath(worker.store.toRealPath(finalDestPath));
}
/* Move files, if needed */
lix/libstore/local-store.cc
+4 -4
View File
@@ -1216,7 +1216,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
if (repair || !isValidPath(info.path)) {
PathLocks outputLock;
std::optional<PathLock> outputLock;
auto realPath = Store::toRealPath(info.path);
@@ -1224,7 +1224,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
from a build hook (whose parent process already acquired a
lock on this path). */
if (!locksHeld.count(printStorePath(info.path)))
outputLock.lockPaths({realPath});
outputLock = lockPath(realPath);
if (repair || !isValidPath(info.path)) {
@@ -1370,7 +1370,7 @@ StorePath LocalStore::addToStoreFromDump(Source & source0, std::string_view name
auto realPath = Store::toRealPath(dstPath);
PathLocks outputLock({realPath});
PathLock outputLock = lockPath(realPath);
if (repair || !isValidPath(dstPath)) {
@@ -1435,7 +1435,7 @@ StorePath LocalStore::addTextToStore(
auto realPath = Store::toRealPath(dstPath);
PathLocks outputLock({realPath});
PathLock outputLock = lockPath(realPath);
if (repair || !isValidPath(dstPath)) {
lix/libstore/pathlocks.cc
+72 -64
View File
@@ -8,6 +8,7 @@
#include <fcntl.h>
#include <kj/common.h>
#include <optional>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/file.h>
@@ -131,80 +132,89 @@ void unlockFile(int fd)
}
PathLocks::PathLocks()
std::optional<PathLock>
PathLock::lockImpl(const Path & path, std::string_view waitMsg, bool wait)
{
Path lockPath = path + ".lock";
debug("locking path '%1%'", path);
while (1) {
/* Open/create the lock file. */
auto fd = openLockFile(lockPath, true);
/* Acquire an exclusive lock. */
if (!tryLockFile(fd.get(), ltWrite)) {
if (wait) {
if (waitMsg != "") printError(std::string(waitMsg));
lockFile(fd.get(), ltWrite);
} else {
return std::nullopt;
}
}
debug("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.get(), &st) == -1)
throw SysError("statting lock file '%1%'", lockPath);
if (st.st_nlink == 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("open lock file '%1%' has become stale", lockPath);
else
return PathLock{std::move(fd), lockPath};
}
}
PathLocks::PathLocks(const PathSet & paths, const std::string & waitMsg)
PathLock lockPath(const Path & path, std::string_view waitMsg)
{
lockPaths(paths, waitMsg);
return std::move(*PathLock::lockImpl(path, waitMsg, true));
}
bool PathLocks::lockPathsImpl(const PathSet & paths,
const std::string & waitMsg, bool wait)
std::optional<PathLock> tryLockPath(const Path & path)
{
assert(fds.empty());
return PathLock::lockImpl(path, "", false);
}
/* Note that `fds' is built incrementally so that the destructor
will only release those locks that we have already acquired. */
static std::optional<PathLocks>
lockPathsImpl(const PathSet & paths, std::string_view waitMsg, bool wait)
{
PathLocks result;
/* Acquire the lock for each path in sorted order. This ensures
that locks are always acquired in the same order, thus
preventing deadlocks. */
for (auto & path : paths) {
checkInterrupt();
Path lockPath = path + ".lock";
debug("locking path '%1%'", path);
AutoCloseFD fd;
while (1) {
/* Open/create the lock file. */
fd = openLockFile(lockPath, true);
/* Acquire an exclusive lock. */
if (!tryLockFile(fd.get(), ltWrite)) {
if (wait) {
if (waitMsg != "") printError(waitMsg);
lockFile(fd.get(), ltWrite);
} else {
/* Failed to lock this path; release all other
locks. */
unlock();
return false;
}
}
debug("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.get(), &st) == -1)
throw SysError("statting lock file '%1%'", lockPath);
if (st.st_nlink == 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("open lock file '%1%' has become stale", lockPath);
else
break;
if (wait) {
result.push_back(lockPath(path, waitMsg));
} else if (auto p = tryLockPath(path); p) {
result.push_back(std::move(*p));
} else {
return std::nullopt;
}
/* Use borrow so that the descriptor isn't closed. */
fds.push_back(FDPair(fd.release(), lockPath));
}
return true;
return result;
}
PathLocks lockPaths(const PathSet & paths, std::string_view waitMsg)
{
return std::move(*lockPathsImpl(paths, waitMsg, true));
}
std::optional<PathLocks> tryLockPaths(const PathSet & paths)
{
return lockPathsImpl(paths, "", false);
}
PathLocks::~PathLocks()
PathLock::~PathLock()
{
try {
unlock();
@@ -214,25 +224,23 @@ PathLocks::~PathLocks()
}
void PathLocks::unlock()
void PathLock::unlock()
{
for (auto & i : fds) {
if (fd) {
// delete the file. if another file descriptor is used to acquire a lock on
// this file it will figure out that the file is stale once it calls stat()
// and inspects the link count. if unlink fails we merely leave around some
// stale lock file paths that can be reused or cleaned up by other threads.
unlink(i.second.c_str());
unlink(path.c_str());
// clobber file contents for compatibility wither other nix implementations
writeFull(i.first, "d");
writeFull(fd.get(), "d");
if (close(i.first) == -1) {
printError("error (ignored): cannot close lock file on '%1%'", i.second);
if (close(fd.release()) == -1) {
printError("error (ignored): cannot close lock file on '%1%'", path);
}
debug("lock released on '%1%'", i.second);
debug("lock released on '%1%'", path);
}
fds.clear();
}
lix/libstore/pathlocks.hh
+22 -16
View File
@@ -31,29 +31,35 @@ bool unsafeLockFileSingleThreaded(int fd, LockType lockType, std::chrono::second
bool tryLockFile(int fd, LockType lockType);
void unlockFile(int fd);
class PathLocks
class PathLock
{
private:
typedef std::pair<int, Path> FDPair;
std::list<FDPair> fds;
friend PathLock lockPath(const Path & path, std::string_view waitMsg);
friend std::optional<PathLock> tryLockPath(const Path & path);
bool lockPathsImpl(const PathSet & _paths, const std::string & waitMsg, bool wait);
AutoCloseFD fd;
Path path;
PathLock(AutoCloseFD fd, const Path & path): fd(std::move(fd)), path(path) {}
static std::optional<PathLock>
lockImpl(const Path & path, std::string_view waitMsg, bool wait);
public:
PathLocks();
PathLocks(const PathSet & paths, const std::string & waitMsg = "");
void lockPaths(const PathSet & _paths, const std::string & waitMsg = "")
{
lockPathsImpl(_paths, waitMsg, true);
}
bool tryLockPaths(const PathSet & _paths)
{
return lockPathsImpl(_paths, "", false);
}
~PathLocks();
PathLock(PathLock &&) = default;
PathLock & operator=(PathLock &&) = default;
~PathLock();
void unlock();
};
PathLock lockPath(const Path & path, std::string_view waitMsg = "");
std::optional<PathLock> tryLockPath(const Path & path);
using PathLocks = std::list<PathLock>;
PathLocks lockPaths(const PathSet & paths, std::string_view waitMsg = "");
std::optional<PathLocks> tryLockPaths(const PathSet & paths);
class FdLock
{
struct Unlocker
lix/libstore/profiles.cc
+7 -12
View File
@@ -143,8 +143,7 @@ static void deleteGeneration2(const Path & profile, GenerationNumber gen, bool d
void deleteGenerations(const Path & profile, const std::set<GenerationNumber> & gensToDelete, bool dryRun)
{
PathLocks lock;
lockProfile(lock, profile);
PathLock lock = lockProfile(profile);
auto [gens, curGen] = findGenerations(profile);
@@ -170,8 +169,7 @@ void deleteGenerationsGreaterThan(const Path & profile, GenerationNumber max, bo
if (max == 0)
throw Error("Must keep at least one generation, otherwise the current one would be deleted");
PathLocks lock;
lockProfile(lock, profile);
PathLock lock = lockProfile(profile);
auto [gens, _curGen] = findGenerations(profile);
auto curGen = _curGen;
@@ -191,8 +189,7 @@ void deleteGenerationsGreaterThan(const Path & profile, GenerationNumber max, bo
void deleteOldGenerations(const Path & profile, bool dryRun)
{
PathLocks lock;
lockProfile(lock, profile);
PathLock lock = lockProfile(profile);
auto [gens, curGen] = findGenerations(profile);
@@ -204,8 +201,7 @@ void deleteOldGenerations(const Path & profile, bool dryRun)
void deleteGenerationsOlderThan(const Path & profile, time_t t, bool dryRun)
{
PathLocks lock;
lockProfile(lock, profile);
PathLock lock = lockProfile(profile);
auto [gens, curGen] = findGenerations(profile);
@@ -265,8 +261,7 @@ void switchGeneration(
std::optional<GenerationNumber> dstGen,
bool dryRun)
{
PathLocks lock;
lockProfile(lock, profile);
PathLock lock = lockProfile(profile);
auto [gens, curGen] = findGenerations(profile);
@@ -291,9 +286,9 @@ void switchGeneration(
}
void lockProfile(PathLocks & lock, const Path & profile)
PathLock lockProfile(const Path & profile)
{
lock.lockPaths({profile}, fmt("waiting for lock on profile '%1%'", profile));
return lockPath(profile, fmt("waiting for lock on profile '%1%'", profile));
}
lix/libstore/profiles.hh
+1 -1
View File
@@ -191,7 +191,7 @@ void switchGeneration(
* Ensure exclusive access to a profile. Any command that modifies
* the profile first acquires this lock.
*/
void lockProfile(PathLocks & lock, const Path & profile);
PathLock lockProfile(const Path & profile);
/**
* Optimistic locking is used by long-running operations like `nix-env