From 50d27f997a16c619b5da39ab7909bdf0753ba4c8 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 13 Jul 2026 15:28:56 +0200 Subject: [PATCH] eval-cache: Synchronize lazy initialization of root values AttrCursor::_value and EvalCache::value were lazily initialized without any synchronization, while `nix flake check/show` call into cursors shared between evaluator threads (e.g. the flake outputs cursor used by every Leaf::derivation() call). Two threads could both see an empty RootValue and move-assign it concurrently, double-freeing a root value slot. Since the root value pool uses an intrusive freelist, a double free corrupts the freelist and makes a subsequent allocRootValueSlot() chase a garbage next pointer, crashing with EXC_BAD_ACCESS (e.g. at address 0x28, a Value discriminator word read as a Slot pointer). Wrap both members in Sync so initialization happens exactly once and concurrent callers block until the value is available. The lock is held while the value is computed; lock ordering is strictly child -> parent (and thunk waiting in parallel eval blocks on a condition variable, without work stealing), so this cannot deadlock. Note: AttrCursor::cachedValue has a similar unsynchronized lazy initialization pattern that still needs fixing. Assisted-by: Claude Fable 5 --- src/libexpr/eval-cache.cc | 22 ++++++++++++++-------- src/libexpr/include/nix/expr/eval-cache.hh | 4 ++-- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index c6dd2766c88b..b775acf5e98e 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -359,11 +359,12 @@ EvalCache::EvalCache( Value * EvalCache::getRootValue() { - if (!value) { + auto value(this->value.lock()); + if (!*value) { debug("getting root value"); - value = RootValue(rootLoader()); + *value = RootValue(rootLoader()); } - return *value; + return **value; } ref EvalCache::getRoot() @@ -378,7 +379,7 @@ AttrCursor::AttrCursor( , cachedValue(std::move(cachedValue)) { if (value) - _value = RootValue(value); + *_value.lock() = RootValue(value); } AttrKey AttrCursor::getKey() @@ -394,18 +395,23 @@ AttrKey AttrCursor::getKey() Value & AttrCursor::getValue() { - if (!_value) { + /* Note: this lock is held while the value is being evaluated, + so concurrent calls block until the value is available. Lock + ordering is strictly child -> parent, so this cannot + deadlock. */ + auto value(_value.lock()); + if (!*value) { if (parent) { auto & vParent = parent->first->getValue(); root->state.forceAttrs(vParent, noPos, "while searching for an attribute"); auto attr = vParent.attrs()->get(parent->second); if (!attr) throw Error("attribute '%s' is unexpectedly missing", getAttrPathStr()); - _value = RootValue(attr->value); + *value = RootValue(attr->value); } else - _value = RootValue(root->getRootValue()); + *value = RootValue(root->getRootValue()); } - return **_value; + return ***value; } void AttrCursor::fetchCachedValue() diff --git a/src/libexpr/include/nix/expr/eval-cache.hh b/src/libexpr/include/nix/expr/eval-cache.hh index 3a148988a102..9f1657a57047 100644 --- a/src/libexpr/include/nix/expr/eval-cache.hh +++ b/src/libexpr/include/nix/expr/eval-cache.hh @@ -45,7 +45,7 @@ public: private: typedef fun RootLoader; RootLoader rootLoader; - RootValue value; + Sync value; Value * getRootValue(); @@ -112,7 +112,7 @@ public: private: using Parent = std::optional, Symbol>>; const Parent parent; - RootValue _value; + Sync _value; std::optional> cachedValue; AttrKey getKey();