diff options
Diffstat (limited to 'src')
47 files changed, 322 insertions, 388 deletions
diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index 83bfd4fb0..e7336c7e8 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -79,7 +79,7 @@ struct AttrDb state->txn->commit(); state->txn.reset(); } catch (...) { - ignoreException(); + ignoreExceptionInDestructor(); } } @@ -90,7 +90,7 @@ struct AttrDb try { return fun(); } catch (SQLiteError &) { - ignoreException(); + ignoreExceptionExceptInterrupt(); failed = true; return 0; } @@ -329,7 +329,7 @@ static std::shared_ptr<AttrDb> makeAttrDb( try { return std::make_shared<AttrDb>(cfg, fingerprint, symbols); } catch (SQLiteError &) { - ignoreException(); + ignoreExceptionExceptInterrupt(); return nullptr; } } diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 68da254e2..4b659b71a 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -21,6 +21,14 @@ std::ostream & operator <<(std::ostream & str, const SymbolStr & symbol) return printIdentifier(str, s); } +AttrName::AttrName(Symbol s) : symbol(s) +{ +} + +AttrName::AttrName(std::unique_ptr<Expr> e) : expr(std::move(e)) +{ +} + void Expr::show(const SymbolTable & symbols, std::ostream & str) const { abort(); diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index d16281c39..4e857b321 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -30,8 +30,8 @@ struct AttrName { Symbol symbol; std::unique_ptr<Expr> expr; - AttrName(Symbol s) : symbol(s) {}; - AttrName(std::unique_ptr<Expr> e) : expr(std::move(e)) {}; + AttrName(Symbol s); + AttrName(std::unique_ptr<Expr> e); }; typedef std::vector<AttrName> AttrPath; diff --git a/src/libexpr/parser/state.hh b/src/libexpr/parser/state.hh index 1d57e2f5f..3b9b90b94 100644 --- a/src/libexpr/parser/state.hh +++ b/src/libexpr/parser/state.hh @@ -9,7 +9,7 @@ namespace nix::parser { struct StringToken { std::string_view s; - bool hasIndentation; + bool hasIndentation = false; operator std::string_view() const { return s; } }; diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 64bd00606..029b457b1 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -395,7 +395,7 @@ RunPager::~RunPager() pid.wait(); } } catch (...) { - ignoreException(); + ignoreExceptionInDestructor(); } } diff --git a/src/libstore/build-result.hh b/src/libstore/build-result.hh index 9634fb944..846c6c9b9 100644 --- a/src/libstore/build-result.hh +++ b/src/libstore/build-result.hh @@ -47,7 +47,7 @@ struct BuildResult * @todo This should be an entire ErrorInfo object, not just a * string, for richer information. */ - std::string errorMsg; + std::string errorMsg = {}; std::string toString() const { auto strStatus = [&]() { @@ -90,7 +90,7 @@ struct BuildResult * For derivations, a mapping from the names of the wanted outputs * to actual paths. */ - SingleDrvOutputs builtOutputs; + SingleDrvOutputs builtOutputs = {}; /** * The start/stop times of the build (or one of the rounds, if it diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index d90a8639d..7f72efa6a 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -107,7 +107,7 @@ DerivationGoal::~DerivationGoal() noexcept(false) { /* Careful: we should never ever throw an exception from a destructor. */ - try { closeLogFile(); } catch (...) { ignoreException(); } + try { closeLogFile(); } catch (...) { ignoreExceptionInDestructor(); } } @@ -118,14 +118,14 @@ void DerivationGoal::killChild() } -Goal::Finished DerivationGoal::timedOut(Error && ex) +Goal::WorkResult DerivationGoal::timedOut(Error && ex) { killChild(); return done(BuildResult::TimedOut, {}, std::move(ex)); } -kj::Promise<Result<Goal::WorkResult>> DerivationGoal::work() noexcept +kj::Promise<Result<Goal::WorkResult>> DerivationGoal::workImpl() noexcept { return useDerivation ? getDerivation() : haveDerivation(); } @@ -267,7 +267,7 @@ try { /* We are first going to try to create the invalid output paths through substitutes. If that doesn't work, we'll build them. */ - kj::Vector<std::pair<GoalPtr, kj::Promise<void>>> dependencies; + kj::Vector<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies; if (settings.useSubstitutes) { if (parsedDrv->substitutesAllowed()) { for (auto & [outputName, status] : initialOutputs) { @@ -376,7 +376,7 @@ try { produced using a substitute. So we have to build instead. */ kj::Promise<Result<Goal::WorkResult>> DerivationGoal::gaveUpOnSubstitution() noexcept try { - kj::Vector<std::pair<GoalPtr, kj::Promise<void>>> dependencies; + kj::Vector<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies; /* At this point we are building all outputs, so if more are wanted there is no need to restart. */ @@ -482,7 +482,7 @@ try { } /* Check each path (slow!). */ - kj::Vector<std::pair<GoalPtr, kj::Promise<void>>> dependencies; + kj::Vector<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies; for (auto & i : outputClosure) { if (worker.pathContentsGood(i)) continue; printError( @@ -728,7 +728,7 @@ retry: if (!actLock) actLock = std::make_unique<Activity>(*logger, lvlWarn, actBuildWaiting, fmt("waiting for lock on %s", Magenta(showPaths(lockFiles)))); - (co_await waitForAWhile()).value(); + co_await waitForAWhile(); // we can loop very often, and `co_return co_await` always allocates a new frame goto retry; } @@ -799,7 +799,7 @@ retry: actLock = std::make_unique<Activity>(*logger, lvlTalkative, actBuildWaiting, fmt("waiting for a machine to build '%s'", Magenta(worker.store.printStorePath(drvPath)))); outputLocks.unlock(); - (co_await waitForAWhile()).value(); + co_await waitForAWhile(); goto retry; } @@ -863,7 +863,7 @@ void replaceValidPath(const Path & storePath, const Path & tmpPath) // attempt to recover movePath(oldPath, storePath); } catch (...) { - ignoreException(); + ignoreExceptionExceptInterrupt(); } throw; } @@ -1331,7 +1331,7 @@ void DerivationGoal::closeLogFile() } -Goal::Finished DerivationGoal::tooMuchLogs() +Goal::WorkResult DerivationGoal::tooMuchLogs() { killChild(); return done( @@ -1380,7 +1380,7 @@ struct DerivationGoal::InputStream final : private kj::AsyncObject } }; -kj::Promise<Outcome<void, Goal::Finished>> DerivationGoal::handleBuilderOutput(InputStream & in) noexcept +kj::Promise<Outcome<void, Goal::WorkResult>> DerivationGoal::handleBuilderOutput(InputStream & in) noexcept try { auto buf = kj::heapArray<char>(4096); while (true) { @@ -1413,7 +1413,7 @@ try { co_return std::current_exception(); } -kj::Promise<Outcome<void, Goal::Finished>> DerivationGoal::handleHookOutput(InputStream & in) noexcept +kj::Promise<Outcome<void, Goal::WorkResult>> DerivationGoal::handleHookOutput(InputStream & in) noexcept try { auto buf = kj::heapArray<char>(4096); while (true) { @@ -1467,7 +1467,7 @@ try { co_return std::current_exception(); } -kj::Promise<Outcome<void, Goal::Finished>> DerivationGoal::handleChildOutput() noexcept +kj::Promise<Outcome<void, Goal::WorkResult>> DerivationGoal::handleChildOutput() noexcept try { assert(builderOutFD); @@ -1483,7 +1483,7 @@ try { handlers = handlers.exclusiveJoin( worker.aio.provider->getTimer() .afterDelay(settings.buildTimeout.get() * kj::SECONDS) - .then([this]() -> Outcome<void, Finished> { + .then([this]() -> Outcome<void, WorkResult> { return timedOut( Error("%1% timed out after %2% seconds", name, settings.buildTimeout) ); @@ -1491,7 +1491,7 @@ try { ); } - return handlers.then([this](auto r) -> Outcome<void, Finished> { + return handlers.then([this](auto r) -> Outcome<void, WorkResult> { if (!currentLogLine.empty()) flushLine(); return r; }); @@ -1499,7 +1499,7 @@ try { return {std::current_exception()}; } -kj::Promise<Outcome<void, Goal::Finished>> DerivationGoal::monitorForSilence() noexcept +kj::Promise<Outcome<void, Goal::WorkResult>> DerivationGoal::monitorForSilence() noexcept { while (true) { const auto stash = lastChildActivity; @@ -1513,13 +1513,13 @@ kj::Promise<Outcome<void, Goal::Finished>> DerivationGoal::monitorForSilence() n } } -kj::Promise<Outcome<void, Goal::Finished>> +kj::Promise<Outcome<void, Goal::WorkResult>> DerivationGoal::handleChildStreams(InputStream & builderIn, InputStream * hookIn) noexcept { lastChildActivity = worker.aio.provider->getTimer().now(); auto handlers = kj::joinPromisesFailFast([&] { - kj::Vector<kj::Promise<Outcome<void, Finished>>> parts{2}; + kj::Vector<kj::Promise<Outcome<void, WorkResult>>> parts{2}; parts.add(handleBuilderOutput(builderIn)); if (hookIn) { @@ -1680,7 +1680,7 @@ SingleDrvOutputs DerivationGoal::assertPathValidity() } -Goal::Finished DerivationGoal::done( +Goal::WorkResult DerivationGoal::done( BuildResult::Status status, SingleDrvOutputs builtOutputs, std::optional<Error> ex) @@ -1717,7 +1717,7 @@ Goal::Finished DerivationGoal::done( logError(ex->info()); } - return Finished{ + return WorkResult{ .exitCode = buildResult.success() ? ecSuccess : ecFailed, .result = buildResult, .ex = ex ? std::make_shared<Error>(std::move(*ex)) : nullptr, diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index 1e2f9b55c..6dd58afd2 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -18,7 +18,7 @@ struct HookInstance; struct HookReplyBase { struct [[nodiscard]] Accept { - kj::Promise<Outcome<void, Goal::Finished>> promise; + kj::Promise<Outcome<void, Goal::WorkResult>> promise; }; struct [[nodiscard]] Decline {}; struct [[nodiscard]] Postpone {}; @@ -63,7 +63,7 @@ struct InitialOutputStatus { struct InitialOutput { bool wanted; Hash outputHash; - std::optional<InitialOutputStatus> known; + std::optional<InitialOutputStatus> known = {}; }; /** @@ -185,6 +185,11 @@ struct DerivationGoal : public Goal std::map<std::string, InitialOutput> initialOutputs; /** + * Build result. + */ + BuildResult buildResult; + + /** * File descriptor for the log file. */ AutoCloseFD fdLogFile; @@ -248,9 +253,9 @@ struct DerivationGoal : public Goal BuildMode buildMode = bmNormal); virtual ~DerivationGoal() noexcept(false); - Finished timedOut(Error && ex); + WorkResult timedOut(Error && ex); - kj::Promise<Result<WorkResult>> work() noexcept override; + kj::Promise<Result<WorkResult>> workImpl() noexcept override; /** * Add wanted outputs to an already existing derivation goal. @@ -319,13 +324,13 @@ struct DerivationGoal : public Goal protected: kj::TimePoint lastChildActivity = kj::minValue; - kj::Promise<Outcome<void, Finished>> handleChildOutput() noexcept; - kj::Promise<Outcome<void, Finished>> + kj::Promise<Outcome<void, WorkResult>> handleChildOutput() noexcept; + kj::Promise<Outcome<void, WorkResult>> handleChildStreams(InputStream & builderIn, InputStream * hookIn) noexcept; - kj::Promise<Outcome<void, Finished>> handleBuilderOutput(InputStream & in) noexcept; - kj::Promise<Outcome<void, Finished>> handleHookOutput(InputStream & in) noexcept; - kj::Promise<Outcome<void, Finished>> monitorForSilence() noexcept; - Finished tooMuchLogs(); + kj::Promise<Outcome<void, WorkResult>> handleBuilderOutput(InputStream & in) noexcept; + kj::Promise<Outcome<void, WorkResult>> handleHookOutput(InputStream & in) noexcept; + kj::Promise<Outcome<void, WorkResult>> monitorForSilence() noexcept; + WorkResult tooMuchLogs(); void flushLine(); public: @@ -360,7 +365,7 @@ public: void started(); - Finished done( + WorkResult done( BuildResult::Status status, SingleDrvOutputs builtOutputs = {}, std::optional<Error> ex = {}); diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index 9ffa33d7b..f04beb884 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -24,13 +24,13 @@ DrvOutputSubstitutionGoal::DrvOutputSubstitutionGoal( } -kj::Promise<Result<Goal::WorkResult>> DrvOutputSubstitutionGoal::work() noexcept +kj::Promise<Result<Goal::WorkResult>> DrvOutputSubstitutionGoal::workImpl() noexcept try { trace("init"); /* If the derivation already exists, we’re done */ if (worker.store.queryRealisation(id)) { - co_return Finished{ecSuccess, std::move(buildResult)}; + co_return WorkResult{ecSuccess}; } subs = settings.useSubstitutes ? getDefaultSubstituters() : std::list<ref<Store>>(); @@ -61,7 +61,7 @@ try { /* Hack: don't indicate failure if there were no substituters. In that case the calling derivation should just do a build. */ - co_return Finished{substituterFailed ? ecFailed : ecNoSubstituters, std::move(buildResult)}; + co_return WorkResult{substituterFailed ? ecFailed : ecNoSubstituters}; } sub = subs.front(); @@ -103,7 +103,7 @@ try { co_return co_await tryNext(); } - kj::Vector<std::pair<GoalPtr, kj::Promise<void>>> dependencies; + kj::Vector<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies; for (const auto & [depId, depPath] : outputInfo->dependentRealisations) { if (depId != id) { if (auto localOutputInfo = worker.store.queryRealisation(depId); @@ -140,9 +140,8 @@ try { if (nrFailed > 0) { debug("The output path of the derivation output '%s' could not be substituted", id.to_string()); - return {Finished{ + return {WorkResult{ nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed, - std::move(buildResult), }}; } @@ -155,7 +154,7 @@ try { kj::Promise<Result<Goal::WorkResult>> DrvOutputSubstitutionGoal::finished() noexcept try { trace("finished"); - return {Finished{ecSuccess, std::move(buildResult)}}; + return {WorkResult{ecSuccess}}; } catch (...) { return {std::current_exception()}; } diff --git a/src/libstore/build/drv-output-substitution-goal.hh b/src/libstore/build/drv-output-substitution-goal.hh index 86020705e..f959e2a7b 100644 --- a/src/libstore/build/drv-output-substitution-goal.hh +++ b/src/libstore/build/drv-output-substitution-goal.hh @@ -70,7 +70,7 @@ public: kj::Promise<Result<WorkResult>> outPathValid() noexcept; kj::Promise<Result<WorkResult>> finished() noexcept; - kj::Promise<Result<WorkResult>> work() noexcept override; + kj::Promise<Result<WorkResult>> workImpl() noexcept override; JobCategory jobCategory() const override { return JobCategory::Substitution; diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index 27c341295..808179a4d 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -25,14 +25,14 @@ void Store::buildPaths(const std::vector<DerivedPath> & reqs, BuildMode buildMod StringSet failed; std::shared_ptr<Error> ex; - for (auto & i : goals) { - if (i->ex) { + for (auto & [i, result] : goals) { + if (result.ex) { if (ex) - logError(i->ex->info()); + logError(result.ex->info()); else - ex = i->ex; + ex = result.ex; } - if (i->exitCode != Goal::ecSuccess) { + if (result.exitCode != Goal::ecSuccess) { if (auto i2 = dynamic_cast<DerivationGoal *>(i.get())) failed.insert(printStorePath(i2->drvPath)); else if (auto i2 = dynamic_cast<PathSubstitutionGoal *>(i.get())) @@ -72,7 +72,7 @@ std::vector<KeyedBuildResult> Store::buildPathsWithResults( std::vector<KeyedBuildResult> results; for (auto & [req, goalPtr] : state) - results.emplace_back(goalPtr->buildResult.restrictTo(req)); + results.emplace_back(goals[goalPtr].result.restrictTo(req)); return results; } @@ -89,8 +89,8 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat goals.emplace(gf.makeBasicDerivationGoal(drvPath, drv, OutputsSpec::All{}, buildMode)); return goals; }); - auto goal = *goals.begin(); - return goal->buildResult.restrictTo(DerivedPath::Built { + auto [goal, result] = *goals.begin(); + return result.result.restrictTo(DerivedPath::Built { .drvPath = makeConstantStorePathRef(drvPath), .outputs = OutputsSpec::All {}, }); @@ -116,12 +116,12 @@ void Store::ensurePath(const StorePath & path) goals.emplace(gf.makePathSubstitutionGoal(path)); return goals; }); - auto goal = *goals.begin(); + auto [goal, result] = *goals.begin(); - if (goal->exitCode != Goal::ecSuccess) { - if (goal->ex) { - goal->ex->withExitStatus(worker.failingExitStatus()); - throw std::move(*goal->ex); + if (result.exitCode != Goal::ecSuccess) { + if (result.ex) { + result.ex->withExitStatus(worker.failingExitStatus()); + throw std::move(*result.ex); } else throw Error(worker.failingExitStatus(), "path '%s' does not exist and cannot be created", printStorePath(path)); } @@ -138,9 +138,9 @@ void Store::repairPath(const StorePath & path) goals.emplace(gf.makePathSubstitutionGoal(path, Repair)); return goals; }); - auto goal = *goals.begin(); + auto [goal, result] = *goals.begin(); - if (goal->exitCode != Goal::ecSuccess) { + if (result.exitCode != Goal::ecSuccess) { /* Since substituting the path didn't work, if we have a valid deriver, then rebuild the deriver. */ auto info = queryPathInfo(path); diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index cfdb6717c..02b22b8ad 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -1,6 +1,7 @@ #include "goal.hh" #include "async-collect.hh" #include "worker.hh" +#include <boost/outcome/try.hpp> #include <kj/time.h> namespace nix { @@ -11,45 +12,60 @@ void Goal::trace(std::string_view s) debug("%1%: %2%", name, s); } -kj::Promise<Result<Goal::WorkResult>> Goal::waitForAWhile() -try { +kj::Promise<void> Goal::waitForAWhile() +{ trace("wait for a while"); /* If we are polling goals that are waiting for a lock, then wake up after a few seconds at most. */ - co_await worker.aio.provider->getTimer().afterDelay(settings.pollInterval.get() * kj::SECONDS); - co_return StillAlive{}; + return worker.aio.provider->getTimer().afterDelay(settings.pollInterval.get() * kj::SECONDS); +} + +kj::Promise<Result<Goal::WorkResult>> Goal::work() noexcept +try { + BOOST_OUTCOME_CO_TRY(auto result, co_await workImpl()); + + trace("done"); + + cleanup(); + + co_return std::move(result); } catch (...) { - co_return std::current_exception(); + co_return result::failure(std::current_exception()); } -kj::Promise<Result<Goal::WorkResult>> -Goal::waitForGoals(kj::Array<std::pair<GoalPtr, kj::Promise<void>>> dependencies) noexcept +kj::Promise<Result<void>> +Goal::waitForGoals(kj::Array<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies) noexcept try { auto left = dependencies.size(); for (auto & [dep, p] : dependencies) { - p = p.then([this, dep, &left] { + p = p.then([this, dep, &left](auto _result) -> Result<WorkResult> { + BOOST_OUTCOME_TRY(auto result, _result); + left--; trace(fmt("waitee '%s' done; %d left", dep->name, left)); - if (dep->exitCode != Goal::ecSuccess) ++nrFailed; - if (dep->exitCode == Goal::ecNoSubstituters) ++nrNoSubstituters; - if (dep->exitCode == Goal::ecIncompleteClosure) ++nrIncompleteClosure; + if (result.exitCode != Goal::ecSuccess) ++nrFailed; + if (result.exitCode == Goal::ecNoSubstituters) ++nrNoSubstituters; + if (result.exitCode == Goal::ecIncompleteClosure) ++nrIncompleteClosure; + + return std::move(result); }).eagerlyEvaluate(nullptr); } auto collectDeps = asyncCollect(std::move(dependencies)); while (auto item = co_await collectDeps.next()) { - auto & dep = *item; + auto & [dep, _result] = *item; + BOOST_OUTCOME_CO_TRY(auto result, _result); waiteeDone(dep); - if (dep->exitCode == ecFailed && !settings.keepGoing) { - co_return result::success(StillAlive{}); + if (result.exitCode == ecFailed && !settings.keepGoing) { + co_return result::success(); } } - co_return result::success(StillAlive{}); + co_return result::success(); } catch (...) { co_return result::failure(std::current_exception()); } diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index 17c3d85db..29540dcd3 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -82,64 +82,37 @@ struct Goal */ std::string name; - /** - * Whether the goal is finished. - */ - std::optional<ExitCode> exitCode; - - /** - * Build result. - */ - BuildResult buildResult; - - // for use by Worker only. will go away once work() is a promise. - kj::Own<kj::PromiseFulfiller<void>> notify; - protected: AsyncSemaphore::Token slotToken; public: - - struct Finished; - - struct [[nodiscard]] StillAlive {}; - struct [[nodiscard]] Finished { + struct [[nodiscard]] WorkResult { ExitCode exitCode; - BuildResult result; - std::shared_ptr<Error> ex; + BuildResult result = {}; + std::shared_ptr<Error> ex = {}; bool permanentFailure = false; bool timedOut = false; bool hashMismatch = false; bool checkMismatch = false; }; - struct [[nodiscard]] WorkResult : std::variant< - StillAlive, - Finished> - { - WorkResult() = delete; - using variant::variant; - }; - protected: - kj::Promise<Result<WorkResult>> waitForAWhile(); - kj::Promise<Result<WorkResult>> - waitForGoals(kj::Array<std::pair<GoalPtr, kj::Promise<void>>> dependencies) noexcept; + kj::Promise<void> waitForAWhile(); + kj::Promise<Result<void>> + waitForGoals(kj::Array<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies) noexcept; template<std::derived_from<Goal>... G> - kj::Promise<Result<Goal::WorkResult>> - waitForGoals(std::pair<std::shared_ptr<G>, kj::Promise<void>>... goals) noexcept + kj::Promise<Result<void>> + waitForGoals(std::pair<std::shared_ptr<G>, kj::Promise<Result<WorkResult>>>... goals) noexcept { - return waitForGoals(kj::arrOf<std::pair<GoalPtr, kj::Promise<void>>>(std::move(goals)...)); + return waitForGoals( + kj::arrOf<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>>(std::move(goals)...) + ); } -public: - - /** - * Exception containing an error message, if any. - */ - std::shared_ptr<Error> ex; + virtual kj::Promise<Result<WorkResult>> workImpl() noexcept = 0; +public: explicit Goal(Worker & worker, bool isDependency) : worker(worker) , isDependency(isDependency) @@ -150,7 +123,7 @@ public: trace("goal destroyed"); } - virtual kj::Promise<Result<WorkResult>> work() noexcept = 0; + kj::Promise<Result<WorkResult>> work() noexcept; virtual void waiteeDone(GoalPtr waitee) { } diff --git a/src/libstore/build/hook-instance.cc b/src/libstore/build/hook-instance.cc index f91a904cc..521f34917 100644 --- a/src/libstore/build/hook-instance.cc +++ b/src/libstore/build/hook-instance.cc @@ -1,4 +1,5 @@ #include "child.hh" +#include "error.hh" #include "file-system.hh" #include "globals.hh" #include "hook-instance.hh" @@ -86,7 +87,7 @@ HookInstance::~HookInstance() toHook.reset(); if (pid) pid.kill(); } catch (...) { - ignoreException(); + ignoreExceptionInDestructor(); } } diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index f3d0bc8b4..c8c68f99f 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1,4 +1,5 @@ #include "local-derivation-goal.hh" +#include "error.hh" #include "indirect-root-store.hh" #include "machines.hh" #include "store-api.hh" @@ -98,9 +99,9 @@ LocalDerivationGoal::~LocalDerivationGoal() noexcept(false) { /* Careful: we should never ever throw an exception from a destructor. */ - try { deleteTmpDir(false); } catch (...) { ignoreException(); } - try { killChild(); } catch (...) { ignoreException(); } - try { stopDaemon(); } catch (...) { ignoreException(); } + try { deleteTmpDir(false); } catch (...) { ignoreExceptionInDestructor(); } + try { killChild(); } catch (...) { ignoreExceptionInDestructor(); } + try { stopDaemon(); } catch (...) { ignoreExceptionInDestructor(); } } @@ -213,7 +214,7 @@ retry: if (!actLock) actLock = std::make_unique<Activity>(*logger, lvlWarn, actBuildWaiting, fmt("waiting for a free build user ID for '%s'", Magenta(worker.store.printStorePath(drvPath)))); - (co_await waitForAWhile()).value(); + co_await waitForAWhile(); // we can loop very often, and `co_return co_await` always allocates a new frame goto retry; } @@ -398,7 +399,7 @@ void LocalDerivationGoal::cleanupPostOutputsRegisteredModeNonCheck() // NOTE this one isn't noexcept because it's called from places that expect // exceptions to signal failure to launch. we should change this some time. -kj::Promise<Outcome<void, Goal::Finished>> LocalDerivationGoal::startBuilder() +kj::Promise<Outcome<void, Goal::WorkResult>> LocalDerivationGoal::startBuilder() { if ((buildUser && buildUser->getUIDCount() != 1) #if __linux__ @@ -1249,7 +1250,7 @@ void LocalDerivationGoal::startDaemon() NotTrusted, daemon::Recursive); debug("terminated daemon connection"); } catch (SysError &) { - ignoreException(); + ignoreExceptionExceptInterrupt(); } }); diff --git a/src/libstore/build/local-derivation-goal.hh b/src/libstore/build/local-derivation-goal.hh index 44bcd2ffe..cd6ea2b55 100644 --- a/src/libstore/build/local-derivation-goal.hh +++ b/src/libstore/build/local-derivation-goal.hh @@ -218,7 +218,7 @@ struct LocalDerivationGoal : public DerivationGoal /** * Start building a derivation. */ - kj::Promise<Outcome<void, Finished>> startBuilder(); + kj::Promise<Outcome<void, WorkResult>> startBuilder(); /** * Fill in the environment for the builder. diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index 8088bf668..e0ca23a86 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -32,21 +32,21 @@ PathSubstitutionGoal::~PathSubstitutionGoal() } -Goal::Finished PathSubstitutionGoal::done( +Goal::WorkResult PathSubstitutionGoal::done( ExitCode result, BuildResult::Status status, std::optional<std::string> errorMsg) { - buildResult.status = status; + BuildResult buildResult{.status = status}; if (errorMsg) { debug(*errorMsg); buildResult.errorMsg = *errorMsg; } - return Finished{result, std::move(buildResult)}; + return WorkResult{result, std::move(buildResult)}; } -kj::Promise<Result<Goal::WorkResult>> PathSubstitutionGoal::work() noexcept +kj::Promise<Result<Goal::WorkResult>> PathSubstitutionGoal::workImpl() noexcept try { trace("init"); @@ -157,7 +157,7 @@ try { /* To maintain the closure invariant, we first have to realise the paths referenced by this one. */ - kj::Vector<std::pair<GoalPtr, kj::Promise<void>>> dependencies; + kj::Vector<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies; for (auto & i : info->references) if (i != storePath) /* ignore self-references */ dependencies.add(worker.goalFactory().makePathSubstitutionGoal(i)); @@ -291,7 +291,7 @@ void PathSubstitutionGoal::cleanup() thr.get(); } } catch (...) { - ignoreException(); + ignoreExceptionInDestructor(); } } diff --git a/src/libstore/build/substitution-goal.hh b/src/libstore/build/substitution-goal.hh index dc701bcba..18b4262a4 100644 --- a/src/libstore/build/substitution-goal.hh +++ b/src/libstore/build/substitution-goal.hh @@ -72,7 +72,7 @@ struct PathSubstitutionGoal : public Goal */ std::optional<ContentAddress> ca; - Finished done( + WorkResult done( ExitCode result, BuildResult::Status status, std::optional<std::string> errorMsg = {}); @@ -87,7 +87,7 @@ public: ); ~PathSubstitutionGoal(); - kj::Promise<Result<WorkResult>> work() noexcept override; + kj::Promise<Result<WorkResult>> workImpl() noexcept override; /** * The states. diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index e063ede71..d1c1abdf8 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -1,3 +1,4 @@ +#include "async-collect.hh" #include "charptr-cast.hh" #include "worker.hh" #include "finally.hh" @@ -6,6 +7,8 @@ #include "local-derivation-goal.hh" #include "signals.hh" #include "hook-instance.hh" // IWYU pragma: keep +#include <boost/outcome/try.hpp> +#include <kj/vector.h> namespace nix { @@ -43,9 +46,12 @@ Worker::~Worker() goals that refer to this worker should be gone. (Otherwise we are in trouble, since goals may call childTerminated() etc. in their destructors). */ - topGoals.clear(); children.clear(); + derivationGoals.clear(); + drvOutputSubstitutionGoals.clear(); + substitutionGoals.clear(); + assert(expectedSubstitutions == 0); assert(expectedDownloadSize == 0); assert(expectedNarSize == 0); @@ -53,7 +59,7 @@ Worker::~Worker() template<typename ID, std::derived_from<Goal> G> -std::pair<std::shared_ptr<G>, kj::Promise<void>> Worker::makeGoalCommon( +std::pair<std::shared_ptr<G>, kj::Promise<Result<Goal::WorkResult>>> Worker::makeGoalCommon( std::map<ID, CachedGoal<G>> & map, const ID & key, InvocableR<std::unique_ptr<G>> auto create, @@ -65,31 +71,51 @@ std::pair<std::shared_ptr<G>, kj::Promise<void>> Worker::makeGoalCommon( // and then we only want to recreate the goal *once*. concurrent accesses // to the worker are not sound, we want to catch them if at all possible. for ([[maybe_unused]] auto _attempt : {1, 2}) { - auto & goal_weak = it->second; - auto goal = goal_weak.goal.lock(); + auto & cachedGoal = it->second; + auto & goal = cachedGoal.goal; if (!goal) { goal = create(); - goal->notify = std::move(goal_weak.fulfiller); - goal_weak.goal = goal; // do not start working immediately. if we are not yet running we // may create dependencies as though they were toplevel goals, in // which case the dependencies will not report build errors. when // we are running we may be called for this same goal more times, // and then we want to modify rather than recreate when possible. - childStarted(goal, kj::evalLater([goal] { return goal->work(); })); + auto removeWhenDone = [goal, &map, it] { + // c++ lambda coroutine capture semantics are *so* fucked up. + return [](auto goal, auto & map, auto it) -> kj::Promise<Result<Goal::WorkResult>> { + auto result = co_await goal->work(); + // a concurrent call to makeGoalCommon may have reset our + // cached goal and replaced it with a new instance. don't + // remove the goal in this case, otherwise we will crash. + if (goal == it->second.goal) { + map.erase(it); + } + co_return result; + }(goal, map, it); + }; + cachedGoal.promise = kj::evalLater(std::move(removeWhenDone)).fork(); + children.add(cachedGoal.promise.addBranch().then([this](auto _result) { + if (_result.has_value()) { + auto & result = _result.value(); + permanentFailure |= result.permanentFailure; + timedOut |= result.timedOut; + hashMismatch |= result.hashMismatch; + checkMismatch |= result.checkMismatch; + } + })); } else { if (!modify(*goal)) { - goal_weak = {}; + cachedGoal = {}; continue; } } - return {goal, goal_weak.promise->addBranch()}; + return {goal, cachedGoal.promise.addBranch()}; } assert(false && "could not make a goal. possible concurrent worker access"); } -std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<void>> Worker::makeDerivationGoal( +std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<Result<Goal::WorkResult>>> Worker::makeDerivationGoal( const StorePath & drvPath, const OutputsSpec & wantedOutputs, BuildMode buildMode ) { @@ -110,7 +136,7 @@ std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<void>> Worker::makeDeriva } -std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<void>> Worker::makeBasicDerivationGoal( +std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<Result<Goal::WorkResult>>> Worker::makeBasicDerivationGoal( const StorePath & drvPath, const BasicDerivation & drv, const OutputsSpec & wantedOutputs, @@ -134,7 +160,7 @@ std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<void>> Worker::makeBasicD } -std::pair<std::shared_ptr<PathSubstitutionGoal>, kj::Promise<void>> +std::pair<std::shared_ptr<PathSubstitutionGoal>, kj::Promise<Result<Goal::WorkResult>>> Worker::makePathSubstitutionGoal( const StorePath & path, RepairFlag repair, std::optional<ContentAddress> ca ) @@ -148,7 +174,7 @@ Worker::makePathSubstitutionGoal( } -std::pair<std::shared_ptr<DrvOutputSubstitutionGoal>, kj::Promise<void>> +std::pair<std::shared_ptr<DrvOutputSubstitutionGoal>, kj::Promise<Result<Goal::WorkResult>>> Worker::makeDrvOutputSubstitutionGoal( const DrvOutput & id, RepairFlag repair, std::optional<ContentAddress> ca ) @@ -162,114 +188,22 @@ Worker::makeDrvOutputSubstitutionGoal( } -std::pair<GoalPtr, kj::Promise<void>> Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) +std::pair<GoalPtr, kj::Promise<Result<Goal::WorkResult>>> Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) { return std::visit(overloaded { - [&](const DerivedPath::Built & bfd) -> std::pair<GoalPtr, kj::Promise<void>> { + [&](const DerivedPath::Built & bfd) -> std::pair<GoalPtr, kj::Promise<Result<Goal::WorkResult>>> { if (auto bop = std::get_if<DerivedPath::Opaque>(&*bfd.drvPath)) return makeDerivationGoal(bop->path, bfd.outputs, buildMode); else throw UnimplementedError("Building dynamic derivations in one shot is not yet implemented."); }, - [&](const DerivedPath::Opaque & bo) -> std::pair<GoalPtr, kj::Promise<void>> { + [&](const DerivedPath::Opaque & bo) -> std::pair<GoalPtr, kj::Promise<Result<Goal::WorkResult>>> { return makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair); }, }, req.raw()); } - -template<typename G> -static void removeGoal(std::shared_ptr<G> goal, auto & goalMap) -{ - /* !!! inefficient */ - for (auto i = goalMap.begin(); - i != goalMap.end(); ) - if (i->second.goal.lock() == goal) { - auto j = i; ++j; - goalMap.erase(i); - i = j; - } - else ++i; -} - - -void Worker::goalFinished(GoalPtr goal, Goal::Finished & f) -{ - goal->trace("done"); - assert(!goal->exitCode.has_value()); - goal->exitCode = f.exitCode; - goal->ex = f.ex; - - permanentFailure |= f.permanentFailure; - timedOut |= f.timedOut; - hashMismatch |= f.hashMismatch; - checkMismatch |= f.checkMismatch; - - removeGoal(goal); - goal->notify->fulfill(); - goal->cleanup(); -} - -void Worker::handleWorkResult(GoalPtr goal, Goal::WorkResult how) -{ - std::visit( - overloaded{ - [&](Goal::StillAlive) { - childStarted(goal, kj::evalLater([goal] { return goal->work(); })); - }, - [&](Goal::Finished & f) { goalFinished(goal, f); }, - }, - how - ); - updateStatistics(); -} - -void Worker::removeGoal(GoalPtr goal) -{ - if (auto drvGoal = std::dynamic_pointer_cast<DerivationGoal>(goal)) - nix::removeGoal(drvGoal, derivationGoals); - else if (auto subGoal = std::dynamic_pointer_cast<PathSubstitutionGoal>(goal)) - nix::removeGoal(subGoal, substitutionGoals); - else if (auto subGoal = std::dynamic_pointer_cast<DrvOutputSubstitutionGoal>(goal)) - nix::removeGoal(subGoal, drvOutputSubstitutionGoals); - else - assert(false); - - if (topGoals.find(goal) != topGoals.end()) { - topGoals.erase(goal); - /* If a top-level goal failed, then kill all other goals - (unless keepGoing was set). */ - if (goal->exitCode == Goal::ecFailed && !settings.keepGoing) - topGoals.clear(); - } -} - - -void Worker::childStarted(GoalPtr goal, kj::Promise<Result<Goal::WorkResult>> promise) -{ - children.add(promise - .then([this, goal](auto result) { - if (result.has_value()) { - handleWorkResult(goal, std::move(result.assume_value())); - } else { - childException = result.assume_error(); - } - }) - .attach(Finally{[this, goal] { - childTerminated(goal); - }})); -} - - -void Worker::childTerminated(GoalPtr goal) -{ - if (childFinished) { - childFinished->fulfill(); - } -} - - -kj::Promise<Result<void>> Worker::updateStatistics() +kj::Promise<Result<Worker::Results>> Worker::updateStatistics() try { while (true) { statisticsUpdateInhibitor = co_await statisticsUpdateSignal.acquire(); @@ -295,52 +229,54 @@ try { co_return result::failure(std::current_exception()); } -std::vector<GoalPtr> Worker::run(std::function<Targets (GoalFactory &)> req) +Worker::Results Worker::run(std::function<Targets (GoalFactory &)> req) { - auto _topGoals = req(goalFactory()); + auto topGoals = req(goalFactory()); assert(!running); running = true; Finally const _stop([&] { running = false; }); - topGoals.clear(); - for (auto & [goal, _promise] : _topGoals) { - topGoals.insert(goal); - } + auto onInterrupt = kj::newPromiseAndCrossThreadFulfiller<Result<Results>>(); + auto interruptCallback = createInterruptCallback([&] { + return result::failure(std::make_exception_ptr(makeInterrupted())); + }); - auto promise = runImpl().exclusiveJoin(updateStatistics()); + auto promise = runImpl(std::move(topGoals)) + .exclusiveJoin(updateStatistics()) + .exclusiveJoin(std::move(onInterrupt.promise)); // TODO GC interface? - if (auto localStore = dynamic_cast<LocalStore *>(&store); localStore && settings.minFree != 0) { + if (auto localStore = dynamic_cast<LocalStore *>(&store); localStore && settings.minFree != 0u) { // Periodically wake up to see if we need to run the garbage collector. promise = promise.exclusiveJoin(boopGC(*localStore)); } - promise.wait(aio.waitScope).value(); - - std::vector<GoalPtr> results; - for (auto & [i, _p] : _topGoals) { - results.push_back(i); - } - return results; + return promise.wait(aio.waitScope).value(); } -kj::Promise<Result<void>> Worker::runImpl() +kj::Promise<Result<Worker::Results>> Worker::runImpl(Targets topGoals) try { debug("entered goal loop"); - while (1) { - - checkInterrupt(); + kj::Vector<Targets::value_type> promises(topGoals.size()); + for (auto & gp : topGoals) { + promises.add(std::move(gp)); + } - if (topGoals.empty()) break; + Results results; - /* Wait for input. */ - if (!children.isEmpty()) - (co_await waitForInput()).value(); + auto collect = AsyncCollect(promises.releaseAsArray()); + while (auto done = co_await collect.next()) { + // propagate goal exceptions outward + BOOST_OUTCOME_CO_TRY(auto result, done->second); + results.emplace(done->first, result); - if (childException) { - std::rethrow_exception(childException); + /* If a top-level goal failed, then kill all other goals + (unless keepGoing was set). */ + if (result.exitCode == Goal::ecFailed && !settings.keepGoing) { + children.clear(); + break; } } @@ -349,12 +285,12 @@ try { --keep-going *is* set, then they must all be finished now. */ assert(!settings.keepGoing || children.isEmpty()); - co_return result::success(); + co_return std::move(results); } catch (...) { co_return result::failure(std::current_exception()); } -kj::Promise<Result<void>> Worker::boopGC(LocalStore & localStore) +kj::Promise<Result<Worker::Results>> Worker::boopGC(LocalStore & localStore) try { while (true) { co_await aio.provider->getTimer().afterDelay(10 * kj::SECONDS); @@ -364,18 +300,6 @@ try { co_return result::failure(std::current_exception()); } -kj::Promise<Result<void>> Worker::waitForInput() -try { - printMsg(lvlVomit, "waiting for children"); - - auto pair = kj::newPromiseAndFulfiller<void>(); - this->childFinished = kj::mv(pair.fulfiller); - co_await pair.promise; - co_return result::success(); -} catch (...) { - co_return result::failure(std::current_exception()); -} - unsigned int Worker::failingExitStatus() { diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index cd49fb860..1a913ca16 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -30,10 +30,12 @@ struct HookInstance; class GoalFactory { public: - virtual std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<void>> makeDerivationGoal( + virtual std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<Result<Goal::WorkResult>>> + makeDerivationGoal( const StorePath & drvPath, const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal ) = 0; - virtual std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<void>> makeBasicDerivationGoal( + virtual std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<Result<Goal::WorkResult>>> + makeBasicDerivationGoal( const StorePath & drvPath, const BasicDerivation & drv, const OutputsSpec & wantedOutputs, @@ -43,13 +45,13 @@ public: /** * @ref SubstitutionGoal "substitution goal" */ - virtual std::pair<std::shared_ptr<PathSubstitutionGoal>, kj::Promise<void>> + virtual std::pair<std::shared_ptr<PathSubstitutionGoal>, kj::Promise<Result<Goal::WorkResult>>> makePathSubstitutionGoal( const StorePath & storePath, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt ) = 0; - virtual std::pair<std::shared_ptr<DrvOutputSubstitutionGoal>, kj::Promise<void>> + virtual std::pair<std::shared_ptr<DrvOutputSubstitutionGoal>, kj::Promise<Result<Goal::WorkResult>>> makeDrvOutputSubstitutionGoal( const DrvOutput & id, RepairFlag repair = NoRepair, @@ -62,7 +64,7 @@ public: * It will be a `DerivationGoal` for a `DerivedPath::Built` or * a `SubstitutionGoal` for a `DerivedPath::Opaque`. */ - virtual std::pair<GoalPtr, kj::Promise<void>> + virtual std::pair<GoalPtr, kj::Promise<Result<Goal::WorkResult>>> makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal) = 0; }; @@ -82,28 +84,19 @@ protected: */ class Worker : public WorkerBase { +public: + using Targets = std::map<GoalPtr, kj::Promise<Result<Goal::WorkResult>>>; + using Results = std::map<GoalPtr, Goal::WorkResult>; + private: bool running = false; - /** - * The top-level goals of the worker. - */ - Goals topGoals; - template<typename G> struct CachedGoal { - std::weak_ptr<G> goal; - kj::Own<kj::ForkedPromise<void>> promise; - kj::Own<kj::PromiseFulfiller<void>> fulfiller; - - CachedGoal() - { - auto pf = kj::newPromiseAndFulfiller<void>(); - promise = kj::heap(pf.promise.fork()); - fulfiller = std::move(pf.fulfiller); - } + std::shared_ptr<G> goal; + kj::ForkedPromise<Result<Goal::WorkResult>> promise{nullptr}; }; /** * Maps used to prevent multiple instantiations of a goal for the @@ -139,35 +132,10 @@ private: */ bool checkMismatch = false; - void goalFinished(GoalPtr goal, Goal::Finished & f); - void handleWorkResult(GoalPtr goal, Goal::WorkResult how); - - kj::Own<kj::PromiseFulfiller<void>> childFinished; - - /** - * Wait for input to become available. - */ - kj::Promise<Result<void>> waitForInput(); - - /** - * Remove a dead goal. - */ - void removeGoal(GoalPtr goal); - - /** - * Registers a running child process. - */ - void childStarted(GoalPtr goal, kj::Promise<Result<Goal::WorkResult>> promise); - - /** - * Unregisters a running child process. - */ - void childTerminated(GoalPtr goal); - /** * Pass current stats counters to the logger for progress bar updates. */ - kj::Promise<Result<void>> updateStatistics(); + kj::Promise<Result<Results>> updateStatistics(); AsyncSemaphore statisticsUpdateSignal{1}; std::optional<AsyncSemaphore::Token> statisticsUpdateInhibitor; @@ -180,8 +148,8 @@ private: statisticsUpdateInhibitor = {}; } - kj::Promise<Result<void>> runImpl(); - kj::Promise<Result<void>> boopGC(LocalStore & localStore); + kj::Promise<Result<Results>> runImpl(Targets topGoals); + kj::Promise<Result<Results>> boopGC(LocalStore & localStore); public: @@ -196,7 +164,6 @@ public: private: kj::TaskSet children; - std::exception_ptr childException; public: struct HookState { @@ -237,29 +204,29 @@ public: */ private: template<typename ID, std::derived_from<Goal> G> - std::pair<std::shared_ptr<G>, kj::Promise<void>> makeGoalCommon( + std::pair<std::shared_ptr<G>, kj::Promise<Result<Goal::WorkResult>>> makeGoalCommon( std::map<ID, CachedGoal<G>> & map, const ID & key, InvocableR<std::unique_ptr<G>> auto create, InvocableR<bool, G &> auto modify ); - std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<void>> makeDerivationGoal( + std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<Result<Goal::WorkResult>>> makeDerivationGoal( const StorePath & drvPath, const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal) override; - std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<void>> makeBasicDerivationGoal( + std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<Result<Goal::WorkResult>>> makeBasicDerivationGoal( const StorePath & drvPath, const BasicDerivation & drv, const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal) override; /** * @ref SubstitutionGoal "substitution goal" */ - std::pair<std::shared_ptr<PathSubstitutionGoal>, kj::Promise<void>> + std::pair<std::shared_ptr<PathSubstitutionGoal>, kj::Promise<Result<Goal::WorkResult>>> makePathSubstitutionGoal( const StorePath & storePath, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt ) override; - std::pair<std::shared_ptr<DrvOutputSubstitutionGoal>, kj::Promise<void>> + std::pair<std::shared_ptr<DrvOutputSubstitutionGoal>, kj::Promise<Result<Goal::WorkResult>>> makeDrvOutputSubstitutionGoal( const DrvOutput & id, RepairFlag repair = NoRepair, @@ -272,16 +239,14 @@ private: * It will be a `DerivationGoal` for a `DerivedPath::Built` or * a `SubstitutionGoal` for a `DerivedPath::Opaque`. */ - std::pair<GoalPtr, kj::Promise<void>> + std::pair<GoalPtr, kj::Promise<Result<Goal::WorkResult>>> makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal) override; public: - using Targets = std::map<GoalPtr, kj::Promise<void>>; - /** * Loop until the specified top-level goals have finished. */ - std::vector<GoalPtr> run(std::function<Targets (GoalFactory &)> req); + Results run(std::function<Targets (GoalFactory &)> req); /*** * The exit status in case of failure. diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 10c810e49..34b92148e 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -6,6 +6,7 @@ #include "signals.hh" #include "compression.hh" #include "strings.hh" +#include <cstddef> #if ENABLE_S3 #include <aws/core/client/ClientConfiguration.h> @@ -115,7 +116,7 @@ struct curlFileTransfer : public FileTransfer if (!done) fail(FileTransferError(Interrupted, {}, "download of '%s' was interrupted", request.uri)); } catch (...) { - ignoreException(); + ignoreExceptionInDestructor(); } } @@ -784,8 +785,10 @@ struct curlFileTransfer : public FileTransfer size_t read(char * data, size_t len) override { - auto readPartial = [this](char * data, size_t len) { + auto readPartial = [this](char * data, size_t len) -> size_t { const auto available = std::min(len, buffered.size()); + if (available == 0u) return 0u; + memcpy(data, buffered.data(), available); buffered.remove_prefix(available); return available; diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index d5903d01e..99bf80994 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -923,8 +923,8 @@ void LocalStore::autoGC(bool sync) } catch (...) { // FIXME: we could propagate the exception to the - // future, but we don't really care. - ignoreException(); + // future, but we don't really care. (what??) + ignoreExceptionInDestructor(); } }).detach(); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 1af0f54de..c3248c2c3 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -481,7 +481,7 @@ LocalStore::~LocalStore() unlink(fnTempRoots.c_str()); } } catch (...) { - ignoreException(); + ignoreExceptionInDestructor(); } } @@ -1222,7 +1222,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, try { parseDump(sink, source); } catch (...) { - ignoreException(); + ignoreExceptionExceptInterrupt(); } } }; diff --git a/src/libstore/nar-accessor.cc b/src/libstore/nar-accessor.cc index 7600de6e7..f228004a9 100644 --- a/src/libstore/nar-accessor.cc +++ b/src/libstore/nar-accessor.cc @@ -20,10 +20,10 @@ struct NarMember file in the NAR. */ uint64_t start = 0, size = 0; - std::string target; + std::string target = {}; /* If this is a directory, all the children of the directory. */ - std::map<std::string, NarMember> children; + std::map<std::string, NarMember> children = {}; }; struct NarAccessor : public FSAccessor diff --git a/src/libstore/optimise-store.cc b/src/libstore/optimise-store.cc index c60e5a85d..14381b6e0 100644 --- a/src/libstore/optimise-store.cc +++ b/src/libstore/optimise-store.cc @@ -31,7 +31,7 @@ struct MakeReadOnly /* This will make the path read-only. */ if (path != "") canonicaliseTimestampAndPermissions(path); } catch (...) { - ignoreException(); + ignoreExceptionInDestructor(); } } }; diff --git a/src/libstore/path-with-outputs.hh b/src/libstore/path-with-outputs.hh index 57e03252d..8e2da1908 100644 --- a/src/libstore/path-with-outputs.hh +++ b/src/libstore/path-with-outputs.hh @@ -17,7 +17,7 @@ namespace nix { struct StorePathWithOutputs { StorePath path; - std::set<std::string> outputs; + std::set<std::string> outputs = {}; std::string to_string(const Store & store) const; diff --git a/src/libstore/pathlocks.cc b/src/libstore/pathlocks.cc index ced0f30bb..3225857ec 100644 --- a/src/libstore/pathlocks.cc +++ b/src/libstore/pathlocks.cc @@ -145,7 +145,7 @@ PathLocks::~PathLocks() try { unlock(); } catch (...) { - ignoreException(); + ignoreExceptionInDestructor(); } } diff --git a/src/libstore/pathlocks.hh b/src/libstore/pathlocks.hh index d06d031b5..feb0f5548 100644 --- a/src/libstore/pathlocks.hh +++ b/src/libstore/pathlocks.hh @@ -1,6 +1,7 @@ #pragma once ///@file +#include "error.hh" #include "file-descriptor.hh" namespace nix { @@ -53,7 +54,7 @@ struct FdLock if (acquired) lockFile(fd, ltNone, false); } catch (SysError &) { - ignoreException(); + ignoreExceptionInDestructor(); } } }; diff --git a/src/libstore/realisation.hh b/src/libstore/realisation.hh index f2b228fa0..baeb7a2c9 100644 --- a/src/libstore/realisation.hh +++ b/src/libstore/realisation.hh @@ -50,7 +50,7 @@ struct Realisation { DrvOutput id; StorePath outPath; - StringSet signatures; + StringSet signatures = {}; /** * The realisations that are required for the current one to be valid. @@ -58,7 +58,7 @@ struct Realisation { * When importing this realisation, the store will first check that all its * dependencies exist, and map to the correct output path */ - std::map<DrvOutput, StorePath> dependentRealisations; + std::map<DrvOutput, StorePath> dependentRealisations = {}; nlohmann::json toJSON() const; static Realisation fromJSON(const nlohmann::json& json, const std::string& whence); diff --git a/src/libstore/remote-fs-accessor.cc b/src/libstore/remote-fs-accessor.cc index 0689ce74d..59d267873 100644 --- a/src/libstore/remote-fs-accessor.cc +++ b/src/libstore/remote-fs-accessor.cc @@ -29,7 +29,7 @@ ref<FSAccessor> RemoteFSAccessor::addToCache(std::string_view hashPart, std::str /* FIXME: do this asynchronously. */ writeFile(makeCacheFile(hashPart, "nar"), nar); } catch (...) { - ignoreException(); + ignoreExceptionExceptInterrupt(); } } @@ -41,7 +41,7 @@ ref<FSAccessor> RemoteFSAccessor::addToCache(std::string_view hashPart, std::str nlohmann::json j = listNar(narAccessor, "", true); writeFile(makeCacheFile(hashPart, "ls"), j.dump()); } catch (...) { - ignoreException(); + ignoreExceptionExceptInterrupt(); } } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 1f94ca03f..a9f9818be 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -1,3 +1,4 @@ +#include "error.hh" #include "serialise.hh" #include "signals.hh" #include "path-with-outputs.hh" @@ -855,7 +856,7 @@ RemoteStore::Connection::~Connection() try { to.flush(); } catch (...) { - ignoreException(); + ignoreExceptionInDestructor(); } } @@ -985,7 +986,7 @@ void RemoteStore::ConnectionHandle::withFramedSink(std::function<void(Sink & sin try { std::rethrow_exception(ex); } catch (...) { - ignoreException(); + ignoreExceptionExceptInterrupt(); } } } diff --git a/src/libstore/sqlite.cc b/src/libstore/sqlite.cc index 8d0bfcb11..7aa0b6629 100644 --- a/src/libstore/sqlite.cc +++ b/src/libstore/sqlite.cc @@ -85,7 +85,7 @@ SQLite::~SQLite() if (db && sqlite3_close(db) != SQLITE_OK) SQLiteError::throw_(db, "closing database"); } catch (...) { - ignoreException(); + ignoreExceptionInDestructor(); } } @@ -124,7 +124,7 @@ SQLiteStmt::~SQLiteStmt() if (stmt && sqlite3_finalize(stmt) != SQLITE_OK) SQLiteError::throw_(db, "finalizing statement '%s'", sql); } catch (...) { - ignoreException(); + ignoreExceptionInDestructor(); } } @@ -248,7 +248,7 @@ SQLiteTxn::~SQLiteTxn() if (active && sqlite3_exec(db, "rollback;", 0, 0, 0) != SQLITE_OK) SQLiteError::throw_(db, "aborting transaction"); } catch (...) { - ignoreException(); + ignoreExceptionInDestructor(); } } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 50d392779..18f80eef8 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -829,7 +829,7 @@ StorePathSet Store::queryValidPaths(const StorePathSet & paths, SubstituteFlag m { size_t left; StorePathSet valid; - std::exception_ptr exc; + std::exception_ptr exc = {}; }; Sync<State> state_(State{paths.size(), StorePathSet()}); @@ -1163,7 +1163,7 @@ std::map<StorePath, StorePath> copyPaths( // not be within our control to change that, and we might still want // to at least copy the output paths. if (e.missingFeature == Xp::CaDerivations) - ignoreException(); + ignoreExceptionExceptInterrupt(); else throw; } diff --git a/src/libutil/current-process.cc b/src/libutil/current-process.cc index 33cda211b..3b3e46a9a 100644 --- a/src/libutil/current-process.cc +++ b/src/libutil/current-process.cc @@ -49,7 +49,7 @@ unsigned int getMaxCPU() auto period = cpuMaxParts[1]; if (quota != "max") return std::ceil(std::stoi(quota) / std::stof(period)); - } catch (Error &) { ignoreException(lvlDebug); } + } catch (Error &) { ignoreExceptionInDestructor(lvlDebug); } #endif return 0; diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 6d178dfe5..f57e3ef7d 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -4,6 +4,7 @@ #include "position.hh" #include "terminal.hh" #include "strings.hh" +#include "signals.hh" #include <iostream> #include <optional> @@ -416,7 +417,7 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s return out; } -void ignoreException(Verbosity lvl) +void ignoreExceptionInDestructor(Verbosity lvl) { /* Make sure no exceptions leave this function. printError() also throws when remote is closed. */ @@ -429,4 +430,15 @@ void ignoreException(Verbosity lvl) } catch (...) { } } +void ignoreExceptionExceptInterrupt(Verbosity lvl) +{ + try { + throw; + } catch (const Interrupted & e) { + throw; + } catch (std::exception & e) { + printMsg(lvl, "error (ignored): %1%", e.what()); + } +} + } diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 73c1ccadd..885a2b218 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -70,17 +70,17 @@ inline bool operator<=(const Trace& lhs, const Trace& rhs); inline bool operator>=(const Trace& lhs, const Trace& rhs); struct ErrorInfo { - Verbosity level; + Verbosity level = Verbosity::lvlError; HintFmt msg; std::shared_ptr<Pos> pos; - std::list<Trace> traces; + std::list<Trace> traces = {}; /** * Exit status. */ unsigned int status = 1; - Suggestions suggestions; + Suggestions suggestions = {}; static std::optional<std::string> programName; }; @@ -204,7 +204,22 @@ public: /** * Exception handling in destructors: print an error message, then * ignore the exception. + * + * If you're not in a destructor, you usually want to use `ignoreExceptionExceptInterrupt()`. + * + * This function might also be used in callbacks whose caller may not handle exceptions, + * but ideally we propagate the exception using an exception_ptr in such cases. + * See e.g. `PackBuilderContext` + */ +void ignoreExceptionInDestructor(Verbosity lvl = lvlError); + +/** + * Not destructor-safe. + * Print an error message, then ignore the exception. + * If the exception is an `Interrupted` exception, rethrow it. + * + * This may be used in a few places where Interrupt can't happen, but that's ok. */ -void ignoreException(Verbosity lvl = lvlError); +void ignoreExceptionExceptInterrupt(Verbosity lvl = lvlError); } diff --git a/src/libutil/file-descriptor.cc b/src/libutil/file-descriptor.cc index 8385ea402..cbb2bb539 100644 --- a/src/libutil/file-descriptor.cc +++ b/src/libutil/file-descriptor.cc @@ -146,7 +146,7 @@ AutoCloseFD::~AutoCloseFD() try { close(); } catch (...) { - ignoreException(); + ignoreExceptionInDestructor(); } } diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index 1d3eba58f..c4ffb1d0c 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -522,7 +522,7 @@ AutoDelete::~AutoDelete() } } } catch (...) { - ignoreException(); + ignoreExceptionInDestructor(); } } diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 7d9482814..7609e6e39 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -352,7 +352,7 @@ Activity::~Activity() try { logger.stopActivity(id); } catch (...) { - ignoreException(); + ignoreExceptionInDestructor(); } } diff --git a/src/libutil/processes.hh b/src/libutil/processes.hh index dc09a9ba4..dd6e2978e 100644 --- a/src/libutil/processes.hh +++ b/src/libutil/processes.hh @@ -78,11 +78,11 @@ struct RunOptions { Path program; bool searchPath = true; - Strings args; - std::optional<uid_t> uid; - std::optional<uid_t> gid; - std::optional<Path> chdir; - std::optional<std::map<std::string, std::string>> environment; + Strings args = {}; + std::optional<uid_t> uid = {}; + std::optional<uid_t> gid = {}; + std::optional<Path> chdir = {}; + std::optional<std::map<std::string, std::string>> environment = {}; bool captureStdout = false; bool mergeStderrToStdout = false; bool isInteractive = false; diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index f509fedff..2f5a11a28 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -83,7 +83,7 @@ void BufferedSink::flush() FdSink::~FdSink() { - try { flush(); } catch (...) { ignoreException(); } + try { flush(); } catch (...) { ignoreExceptionInDestructor(); } } diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index 3a9685e0e..08ea9a135 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -549,7 +549,7 @@ struct FramedSource : Source } } } catch (...) { - ignoreException(); + ignoreExceptionInDestructor(); } } @@ -595,7 +595,7 @@ struct FramedSink : nix::BufferedSink to << 0; to.flush(); } catch (...) { - ignoreException(); + ignoreExceptionInDestructor(); } } diff --git a/src/libutil/signals.cc b/src/libutil/signals.cc index 04a697d01..dac2964ae 100644 --- a/src/libutil/signals.cc +++ b/src/libutil/signals.cc @@ -12,13 +12,18 @@ std::atomic<bool> _isInterrupted = false; thread_local std::function<bool()> interruptCheck; +Interrupted makeInterrupted() +{ + return Interrupted("interrupted by the user"); +} + void _interrupted() { /* Block user interrupts while an exception is being handled. Throwing an exception while another exception is being handled kills the program! */ if (!std::uncaught_exceptions()) { - throw Interrupted("interrupted by the user"); + throw makeInterrupted(); } } @@ -78,7 +83,7 @@ void triggerInterrupt() try { callback(); } catch (...) { - ignoreException(); + ignoreExceptionInDestructor(); } } } diff --git a/src/libutil/signals.hh b/src/libutil/signals.hh index 02f8d2ca3..538ff94b4 100644 --- a/src/libutil/signals.hh +++ b/src/libutil/signals.hh @@ -16,10 +16,13 @@ namespace nix { /* User interruption. */ +class Interrupted; + extern std::atomic<bool> _isInterrupted; extern thread_local std::function<bool()> interruptCheck; +Interrupted makeInterrupted(); void _interrupted(); void inline checkInterrupt() diff --git a/src/libutil/thread-pool.cc b/src/libutil/thread-pool.cc index 0ff83e997..cd380b608 100644 --- a/src/libutil/thread-pool.cc +++ b/src/libutil/thread-pool.cc @@ -109,9 +109,8 @@ void ThreadPool::doWork(bool mainThread) try { std::rethrow_exception(exc); } catch (std::exception & e) { - if (!dynamic_cast<Interrupted*>(&e) && - !dynamic_cast<ThreadPoolShutDown*>(&e)) - ignoreException(); + if (!dynamic_cast<ThreadPoolShutDown*>(&e)) + ignoreExceptionExceptInterrupt(); } catch (...) { } } diff --git a/src/nix/develop.cc b/src/nix/develop.cc index fb144c904..d1615ecdc 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -639,7 +639,7 @@ struct CmdDevelop : Common, MixEnvironment throw Error("package 'nixpkgs#bashInteractive' does not provide a 'bin/bash'"); } catch (Error &) { - ignoreException(); + ignoreExceptionExceptInterrupt(); } // Override SHELL with the one chosen for this environment. diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 15c393c90..0c704a995 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -16,6 +16,7 @@ #include "eval-cache.hh" #include "markdown.hh" #include "terminal.hh" +#include "signals.hh" #include <limits> #include <nlohmann/json.hpp> @@ -367,9 +368,11 @@ struct CmdFlakeCheck : FlakeCommand auto reportError = [&](const Error & e) { try { throw e; + } catch (Interrupted & e) { + throw; } catch (Error & e) { if (settings.keepGoing) { - ignoreException(); + ignoreExceptionExceptInterrupt(); hasErrors = true; } else |