From 49da508f46a1a90b04faed88a7d865976ae7c6fb Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 28 Aug 2025 12:40:55 -0400 Subject: [PATCH] Write a destructor for `DerivationBuilderImpl` This allows `DerivationBuildingGoal` to know less. --- .../build/derivation-building-goal.cc | 14 ++------ .../nix/store/build/derivation-builder.hh | 14 -------- src/libstore/unix/build/derivation-builder.cc | 36 ++++++++++++++++--- 3 files changed, 33 insertions(+), 31 deletions(-) diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index 24244ebd4..61f726bf9 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -59,18 +59,8 @@ DerivationBuildingGoal::~DerivationBuildingGoal() ignoreExceptionInDestructor(); } #ifndef _WIN32 // TODO enable `DerivationBuilder` on Windows - if (builder) { - try { - builder->stopDaemon(); - } catch (...) { - ignoreExceptionInDestructor(); - } - try { - builder->cleanupBuild(false); - } catch (...) { - ignoreExceptionInDestructor(); - } - } + if (builder) + builder.reset(); #endif try { closeLogFile(); diff --git a/src/libstore/include/nix/store/build/derivation-builder.hh b/src/libstore/include/nix/store/build/derivation-builder.hh index 512d001e0..65d044a79 100644 --- a/src/libstore/include/nix/store/build/derivation-builder.hh +++ b/src/libstore/include/nix/store/build/derivation-builder.hh @@ -192,20 +192,6 @@ struct DerivationBuilder : RestrictionContext */ virtual SingleDrvOutputs unprepareBuild() = 0; - /** - * Stop the in-process nix daemon thread. - * @see startDaemon - */ - virtual void stopDaemon() = 0; - - /** - * Delete the temporary directory, if we have one. - * - * @param force We know the build suceeded, so don't attempt to - * preseve anything for debugging. - */ - virtual void cleanupBuild(bool force) = 0; - /** * Kill any processes running under the build user UID or in the * cgroup of the build. diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 241d98ace..4678dae42 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -85,6 +85,22 @@ public: { } + ~DerivationBuilderImpl() + { + /* Careful: we should never ever throw an exception from a + destructor. */ + try { + stopDaemon(); + } catch (...) { + ignoreExceptionInDestructor(); + } + try { + cleanupBuild(false); + } catch (...) { + ignoreExceptionInDestructor(); + } + } + protected: /** @@ -285,9 +301,11 @@ private: */ void startDaemon(); -public: - - void stopDaemon() override; + /** + * Stop the in-process nix daemon thread. + * @see startDaemon + */ + void stopDaemon(); protected: @@ -342,9 +360,17 @@ private: */ SingleDrvOutputs registerOutputs(); -public: +protected: - void cleanupBuild(bool force) override; + /** + * Delete the temporary directory, if we have one. + * + * @param force We know the build suceeded, so don't attempt to + * preseve anything for debugging. + */ + virtual void cleanupBuild(bool force); + +public: void killSandbox(bool getStats) override;