diff options
author | eldritch horrors <pennae@lix.systems> | 2024-10-05 00:38:35 +0200 |
---|---|---|
committer | eldritch horrors <pennae@lix.systems> | 2024-10-05 21:19:51 +0000 |
commit | 9adf6f4568d5a6b1c61ff93beb12a45b962c2602 (patch) | |
tree | e4c158535a8db9f2e1e1312f0d829bfb30648dd5 | |
parent | 03cbc0ecb9402fe7bbe1a2acd4643995003d7bb2 (diff) |
libstore: remove Goal::notify
Goal::work() is a fully usable promise that does not rely on the worker
to report completion conditions. as such we no longer need the `notify`
field that enabled this interplay. we do have to clear goal caches when
destroying the worker though, otherwise goal promises may (incorrectly)
keep goals alive due to strong shared pointers created by childStarted.
Change-Id: Ie607209aafec064dbdf3464fe207d70ba9ee158a
-rw-r--r-- | src/libstore/build/goal.cc | 1 | ||||
-rw-r--r-- | src/libstore/build/worker.cc | 12 | ||||
-rw-r--r-- | src/libstore/build/worker.hh | 10 |
3 files changed, 8 insertions, 15 deletions
diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index 3f30c922b..02b22b8ad 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -26,7 +26,6 @@ try { trace("done"); - notify->fulfill(result); cleanup(); co_return std::move(result); diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 0f89159e4..4e8fa38db 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -48,6 +48,10 @@ Worker::~Worker() their destructors). */ children.clear(); + derivationGoals.clear(); + drvOutputSubstitutionGoals.clear(); + substitutionGoals.clear(); + assert(expectedSubstitutions == 0); assert(expectedDownloadSize == 0); assert(expectedNarSize == 0); @@ -71,21 +75,21 @@ std::pair<std::shared_ptr<G>, kj::Promise<Result<Goal::WorkResult>>> Worker::mak auto goal = goal_weak.goal.lock(); 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(); })); + goal_weak.promise = kj::evalLater([goal] { return goal->work(); }).fork(); + childStarted(goal, goal_weak.promise.addBranch()); } else { if (!modify(*goal)) { goal_weak = {}; continue; } } - return {goal, goal_weak.promise->addBranch()}; + return {goal, goal_weak.promise.addBranch()}; } assert(false && "could not make a goal. possible concurrent worker access"); } @@ -224,8 +228,6 @@ void Worker::childStarted(GoalPtr goal, kj::Promise<Result<Goal::WorkResult>> pr .then([this, goal](auto result) { if (result.has_value()) { goalFinished(goal, result.assume_value()); - } else { - goal->notify->fulfill(result.assume_error()); } })); } diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index 6569de6ee..923092b51 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -96,15 +96,7 @@ private: struct CachedGoal { std::weak_ptr<G> goal; - kj::Own<kj::ForkedPromise<Result<Goal::WorkResult>>> promise; - kj::Own<kj::PromiseFulfiller<Result<Goal::WorkResult>>> fulfiller; - - CachedGoal() - { - auto pf = kj::newPromiseAndFulfiller<Result<Goal::WorkResult>>(); - promise = kj::heap(pf.promise.fork()); - fulfiller = std::move(pf.fulfiller); - } + kj::ForkedPromise<Result<Goal::WorkResult>> promise{nullptr}; }; /** * Maps used to prevent multiple instantiations of a goal for the |