Fix for unknown serve command 13
#3041
Conversation
zimbatm
force-pushed
the
nix-store-error-13
branch
from
ae76b63
to
5fc5514
Compare
| @@ -950,8 +950,12 @@ static void opServe(Strings opFlags, Strings opArgs) | |||
| info.sigs = readStrings<StringSet>(in); | |||
| in >> info.ca; | |||
|
|
|||
| // FIXME: race if addToStore doesn't read source? | |||
| store->addToStore(info, in, NoRepair, NoCheckSigs); | |||
| SizedSource sizedSource(in, info.narSize); | |||
There was a problem hiding this comment.
I don't really understand what SizedSource is for, in conjunction with drainAll(). Shouldn't drainAll just read and discard remain bytes?
There was a problem hiding this comment.
There should be a check for info.narSize == 0, which can happen in some legacy situations. It's probably fine to just throw an exception.
There was a problem hiding this comment.
The SizedSource is a way to track the remain bytes inside of addToStore() since we don't know how much it will consume. It also acts as a guard that addToStore() won't read more than info.narSize on the socket (not that this has been a problem).
There was a problem hiding this comment.
That's what I thought but I assumed the point is so that you can then skip remain bytes at the end (rather than drain the FD, which introduces problems when the client is pipelining requests or hasn't written everything yet).
zimbatm
force-pushed
the
nix-store-error-13
branch
from
f21662b
to
4b0b5ba
Compare
|
hitting issues with |
zimbatm
force-pushed
the
nix-store-error-13
branch
6 times, most recently
from
df5b6a5
to
fa9550f
Compare
Introduce the SizeSource which allows to bound how much data is being read from a source. It also contains a drainAll() function to discard the rest of the source, useful to keep the nix protocol in sync.
If a NAR is already in the store, addToStore doesn't read the source which makes the protocol go out of sync. This happens for example when two client try to nix-copy-closure the same derivation at the same time.
zimbatm
force-pushed
the
nix-store-error-13
branch
from
fa9550f
to
b226b5c
Compare
|
I removed the nix-daemon patch because it had unexpected side-effects. The PR is now ready to be merged and has been tested manually. |
Fixes #3039
libstore->addToStore(ValidPathInfo & info, Source & source, ...)can exit early if the path already exists into the store. It's an optimization. The source doesn't need to be read and is left untouched.Unfortunately, the remote store protocols assume that the while NAR file will be send. So when that happens the protocol gets out of sync.
This PR introduces a
SizedSource(Source & source, size_t size)which reads thesourceuntilsizeis reached. That way we can wrap the source and make sure it's read untilnarSizeis reached. And then nix-store and nix-daemon are patched to use that new source.If the NAR file is not consumed by
addToStorethe protocol gets out of sync. This introduces a new SizedReader that we can drain at the end in order to keep the protocol in sync. It means that