From 1a002d1a11aed3ccf6f7271d82a3a07e61888cf9 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Fri, 26 Apr 2024 18:04:19 +0200 Subject: [PATCH] libstore: de-callback-ify CA realisation substitution this is the *only* real user of file transfer download completion callbacks, and a pretty spurious user at that (seeing how nothing here is even turned on by default and indeed a dependency of path substitution which *isn't* async, and concurrency-limited). it'll be a real pain to keep this around, and realistically it would be a lot better to overhaul substitution in general to be *actually* async. that requires a proper async framework footing though, and we don't have anything of the sort, but it's also blocking *that* Change-Id: I1bf671f217c654a67377087607bf608728cbfc83 --- .../build/drv-output-substitution-goal.cc | 23 +++++++------------ .../build/drv-output-substitution-goal.hh | 2 +- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index 0e85650a7..17c96a9c7 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -72,25 +72,18 @@ void DrvOutputSubstitutionGoal::tryNext() sub = subs.front(); subs.pop_front(); - // FIXME: Make async - // outputInfo = sub->queryRealisation(id); - - /* The callback of the curl download below can outlive `this` (if + /* The async call to a curl download below can outlive `this` (if some other error occurs), so it must not touch `this`. So put the shared state in a separate refcounted object. */ downloadState = std::make_shared(); downloadState->outPipe.create(); - sub->queryRealisation( - id, - { [downloadState(downloadState)](std::future> res) { - try { - Finally updateStats([&]() { downloadState->outPipe.writeSide.close(); }); - downloadState->promise.set_value(res.get()); - } catch (...) { - downloadState->promise.set_exception(std::current_exception()); - } - } }); + downloadState->result = + std::async(std::launch::async, [downloadState{downloadState}, id{id}, sub{sub}] { + ReceiveInterrupts receiveInterrupts; + Finally updateStats([&]() { downloadState->outPipe.writeSide.close(); }); + return sub->queryRealisation(id); + }); worker.childStarted(shared_from_this(), {downloadState->outPipe.readSide.get()}, true, false); @@ -103,7 +96,7 @@ void DrvOutputSubstitutionGoal::realisationFetched() maintainRunningSubstitutions.reset(); try { - outputInfo = downloadState->promise.get_future().get(); + outputInfo = downloadState->result.get(); } catch (std::exception & e) { printError(e.what()); substituterFailed = true; diff --git a/src/libstore/build/drv-output-substitution-goal.hh b/src/libstore/build/drv-output-substitution-goal.hh index ab6fb796e..008b211a1 100644 --- a/src/libstore/build/drv-output-substitution-goal.hh +++ b/src/libstore/build/drv-output-substitution-goal.hh @@ -51,7 +51,7 @@ class DrvOutputSubstitutionGoal : public Goal { struct DownloadState { Pipe outPipe; - std::promise> promise; + std::future> result; }; std::shared_ptr downloadState;