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
parent cf5df9bc4a
commit 6e34a4d30f

View file

@ -792,17 +792,31 @@ StorePathSet Store::queryValidPaths(const StorePathSet & paths, SubstituteFlag m
auto doQuery = [&](const StorePath & path) { auto doQuery = [&](const StorePath & path) {
checkInterrupt(); checkInterrupt();
auto state(state_.lock());
bool exists = false;
std::optional<std::exception_ptr> newExc;
try { try {
auto info = queryPathInfo(path); queryPathInfo(path);
state->valid.insert(path); exists = true;
} catch (InvalidPath &) { } catch (InvalidPath &) {
} catch (...) { } catch (...) {
state->exc = std::current_exception(); newExc = std::current_exception();
}
{
auto state(state_.lock());
if (exists) {
state->valid.insert(path);
}
if (newExc.has_value()) {
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) for (auto & path : paths)