From 66a9fbb7ffa781f46e246de08700739fa775650c Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Sat, 15 Jun 2024 13:10:03 -0700 Subject: 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 --- src/libstore/store-api.cc | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) (limited to 'src/libstore/store-api.cc') diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 55083e834..4e1fb4bd5 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -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) -- cgit v1.2.3