Move creation of the temp roots file into its own function

This also moves the file handle into its own Sync object so we're not
holding the _state while acquiring the file lock. There was no real
deadlock risk here since locking a newly created file cannot block,
but it's still a bit nicer.
This commit is contained in:
Eelco Dolstra 2023-01-03 14:51:23 +01:00
parent 15341334b5
commit 224b56f10e
3 changed files with 41 additions and 29 deletions

View file

@ -77,12 +77,12 @@ Path LocalFSStore::addPermRoot(const StorePath & storePath, const Path & _gcRoot
} }
void LocalStore::addTempRoot(const StorePath & path) void LocalStore::createTempRootsFile()
{ {
auto state(_state.lock()); auto fdTempRoots(_fdTempRoots.lock());
/* Create the temporary roots file for this process. */ /* Create the temporary roots file for this process. */
if (!state->fdTempRoots) { if (*fdTempRoots) return;
while (1) { while (1) {
if (pathExists(fnTempRoots)) if (pathExists(fnTempRoots))
@ -90,24 +90,30 @@ void LocalStore::addTempRoot(const StorePath & path)
processes with the same pid. */ processes with the same pid. */
unlink(fnTempRoots.c_str()); unlink(fnTempRoots.c_str());
state->fdTempRoots = openLockFile(fnTempRoots, true); *fdTempRoots = openLockFile(fnTempRoots, true);
debug("acquiring write lock on '%s'", fnTempRoots); debug("acquiring write lock on '%s'", fnTempRoots);
lockFile(state->fdTempRoots.get(), ltWrite, true); lockFile(fdTempRoots->get(), ltWrite, true);
/* Check whether the garbage collector didn't get in our /* Check whether the garbage collector didn't get in our
way. */ way. */
struct stat st; struct stat st;
if (fstat(state->fdTempRoots.get(), &st) == -1) if (fstat(fdTempRoots->get(), &st) == -1)
throw SysError("statting '%1%'", fnTempRoots); throw SysError("statting '%1%'", fnTempRoots);
if (st.st_size == 0) break; if (st.st_size == 0) break;
/* The garbage collector deleted this file before we could /* The garbage collector deleted this file before we could get
get a lock. (It won't delete the file after we get a a lock. (It won't delete the file after we get a lock.)
lock.) Try again. */ Try again. */
}
} }
}
void LocalStore::addTempRoot(const StorePath & path)
{
createTempRootsFile();
auto state(_state.lock());
if (!state->fdGCLock) if (!state->fdGCLock)
state->fdGCLock = openGCLock(); state->fdGCLock = openGCLock();
@ -162,7 +168,7 @@ void LocalStore::addTempRoot(const StorePath & path)
/* Append the store path to the temporary roots file. */ /* Append the store path to the temporary roots file. */
auto s = printStorePath(path) + '\0'; auto s = printStorePath(path) + '\0';
writeFull(state->fdTempRoots.get(), s); writeFull(_fdTempRoots.lock()->get(), s);
} }

View file

@ -441,9 +441,9 @@ LocalStore::~LocalStore()
} }
try { try {
auto state(_state.lock()); auto fdTempRoots(_fdTempRoots.lock());
if (state->fdTempRoots) { if (*fdTempRoots) {
state->fdTempRoots = -1; *fdTempRoots = -1;
unlink(fnTempRoots.c_str()); unlink(fnTempRoots.c_str());
} }
} catch (...) { } catch (...) {

View file

@ -62,9 +62,6 @@ private:
/* The global GC lock */ /* The global GC lock */
AutoCloseFD fdGCLock; AutoCloseFD fdGCLock;
/* The file to which we write our temporary roots. */
AutoCloseFD fdTempRoots;
/* Connection to the garbage collector. */ /* Connection to the garbage collector. */
AutoCloseFD fdRootsSocket; AutoCloseFD fdRootsSocket;
@ -156,6 +153,15 @@ public:
void addTempRoot(const StorePath & path) override; void addTempRoot(const StorePath & path) override;
private:
void createTempRootsFile();
/* The file to which we write our temporary roots. */
Sync<AutoCloseFD> _fdTempRoots;
public:
void addIndirectRoot(const Path & path) override; void addIndirectRoot(const Path & path) override;
private: private: