libexpr: add temp roots during evaluation to prevent GC races#446
libexpr: add temp roots during evaluation to prevent GC races#446martinjlowm wants to merge 1 commit into
Conversation
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.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Upstream NixOS#15795 - we mainly use Determinate hence the "forward"-port
Motivation
Evaluation resolves store paths (reads
.drvfiles, computes closures, extracts output paths) without holding the GC lock or registering temp roots. This creates a race window where GC can delete.drvfiles 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 wherelibexprreads derivations from the store:import: before validating and reading an imported .drv filederivationStrict: before computing the FS closure and reading input derivations during derivation instantiationmkSingleDerivedPathStringRaw: before reading a derivation to resolve a built path stringPackageInfoconstructor: before reading a derivation for package metadata queriesThis 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.
Context
We're experiencing concurrent GC passes that regularly wipe
.drv's off of disk when a concurrent process is halted during the eval phase, this leads to the following when the process gets to proceed:This complements NixOS#15469
This happens for us on: nix (Nix) 2.28.3
Now, this is my first contribution to Nix itself, so I'm not aware of any potential performance penalties in extending the set of temproots at eval-time.