Fix crash when NAR is missing from binary cache #9804
Conversation
In the "discard" case (i.e. when the store path already exists
locally), when we call parseDump() from a Finally and it throws an
exception (e.g. if the download of the NAR fails), Nix crashes:
terminate called after throwing an instance of 'nix::SubstituteGone'
what(): error: file 'nar/06br3254rx4gz4cvjzxlv028jrx80zg5i4jr62vjmn416dqihgr7.nar.xz' does not exist in binary cache 'http://localhost'
Aborted (core dumped)
In rare cases (e.g. when using allowSubstitutes = false), it's possible that we simultaneously have a DerivationGoal *and* a SubstitutionGoal building the same path. So if a DerivationGoal already built the path while the SubstitutionGoal was waiting for a download slot, it saves us a superfluous download to exit early.
edolstra
added
the
backport 2.19-maintenance
github-actions
bot
added
the
store
| parseDump(sink, source); | ||
| try { | ||
| parseDump(sink, source); | ||
| } catch (...) { |
There was a problem hiding this comment.
| } catch (...) { | |
| } catch (SubstituteGone &) { |
I don't think we want to swallow any kind of exception, because swallowing genuine NAR parse errors might leave the source in a partially consumed state that'll cause nonsense error down the line
There was a problem hiding this comment.
We cannot make assumptions here about what kind of exception the source might throw. SubstituteGone is just one example, I/O errors are another.
NAR parse errors would already have left the source in an unknown state. The only place where that matters is the daemon connection, and we have FramedSource now to take care of that.
|
BTW, perhaps we should add the |
|
Successfully created backport PR for |
Fix crash when NAR is missing from binary cache (cherry picked from commit 3b20cca) Change-Id: I50ff18f4a6de69c323473b4a8e3e098d1f365145
Motivation
This fixes random crashes like
and
observed with
magic-nix-cachewhen NARs (but not the corresponding .narinfos) had been pruned from the GHA cache.Nix already handles missing NARs via the
SubstituteGoneexception. However, when usingallowSubstitutes = false, it is possible to end up in a situation where Nix simultaneously has aDerivationGoaland aSubstitutionGoalfor the same path. If the path is built first, Nix will then substitute it, causingLocalStore::addToStore()to discard the incoming NAR from aFinallyhandler because the path is already valid. This causes theSubstituteGoneexception to be thrown from a destructor, which crashes Nix.Context
Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.