From b3b498e071b605f1205a209dd1bc28bc1a64ef8a Mon Sep 17 00:00:00 2001 From: Jacob Michels Date: Sat, 5 Sep 2026 20:02:25 -0400 Subject: [PATCH] libexpr: wait for pending derivations before checking validity --- src/libexpr-tests/eval.cc | 109 ++++++++++++++++++++++++++++++++++++++ src/libexpr/eval-cache.cc | 3 ++ 2 files changed, 112 insertions(+) diff --git a/src/libexpr-tests/eval.cc b/src/libexpr-tests/eval.cc index 7562a9da21..bc6f1af800 100644 --- a/src/libexpr-tests/eval.cc +++ b/src/libexpr-tests/eval.cc @@ -3,6 +3,10 @@ #include "nix/expr/eval.hh" #include "nix/expr/tests/libexpr.hh" +#include "nix/expr/eval-cache.hh" +#include "nix/store/async-path-writer.hh" +#include "nix/store/local-store.hh" +#include "nix/util/file-system.hh" #include "nix/util/memory-source-accessor.hh" namespace nix { @@ -212,4 +216,109 @@ TEST_F(PureEvalTest, pathExists) } } +namespace { + +struct DeferredPathWriter : AsyncPathWriter +{ + std::function write; + + StorePath addPath(std::string, std::string, StorePathSet, RepairFlag, std::shared_ptr) override + { + throw Error("unexpected addPath"); + } + + void waitForPath(const StorePath &) override + { + waitForAllPaths(); + } + + void waitForAllPaths() override + { + if (auto pending = std::exchange(write, {})) + pending(); + } + + bool wasAdded(const StorePath &) override + { + return false; + } +}; + +struct RacingPathInfoStore : LocalStore +{ + RacingPathInfoStore(ref config) + : Store(*config) + , LocalFSStore(*config) + , LocalStore(config) + { + } + + std::function finishWrite; + + bool isValidPathUncached(const StorePath & path) override + { + // TrackingStore uses the base implementation, which caches missing paths. + return Store::isValidPathUncached(path); + } + + void queryPathInfoUncached( + const StorePath & path, Callback> callback) noexcept override + { + LocalStore::queryPathInfoUncached(path, {[this, &callback](auto result) { + try { + auto info = result.get(); + // Complete the write after the lookup, but before its missing + // result enters the cache. No threads or sleeps are needed. + if (!info) + if (auto pending = std::exchange(finishWrite, {})) + pending(); + callback(std::move(info)); + } catch (...) { + callback.rethrow(); + } + }}); + } +}; + +} // namespace + +TEST_F(EvalStateTest, forceDerivationWaitsForPendingWrite) +{ + auto tmpDir = createTempDir(); + AutoDelete cleanup(tmpDir); + auto config = make_ref(tmpDir, StoreConfig::Params{}); + auto racingStore = make_ref(config); + auto writer = make_ref(); + EvalState evalState({}, racingStore, fetchSettings, evalSettings, nullptr); + evalState.asyncPathWriter = writer; + + const std::string contents = + R"(Derive([("out","/nix/store/0ngv9b0ck67hr29zy0zak64s3n77pncq-pending","","")],[],[],"dummy","/bin/sh",[],[]))"; + auto path = racingStore->makeFixedOutputPathFromCA( + "pending.drv", TextInfo{.hash = hashString(HashAlgorithm::SHA256, contents), .references = {}}); + writer->write = [&] { + StringSource source(contents); + racingStore->addToStoreFromDump( + source, + "pending.drv", + FileSerialisationMethod::Flat, + ContentAddressMethod::Raw::Text, + HashAlgorithm::SHA256, + {}, + NoRepair, + nullptr); + }; + racingStore->finishWrite = [&] { writer->waitForPath(path); }; + + auto value = evalState.allocValue(); + evalState.eval( + evalState.parseExprFromString( + fmt("{ drvPath = \"%s\"; }", racingStore->printStorePath(path)), evalState.rootPath(CanonPath::root)), + *value); + auto cache = make_ref(std::nullopt, evalState, [value] { return value; }); + + EXPECT_EQ(cache->getRoot()->forceDerivation(), path); + EXPECT_TRUE(racingStore->isValidPath(path)); +} + } // namespace nix diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index b775acf5e9..f0c5953d33 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -779,6 +779,9 @@ StorePath AttrCursor::forceDerivation() auto aDrvPath = getAttr(root->state.s.drvPath); auto drvPath = root->state.store->parseStorePath(aDrvPath->getString()); drvPath.requireDerivation(); + // Evaluation may have queued this derivation. Do not let a concurrent + // validity lookup cache a missing path after the writer invalidates it. + root->state.waitForPath(drvPath); if (!root->state.store->isValidPath(drvPath) && !settings.readOnlyMode) { /* The eval cache contains 'drvPath', but the actual path has been garbage-collected. So force it to be regenerated. */