Fix crash/hang with CA derivations

The curl download can outlive DrvOutputSubstitutionGoal (if some other
error occurs), so at shutdown setting the promise to an exception will
fail because 'this' is no longer valid in the callback. This can
manifest itself as a segfault, "corrupted double-linked list" or hang.

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;