From bf9eab65159bbcac9bfa357ed9b6f49861ee5a26 Mon Sep 17 00:00:00 2001 From: Martin Jesper Low Madsen Date: Mon, 4 May 2026 21:53:16 -0500 Subject: [PATCH] libexpr: add temp roots during evaluation to prevent GC races Evaluation resolves store paths (reads .drv files, computes closures, extracts output paths) without holding the GC lock or registering temp roots. This creates a race window where GC can delete .drv files between evaluation resolving them and the build phase adding its own temp roots, resulting in "No such file or directory" errors. Add addTempRoot() calls at the key points where libexpr reads derivations from the store: - import: before validating and reading an imported .drv file - derivationStrict: before computing the FS closure and reading input derivations during derivation instantiation - mkSingleDerivedPathStringRaw: before reading a derivation to resolve a built path string - PackageInfo constructor: before reading a derivation for package metadata queries This closes the window between evaluation and build where concurrent GC (whether from nix-collect-garbage, nh clean, or the daemon's own min-free auto-GC) could delete paths that evaluation has resolved but not yet protected. --- src/libexpr/eval.cc | 1 + src/libexpr/get-drvs.cc | 3 +++ src/libexpr/primops.cc | 9 +++++++++ 3 files changed, 13 insertions(+) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 7d95f0693d4d..c26a7eb6d092 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1041,6 +1041,7 @@ std::string EvalState::mkSingleDerivedPathStringRaw(const SingleDerivedPath & p) auto optStaticOutputPath = std::visit( overloaded{ [&](const SingleDerivedPath::Opaque & o) { + store->addTempRoot(o.path); waitForPath(o.path); auto drv = store->readDerivation(o.path); auto i = drv.outputs.find(b.output); diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc index 03a1aa455ce0..8ae89fd2d565 100644 --- a/src/libexpr/get-drvs.cc +++ b/src/libexpr/get-drvs.cc @@ -25,6 +25,9 @@ PackageInfo::PackageInfo(EvalState & state, ref store, const std::string this->drvPath = drvPath; + /* Prevent GC from deleting the .drv between now and when the + build phase adds its own temp root. */ + store->addTempRoot(drvPath); auto drv = store->derivationFromPath(drvPath); name = drvPath.name(); diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index a997c96c760f..c7e5b143b2e4 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -309,6 +309,10 @@ static void import(EvalState & state, const PosIdx pos, Value & vPath, Value * v if (!state.store->isStorePath(path2)) return std::nullopt; auto storePath = state.store->parseStorePath(path2); + /* Add a temp root before checking validity or reading the + derivation, to prevent GC from deleting the path between + our validity check and the subsequent readDerivation(). */ + state.store->addTempRoot(storePath); state.waitForPath(storePath); if (!(state.store->isValidPath(storePath) && isDerivation(path2))) return std::nullopt; @@ -1760,9 +1764,14 @@ static void derivationStrictInternal( /* !!! This doesn't work if readOnlyMode is set. */ StorePathSet refs; // FIXME: don't need to wait, we only need the references. + state.store->addTempRoot(d.drvPath); state.waitForPath(d.drvPath); state.store->computeFSClosure(d.drvPath, refs); for (auto & j : refs) { + /* Prevent GC from deleting derivations in the + closure before we can read them. */ + if (j.isDerivation()) + state.store->addTempRoot(j); drv.inputSrcs.insert(j); if (j.isDerivation()) { drv.inputDrvs.map[j].value = state.store->readDerivation(j).outputNames();