From ee0655240270480d7f6063dcf12ec47f04d2ded6 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Wed, 11 Jun 2025 17:22:33 +0200 Subject: [PATCH] libstore: asyncify RemoteStore::Connection::processStderr we need a wrapper type for the remote exception because our Result type does not deal well with its good type being the same as its error type. we could have also return a `Result>` to fix this, but the wrapper type clarifies via its name where the exception_ptr originates. Change-Id: Ia6ce67b962cb8d6528b017f4cb682a55d6918939 --- lix/libstore/remote-store-connection.hh | 17 +++++++--- lix/libstore/remote-store.cc | 43 ++++++++++++++----------- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/lix/libstore/remote-store-connection.hh b/lix/libstore/remote-store-connection.hh index fa3414d0b..f11303359 100644 --- a/lix/libstore/remote-store-connection.hh +++ b/lix/libstore/remote-store-connection.hh @@ -4,9 +4,11 @@ #include "lix/libstore/remote-store.hh" #include "lix/libstore/worker-protocol.hh" #include "lix/libstore/worker-protocol-impl.hh" +#include "lix/libutil/async.hh" #include "lix/libutil/pool.hh" #include "lix/libutil/result.hh" #include "lix/libutil/serialise.hh" +#include #include #include #include @@ -97,7 +99,14 @@ struct RemoteStore::Connection virtual ~Connection(); - std::exception_ptr processStderr(); + // wrapper type for remote errors because `Result` + // does not work very well and `Result>` is too confusing + struct [[nodiscard]] RemoteError + { + std::exception_ptr e; + }; + + kj::Promise> processStderr(); }; /** @@ -134,7 +143,7 @@ struct RemoteStore::ConnectionHandle RemoteStore::Connection & operator * () { return *handle; } RemoteStore::Connection * operator -> () { return &*handle; } - void processStderr(); + kj::Promise> processStderr(); kj::Promise> withFramedSinkAsync(std::function>(Sink & sink)> fun); @@ -154,7 +163,7 @@ struct RemoteStore::ConnectionHandle if constexpr (requires { handle->to << std::declval(); }) { ((handle->to << std::forward(args)), ...); handle->to.flush(); - processStderr(); + LIX_TRY_AWAIT(processStderr()); } else { using ImmediateArgsIdxs = std::make_index_sequence; AllArgsT allArgs(std::forward(args)...); @@ -180,7 +189,7 @@ private: struct FramedSinkHandler { std::exception_ptr ex; - std::packaged_task stderrHandler; + std::packaged_task stderrHandler; explicit FramedSinkHandler(ConnectionHandle & conn, ThreadPool & handlerThreads); diff --git a/lix/libstore/remote-store.cc b/lix/libstore/remote-store.cc index ecff180f4..86a32e5fd 100644 --- a/lix/libstore/remote-store.cc +++ b/lix/libstore/remote-store.cc @@ -105,8 +105,10 @@ try { conn.daemonNixVersion = readString(conn.from); conn.remoteTrustsUs = WorkerProto::Serialise>::read(conn); - auto ex = conn.processStderr(); - if (ex) std::rethrow_exception(ex); + auto ex = TRY_AWAIT(conn.processStderr()); + if (ex.e) { + std::rethrow_exception(ex.e); + } } catch (Error & e) { throw Error("cannot open connection to remote store '%s': %s", getUri(), e.what()); @@ -157,8 +159,10 @@ try { StringSource{command.s}.drainInto(conn.to); conn.to.flush(); - auto ex = conn.processStderr(); - if (ex) std::rethrow_exception(ex); + auto ex = TRY_AWAIT(conn.processStderr()); + if (ex.e) { + std::rethrow_exception(ex.e); + } co_return result::success(); } catch (...) { co_return result::current_exception(); @@ -172,16 +176,18 @@ RemoteStore::ConnectionHandle::~ConnectionHandle() } } -void RemoteStore::ConnectionHandle::processStderr() -{ - auto ex = handle->processStderr(); - if (ex) { +kj::Promise> RemoteStore::ConnectionHandle::processStderr() +try { + auto ex = TRY_AWAIT(handle->processStderr()); + if (ex.e) { daemonException = true; - std::rethrow_exception(ex); + std::rethrow_exception(ex.e); } + co_return result::success(); +} catch (...) { + co_return result::current_exception(); } - kj::Promise> RemoteStore::getConnection() try { co_return ConnectionHandle(TRY_AWAIT(connections->get()), handlerThreads); @@ -746,15 +752,14 @@ static Logger::Fields readFields(Source & from) return fields; } - -std::exception_ptr RemoteStore::Connection::processStderr() -{ +kj::Promise> RemoteStore::Connection::processStderr() +try { while (true) { auto msg = readNum(from); if (msg == STDERR_ERROR) { - return std::make_exception_ptr(readError(from)); + co_return RemoteError{std::make_exception_ptr(readError(from))}; } else if (msg == STDERR_NEXT) @@ -789,21 +794,23 @@ std::exception_ptr RemoteStore::Connection::processStderr() throw Error("got unknown message type %x from Nix daemon", msg); } - return nullptr; + co_return RemoteError{nullptr}; +} catch (...) { + co_return result::current_exception(); } RemoteStore::ConnectionHandle::FramedSinkHandler::FramedSinkHandler( ConnectionHandle & conn, ThreadPool & handlerThreads ) - : stderrHandler([&]() { + : stderrHandler([&](AsyncIoRoot & aio) { try { - conn.processStderr(); + aio.blockOn(conn.processStderr()); } catch (...) { ex = std::current_exception(); } }) { - handlerThreads.enqueue([&] { stderrHandler(); }); + handlerThreads.enqueueWithAio([&](AsyncIoRoot & aio) { stderrHandler(aio); }); } RemoteStore::ConnectionHandle::FramedSinkHandler::~FramedSinkHandler() noexcept(false)