libstore: fix queryValidPaths concurrency

The lock usage was obviously wrong so it was entirely serialized. This
has the predicted speedups, the only question is whether it is sound
because it's exposing a bunch of new code to actual concurrency.

I did audit all the stores' queryPathInfoUncached implementations and
they all look *intended* to be thread safe, but whether that is actually
sound or not: lol lmao. I am highly confident in the s3 one because it
is calling s3 sdk methods that are thread safe and has no actual state.

Others are using Pool and look to be *supposed* to be thread safe, but
unsure if they actually are.

Change-Id: I0369152a510e878b5ac56c9ac956a98d48cd5fef
This commit is contained in:
jade 2024-06-15 13:10:03 -07:00 committed by jade
parent b9b1bbd22f
commit 66a9fbb7ff

View file

@ -803,17 +803,31 @@ StorePathSet Store::queryValidPaths(const StorePathSet & paths, SubstituteFlag m
auto doQuery = [&](const StorePath & path) {
checkInterrupt();
auto state(state_.lock());
bool exists = false;
std::exception_ptr newExc{};
try {
auto info = queryPathInfo(path);
state->valid.insert(path);
queryPathInfo(path);
exists = true;
} catch (InvalidPath &) {
} catch (...) {
state->exc = std::current_exception();
newExc = std::current_exception();
}
{
auto state(state_.lock());
if (exists) {
state->valid.insert(path);
}
if (newExc != nullptr) {
state->exc = newExc;
}
assert(state->left);
if (!--state->left)
wakeup.notify_one();
}
assert(state->left);
if (!--state->left)
wakeup.notify_one();
};
for (auto & path : paths)