aboutsummaryrefslogtreecommitdiff
path: root/src/libstore/build/drv-output-substitution-goal.cc
diff options
context:
space:
mode:
authorEelco Dolstra <edolstra@gmail.com>2023-03-07 13:51:02 +0100
committerEelco Dolstra <edolstra@gmail.com>2023-03-08 11:09:15 +0100
commit7bfed34367f49ea2d4fe52e5861e5969fc9f4b59 (patch)
treec5316dd6b8ae983cafeb1fec342118abf6acb5e1 /src/libstore/build/drv-output-substitution-goal.cc
parentba0486f045d4f7f304bd8c4a939ca2e658affcc8 (diff)
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.
Diffstat (limited to 'src/libstore/build/drv-output-substitution-goal.cc')
-rw-r--r--src/libstore/build/drv-output-substitution-goal.cc23
1 files changed, 14 insertions, 9 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());
}