diff --git a/src/libexpr-tests/eval.cc b/src/libexpr-tests/eval.cc index 7562a9da2..3529aee9b 100644 --- a/src/libexpr-tests/eval.cc +++ b/src/libexpr-tests/eval.cc @@ -3,6 +3,11 @@ #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/finally.hh" #include "nix/util/memory-source-accessor.hh" namespace nix { @@ -212,4 +217,127 @@ 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)); +} + +TEST_F(EvalStateTest, forceDerivationReadOnlySkipsPendingWrite) +{ + auto previousReadOnlyMode = std::exchange(settings.readOnlyMode, true); + Finally restoreReadOnlyMode([&] { settings.readOnlyMode = previousReadOnlyMode; }); + auto writer = make_ref(); + state.asyncPathWriter = writer; + writer->write = [] { ADD_FAILURE() << "read-only evaluation waited for a pending write"; }; + + auto value = state.allocValue(); + *value = eval(R"({ drvPath = "/nix/store/0ngv9b0ck67hr29zy0zak64s3n77pncq-pending.drv"; })"); + auto cache = make_ref(std::nullopt, state, [value] { return value; }); + auto path = state.store->parseStorePath("/nix/store/0ngv9b0ck67hr29zy0zak64s3n77pncq-pending.drv"); + + EXPECT_EQ(cache->getRoot()->forceDerivation(), path); + EXPECT_TRUE(writer->write); + writer->write = {}; +} + } // namespace nix diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index b775acf5e..e9fcdb7a5 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -779,14 +779,19 @@ StorePath AttrCursor::forceDerivation() auto aDrvPath = getAttr(root->state.s.drvPath); auto drvPath = root->state.store->parseStorePath(aDrvPath->getString()); drvPath.requireDerivation(); - 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. */ - aDrvPath->forceValue(); + if (!settings.readOnlyMode) { + // 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)) - throw Error( - "don't know how to recreate store derivation '%s'!", root->state.store->printStorePath(drvPath)); + if (!root->state.store->isValidPath(drvPath)) { + /* The eval cache contains 'drvPath', but the actual path has + been garbage-collected. So force it to be regenerated. */ + aDrvPath->forceValue(); + root->state.waitForPath(drvPath); + if (!root->state.store->isValidPath(drvPath)) + throw Error( + "don't know how to recreate store derivation '%s'!", root->state.store->printStorePath(drvPath)); + } } return drvPath; }