From 99edc2ae38b533ecd742c172a3531cc8958c4be5 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sat, 5 Oct 2024 00:38:35 +0200 Subject: [PATCH] libstore: check for interrupts in parallel promise this simplifies the worker loop, and lets us remove it entirely later. note that ideally only one promise waiting for interrupts should exist in the entire system. not one per event loop, one per *process*. extra interrupt waiters make interrupt response nondeterministic and as such aren't great for user experience. if anything wants to react to aborts caused by explicit interruptions, or anything else, those things would be much better served using RAII guards such as Finally (or KJ_DEFER). Change-Id: I41d035ff40172d536e098153c7375b0972110d51 --- src/libstore/build/worker.cc | 11 +++++++---- src/libutil/signals.cc | 7 ++++++- src/libutil/signals.hh | 3 +++ 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 0ca805b4d..0e8694a6d 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -294,7 +294,13 @@ std::vector Worker::run(std::function req) topGoals.insert(goal); } - auto promise = runImpl().exclusiveJoin(updateStatistics()); + auto onInterrupt = kj::newPromiseAndCrossThreadFulfiller>(); + auto interruptCallback = createInterruptCallback([&] { + return result::failure(std::make_exception_ptr(makeInterrupted())); + }); + + auto promise = + runImpl().exclusiveJoin(updateStatistics()).exclusiveJoin(std::move(onInterrupt.promise)); // TODO GC interface? if (auto localStore = dynamic_cast(&store); localStore && settings.minFree != 0) { @@ -316,9 +322,6 @@ try { debug("entered goal loop"); while (1) { - - checkInterrupt(); - if (topGoals.empty()) break; /* Wait for input. */ diff --git a/src/libutil/signals.cc b/src/libutil/signals.cc index 4e9ed0ba1..dac2964ae 100644 --- a/src/libutil/signals.cc +++ b/src/libutil/signals.cc @@ -12,13 +12,18 @@ std::atomic _isInterrupted = false; thread_local std::function interruptCheck; +Interrupted makeInterrupted() +{ + return Interrupted("interrupted by the user"); +} + void _interrupted() { /* Block user interrupts while an exception is being handled. Throwing an exception while another exception is being handled kills the program! */ if (!std::uncaught_exceptions()) { - throw Interrupted("interrupted by the user"); + throw makeInterrupted(); } } diff --git a/src/libutil/signals.hh b/src/libutil/signals.hh index 02f8d2ca3..538ff94b4 100644 --- a/src/libutil/signals.hh +++ b/src/libutil/signals.hh @@ -16,10 +16,13 @@ namespace nix { /* User interruption. */ +class Interrupted; + extern std::atomic _isInterrupted; extern thread_local std::function interruptCheck; +Interrupted makeInterrupted(); void _interrupted(); void inline checkInterrupt()