From a5240b23abba2724073f79b79648bf4afb38f70a Mon Sep 17 00:00:00 2001
From: eldritch horrors <pennae@lix.systems>
Date: Wed, 25 Sep 2024 23:57:46 +0200
Subject: [PATCH] libstore: make non-cache goal pointers strong

without circular references we do not need weak goal pointers except for
caches, which should not prevent goal destructors running. caches though
cannot create circular references even when they keep strong references.
if we removed goals from caches when their work() is fully finished, not
when their destructors are run, we could keep strong pointers in caches.
since we do not gain much from this we keep those pointers weak for now.

Change-Id: I1d4a6850ff5e264443c90eb4531da89f5e97a3a0
---
 src/libstore/build/goal.hh   | 7 -------
 src/libstore/build/worker.cc | 8 ++------
 src/libstore/build/worker.hh | 5 +----
 3 files changed, 3 insertions(+), 17 deletions(-)

diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh
index 7933fbc31..f808be160 100644
--- a/src/libstore/build/goal.hh
+++ b/src/libstore/build/goal.hh
@@ -21,7 +21,6 @@ class Worker;
  * A pointer to a goal.
  */
 typedef std::shared_ptr<Goal> GoalPtr;
-typedef std::weak_ptr<Goal> WeakGoalPtr;
 
 struct CompareGoalPtrs {
     bool operator() (const GoalPtr & a, const GoalPtr & b) const;
@@ -31,12 +30,6 @@ struct CompareGoalPtrs {
  * Set of goals.
  */
 typedef std::set<GoalPtr, CompareGoalPtrs> Goals;
-typedef std::set<WeakGoalPtr, std::owner_less<WeakGoalPtr>> WeakGoals;
-
-/**
- * A map of paths to goals (and the other way around).
- */
-typedef std::map<StorePath, WeakGoalPtr> WeakGoalMap;
 
 /**
  * Used as a hint to the worker on how to schedule a particular goal. For example,
diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc
index 1b1bf1d5c..2894620a1 100644
--- a/src/libstore/build/worker.cc
+++ b/src/libstore/build/worker.cc
@@ -44,6 +44,7 @@ Worker::~Worker()
        are in trouble, since goals may call childTerminated() etc. in
        their destructors). */
     topGoals.clear();
+    awake.clear();
     children.clear();
 
     assert(expectedSubstitutions == 0);
@@ -310,12 +311,7 @@ std::vector<GoalPtr> Worker::run(std::function<Targets (GoalFactory &)> req)
         /* Call every wake goal (in the ordering established by
            CompareGoalPtrs). */
         while (!awake.empty() && !topGoals.empty()) {
-            Goals awake2;
-            for (auto & i : awake) {
-                GoalPtr goal = i.lock();
-                if (goal) awake2.insert(goal);
-            }
-            awake.clear();
+            Goals awake2 = std::move(awake);
             for (auto & goal : awake2) {
                 checkInterrupt();
                 auto result = goal->work();
diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh
index 46adaa145..097e73cf7 100644
--- a/src/libstore/build/worker.hh
+++ b/src/libstore/build/worker.hh
@@ -84,9 +84,6 @@ private:
 
     bool running = false;
 
-    /* Note: the worker should only have strong pointers to the
-       top-level goals. */
-
     /**
      * The top-level goals of the worker.
      */
@@ -95,7 +92,7 @@ private:
     /**
      * Goals that are ready to do some work.
      */
-    WeakGoals awake;
+    Goals awake;
 
     template<typename G>
     struct CachedGoal