lix-project/lix
20
57
Fork 31
Code Issues 347 Code Review (Gerrit) Projects 3 Releases Wiki Activity

libstore: remove Goal::notify

Browse source 
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
This commit is contained in:
eldritch horrors 2024-10-05 00:38:35 +02:00
parent 03cbc0ecb9
commit 9adf6f4568
3 changed files with 8 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
src/libstore/build/goal.cc
View file

@ -26,7 +26,6 @@ try {
trace("done");
notify->fulfill(result);
cleanup();
co_return std::move(result);

12
src/libstore/build/worker.cc
View file

@ -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());
}
}));
}

10
src/libstore/build/worker.hh
View file

@ -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