From 0e5a5303ad29b62c2acb405cd18097cae0d93ce8 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 10 Oct 2024 12:31:34 +0200 Subject: [PATCH 1/6] fix: Ignore Interrupted in recursive-nix daemon worker Otherwise, if checkInterrupt() in any of the supported store operations would catch onto a user interrupt, the exception would bubble to the thread start and be handled by std::terminate(): a crash. --- src/libstore/unix/build/local-derivation-goal.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index 0eda8455f..394eab875 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -65,6 +65,7 @@ #include #include "strings.hh" +#include "signals.hh" namespace nix { @@ -1579,6 +1580,8 @@ void LocalDerivationGoal::startDaemon() FdSink(remote.get()), NotTrusted, daemon::Recursive); debug("terminated daemon connection"); + } catch (const Interrupted &) { + debug("interrupted daemon connection"); } catch (SystemError &) { ignoreExceptionExceptInterrupt(); } From de41e4617523f9355fb896edcf77cc777d45b564 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 10 Oct 2024 12:38:26 +0200 Subject: [PATCH 2/6] Document recursive-nix startDaemon/stopDaemon --- src/libstore/unix/build/local-derivation-goal.hh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libstore/unix/build/local-derivation-goal.hh b/src/libstore/unix/build/local-derivation-goal.hh index 231393308..1ea247661 100644 --- a/src/libstore/unix/build/local-derivation-goal.hh +++ b/src/libstore/unix/build/local-derivation-goal.hh @@ -225,8 +225,15 @@ struct LocalDerivationGoal : public DerivationGoal */ void writeStructuredAttrs(); + /** + * Start an in-process nix daemon thread for recursive-nix. + */ void startDaemon(); + /** + * Stop the in-process nix daemon thread. + * @see startDaemon + */ void stopDaemon(); /** From 3f9ff10786b879a02750780936b50271e675ea57 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 10 Oct 2024 12:46:36 +0200 Subject: [PATCH 3/6] ThreadPool: catch Interrupted --- src/libutil/thread-pool.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libutil/thread-pool.cc b/src/libutil/thread-pool.cc index 0355e1f07..57172da7e 100644 --- a/src/libutil/thread-pool.cc +++ b/src/libutil/thread-pool.cc @@ -110,6 +110,11 @@ void ThreadPool::doWork(bool mainThread) propagate it. */ try { std::rethrow_exception(exc); + } catch (const Interrupted &) { + // The interrupted state may be picked up multiple + // workers, which is expected, so we should ignore + // it silently and let the first one bubble up, + // rethrown via the original state->exception. } catch (std::exception & e) { if (!dynamic_cast(&e)) ignoreExceptionExceptInterrupt(); From 16320f6d24222c1cf9cb718f24628e4f1f5bf889 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 10 Oct 2024 13:08:26 +0200 Subject: [PATCH 4/6] Handle ThreadPoolShutdown with normal catch --- src/libutil/thread-pool.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libutil/thread-pool.cc b/src/libutil/thread-pool.cc index 57172da7e..cc76b1d37 100644 --- a/src/libutil/thread-pool.cc +++ b/src/libutil/thread-pool.cc @@ -115,9 +115,10 @@ void ThreadPool::doWork(bool mainThread) // workers, which is expected, so we should ignore // it silently and let the first one bubble up, // rethrown via the original state->exception. + } catch (const ThreadPoolShutDown &) { + // Similarly expected. } catch (std::exception & e) { - if (!dynamic_cast(&e)) - ignoreExceptionExceptInterrupt(); + ignoreExceptionExceptInterrupt(); } catch (...) { } } From fd8a4a86d9b6672584cb4b3d7d8d9d94790c55a3 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 10 Oct 2024 13:09:12 +0200 Subject: [PATCH 5/6] ThreadPool: don't silently ignore non-std exceptions Introduced in 8f6b347abd without explanation. Throwing anything that's not that is a programming mistake that we don't want to ignore silently. A crash would be ok, because that means we/they can fix the offending throw. --- src/libutil/thread-pool.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libutil/thread-pool.cc b/src/libutil/thread-pool.cc index cc76b1d37..75424cd0e 100644 --- a/src/libutil/thread-pool.cc +++ b/src/libutil/thread-pool.cc @@ -119,7 +119,6 @@ void ThreadPool::doWork(bool mainThread) // Similarly expected. } catch (std::exception & e) { ignoreExceptionExceptInterrupt(); - } catch (...) { } } } From ed184f0b612401c7a70a9a736ad67fa13a79a45f Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 16 Oct 2024 19:40:45 +0200 Subject: [PATCH 6/6] Typo Co-authored-by: Cole Helbling --- src/libutil/thread-pool.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/thread-pool.cc b/src/libutil/thread-pool.cc index 75424cd0e..0725c1926 100644 --- a/src/libutil/thread-pool.cc +++ b/src/libutil/thread-pool.cc @@ -111,7 +111,7 @@ void ThreadPool::doWork(bool mainThread) try { std::rethrow_exception(exc); } catch (const Interrupted &) { - // The interrupted state may be picked up multiple + // The interrupted state may be picked up by multiple // workers, which is expected, so we should ignore // it silently and let the first one bubble up, // rethrown via the original state->exception.