From 724b345eb985404065a930304b82112a80ee92d3 Mon Sep 17 00:00:00 2001
From: eldritch horrors <pennae@lix.systems>
Date: Fri, 26 Jul 2024 14:07:29 +0200
Subject: [PATCH] libstore: encapsulate worker build hook state

once goals run on multiple threads these fields must by synchronized as
one, or we try to run build hooks to often (or worse, not often enough)

Change-Id: I47860e46fe5c6db41755b2a3a1d9dbb5701c4ca4
---
 src/libstore/build/derivation-goal.cc | 24 ++++++++++++------------
 src/libstore/build/worker.hh          | 18 +++++++++++-------
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
index 38c54e854..975a0658d 100644
--- a/src/libstore/build/derivation-goal.cc
+++ b/src/libstore/build/derivation-goal.cc
@@ -1127,21 +1127,21 @@ void DerivationGoal::resolvedFinished()
 
 HookReply DerivationGoal::tryBuildHook()
 {
-    if (!worker.tryBuildHook || !useDerivation) return rpDecline;
+    if (!worker.hook.available || !useDerivation) return rpDecline;
 
-    if (!worker.hook)
-        worker.hook = std::make_unique<HookInstance>();
+    if (!worker.hook.instance)
+        worker.hook.instance = std::make_unique<HookInstance>();
 
     try {
 
         /* Send the request to the hook. */
-        worker.hook->sink
+        worker.hook.instance->sink
             << "try"
             << (worker.getNrLocalBuilds() < settings.maxBuildJobs ? 1 : 0)
             << drv->platform
             << worker.store.printStorePath(drvPath)
             << parsedDrv->getRequiredSystemFeatures();
-        worker.hook->sink.flush();
+        worker.hook.instance->sink.flush();
 
         /* Read the first line of input, which should be a word indicating
            whether the hook wishes to perform the build. */
@@ -1149,13 +1149,13 @@ HookReply DerivationGoal::tryBuildHook()
         while (true) {
             auto s = [&]() {
                 try {
-                    return readLine(worker.hook->fromHook.readSide.get());
+                    return readLine(worker.hook.instance->fromHook.readSide.get());
                 } catch (Error & e) {
                     e.addTrace({}, "while reading the response from the build hook");
                     throw;
                 }
             }();
-            if (handleJSONLogMessage(s, worker.act, worker.hook->activities, true))
+            if (handleJSONLogMessage(s, worker.act, worker.hook.instance->activities, true))
                 ;
             else if (s.substr(0, 2) == "# ") {
                 reply = s.substr(2);
@@ -1172,8 +1172,8 @@ HookReply DerivationGoal::tryBuildHook()
         if (reply == "decline")
             return rpDecline;
         else if (reply == "decline-permanently") {
-            worker.tryBuildHook = false;
-            worker.hook = 0;
+            worker.hook.available = false;
+            worker.hook.instance.reset();
             return rpDecline;
         }
         else if (reply == "postpone")
@@ -1185,14 +1185,14 @@ HookReply DerivationGoal::tryBuildHook()
         if (e.errNo == EPIPE) {
             printError(
                 "build hook died unexpectedly: %s",
-                chomp(drainFD(worker.hook->fromHook.readSide.get())));
-            worker.hook = 0;
+                chomp(drainFD(worker.hook.instance->fromHook.readSide.get())));
+            worker.hook.instance.reset();
             return rpDecline;
         } else
             throw;
     }
 
-    hook = std::move(worker.hook);
+    hook = std::move(worker.hook.instance);
 
     try {
         machineName = readLine(hook->fromHook.readSide.get());
diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh
index 3984c9c1c..903380c45 100644
--- a/src/libstore/build/worker.hh
+++ b/src/libstore/build/worker.hh
@@ -135,7 +135,17 @@ public:
     Store & store;
     Store & evalStore;
 
-    std::unique_ptr<HookInstance> hook;
+    struct HookState {
+        std::unique_ptr<HookInstance> instance;
+
+        /**
+         * Whether to ask the build hook if it can build a derivation. If
+         * it answers with "decline-permanently", we don't try again.
+         */
+        bool available = true;
+    };
+
+    HookState hook;
 
     uint64_t expectedBuilds = 0;
     uint64_t doneBuilds = 0;
@@ -151,12 +161,6 @@ public:
     uint64_t expectedNarSize = 0;
     uint64_t doneNarSize = 0;
 
-    /**
-     * Whether to ask the build hook if it can build a derivation. If
-     * it answers with "decline-permanently", we don't try again.
-     */
-    bool tryBuildHook = true;
-
     Worker(Store & store, Store & evalStore);
     ~Worker();