lix-project/lix
20
57
Fork 31
Code Issues 350 Code Review (Gerrit) Projects 3 Releases Wiki Activity

Fix crash/hang with CA derivations

Browse source 
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.
This commit is contained in:
Eelco Dolstra 2023-03-07 13:51:02 +01:00
parent ba0486f045
commit 7bfed34367
2 changed files with 22 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
src/libstore/build/drv-output-substitution-goal.cc
View file

@ -61,20 +61,25 @@ void DrvOutputSubstitutionGoal::tryNext()
// FIXME: Make async // FIXME: Make async
// outputInfo = sub->queryRealisation(id); // 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( sub->queryRealisation(
id, { [&](std::future<std::shared_ptr<const Realisation>> res) { id,
{ [downloadState(downloadState)](std::future<std::shared_ptr<const Realisation>> res) {
try { try {
Finally updateStats([this]() { outPipe.writeSide.close(); }); Finally updateStats([&]() { downloadState->outPipe.writeSide.close(); });
promise.set_value(res.get()); downloadState->promise.set_value(res.get());
} catch (...) { } 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; state = &DrvOutputSubstitutionGoal::realisationFetched;
} }
@ -84,7 +89,7 @@ void DrvOutputSubstitutionGoal::realisationFetched()
worker.childTerminated(this); worker.childTerminated(this);
try { try {
outputInfo = promise.get_future().get(); outputInfo = downloadState->promise.get_future().get();
} catch (std::exception & e) { } catch (std::exception & e) {
printError(e.what()); printError(e.what());
substituterFailed = true; substituterFailed = true;
@ -155,7 +160,7 @@ void DrvOutputSubstitutionGoal::work()
void DrvOutputSubstitutionGoal::handleEOF(int fd) 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());
} }

8
src/libstore/build/drv-output-substitution-goal.hh
View file

@ -16,7 +16,7 @@ class Worker;
// 2. Substitute the corresponding output path // 2. Substitute the corresponding output path
// 3. Register the output info // 3. Register the output info
class DrvOutputSubstitutionGoal : public Goal { class DrvOutputSubstitutionGoal : public Goal {
private:
// The drv output we're trying to substitue // The drv output we're trying to substitue
DrvOutput id; DrvOutput id;
@ -30,9 +30,13 @@ private:
/* The current substituter. */ /* The current substituter. */
std::shared_ptr<Store> sub; std::shared_ptr<Store> sub;
struct DownloadState
{
Pipe outPipe; Pipe outPipe;
std::thread thr;
std::promise<std::shared_ptr<const Realisation>> promise; std::promise<std::shared_ptr<const Realisation>> promise;
};
std::shared_ptr<DownloadState> downloadState;
/* Whether a substituter failed. */ /* Whether a substituter failed. */
bool substituterFailed = false; bool substituterFailed = false;