Merge pull request #7994 from edolstra/fix-ca-crash

Fix crash/hang with CA derivations

2 files changed, 22 insertions, 13 deletions

diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc
index b7f7b5ab1..b30957c84 100644
--- a/src/libstore/build/drv-output-substitution-goal.cc
+++ b/src/libstore/build/drv-output-substitution-goal.cc
@@ -61,20 +61,25 @@ void DrvOutputSubstitutionGoal::tryNext()
 
     // FIXME: Make async
     // outputInfo = sub->queryRealisation(id);
-    outPipe.create();
-    promise = decltype(promise)();
+
+    /* The callback of the curl download below can outlive `this` (if
+       some other error occurs), so it must not touch `this`. So put
+       the shared state in a separate refcounted object. */
+    downloadState = std::make_shared<DownloadState>();
+    downloadState->outPipe.create();
 
     sub->queryRealisation(
-        id, { [&](std::future<std::shared_ptr<const Realisation>> res) {
+        id,
+        { [downloadState(downloadState)](std::future<std::shared_ptr<const Realisation>> res) {
             try {
-                Finally updateStats([this]() { outPipe.writeSide.close(); });
-                promise.set_value(res.get());
+                Finally updateStats([&]() { downloadState->outPipe.writeSide.close(); });
+                downloadState->promise.set_value(res.get());
             } catch (...) {
-                promise.set_exception(std::current_exception());
+                downloadState->promise.set_exception(std::current_exception());
             }
         } });
 
-    worker.childStarted(shared_from_this(), {outPipe.readSide.get()}, true, false);
+    worker.childStarted(shared_from_this(), {downloadState->outPipe.readSide.get()}, true, false);
 
     state = &DrvOutputSubstitutionGoal::realisationFetched;
 }
@@ -84,7 +89,7 @@ void DrvOutputSubstitutionGoal::realisationFetched()
     worker.childTerminated(this);
 
     try {
-        outputInfo = promise.get_future().get();
+        outputInfo = downloadState->promise.get_future().get();
     } catch (std::exception & e) {
         printError(e.what());
         substituterFailed = true;
@@ -155,7 +160,7 @@ void DrvOutputSubstitutionGoal::work()
 
 void DrvOutputSubstitutionGoal::handleEOF(int fd)
 {
-    if (fd == outPipe.readSide.get()) worker.wakeUp(shared_from_this());
+    if (fd == downloadState->outPipe.readSide.get()) worker.wakeUp(shared_from_this());
 }
 
 
diff --git a/src/libstore/build/drv-output-substitution-goal.hh b/src/libstore/build/drv-output-substitution-goal.hh
index 948dbda8f..e4b044790 100644
--- a/src/libstore/build/drv-output-substitution-goal.hh
+++ b/src/libstore/build/drv-output-substitution-goal.hh
@@ -16,7 +16,7 @@ class Worker;
 // 2. Substitute the corresponding output path
 // 3. Register the output info
 class DrvOutputSubstitutionGoal : public Goal {
-private:
+
     // The drv output we're trying to substitue
     DrvOutput id;
 
@@ -30,9 +30,13 @@ private:
     /* The current substituter. */
     std::shared_ptr<Store> sub;
 
-    Pipe outPipe;
-    std::thread thr;
-    std::promise<std::shared_ptr<const Realisation>> promise;
+    struct DownloadState
+    {
+        Pipe outPipe;
+        std::promise<std::shared_ptr<const Realisation>> promise;
+    };
+
+    std::shared_ptr<DownloadState> downloadState;
 
     /* Whether a substituter failed. */
     bool substituterFailed = false;