diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index 12344a8c2..6c43dcc98 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -563,8 +563,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild() { DerivationBuildingGoal & goal; - DerivationBuildingGoalCallbacks( - DerivationBuildingGoal & goal, std::unique_ptr & builder) + DerivationBuildingGoalCallbacks(DerivationBuildingGoal & goal) : goal{goal} { } @@ -632,15 +631,15 @@ Goal::Co DerivationBuildingGoal::tryToBuild() /* If we have to wait and retry (see below), then `builder` will already be created, so we don't need to create it again. */ - builder = externalBuilder ? makeExternalDerivationBuilder( - *localStoreP, - std::make_unique(*this, builder), - std::move(params), - *externalBuilder) - : makeDerivationBuilder( - *localStoreP, - std::make_unique(*this, builder), - std::move(params)); + builder = + externalBuilder + ? makeExternalDerivationBuilder( + *localStoreP, + std::make_unique(*this), + std::move(params), + *externalBuilder) + : makeDerivationBuilder( + *localStoreP, std::make_unique(*this), std::move(params)); } if (auto builderOutOpt = builder->startBuild()) { diff --git a/src/libstore/include/nix/store/build/derivation-builder.hh b/src/libstore/include/nix/store/build/derivation-builder.hh index f28fd8e56..ca6319855 100644 --- a/src/libstore/include/nix/store/build/derivation-builder.hh +++ b/src/libstore/include/nix/store/build/derivation-builder.hh @@ -189,15 +189,22 @@ struct ExternalBuilder std::vector args; }; +struct DerivationBuilderDeleter +{ + void operator()(DerivationBuilder * builder) noexcept; +}; + +using DerivationBuilderUnique = std::unique_ptr; + #ifndef _WIN32 // TODO enable `DerivationBuilder` on Windows -std::unique_ptr makeDerivationBuilder( +DerivationBuilderUnique makeDerivationBuilder( LocalStore & store, std::unique_ptr miscMethods, DerivationBuilderParams params); /** * @param handler Must be chosen such that it supports the given * derivation. */ -std::unique_ptr makeExternalDerivationBuilder( +DerivationBuilderUnique makeExternalDerivationBuilder( LocalStore & store, std::unique_ptr miscMethods, DerivationBuilderParams params, diff --git a/src/libstore/include/nix/store/build/derivation-building-goal.hh b/src/libstore/include/nix/store/build/derivation-building-goal.hh index be95c796b..e745c12c7 100644 --- a/src/libstore/include/nix/store/build/derivation-building-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-building-goal.hh @@ -5,6 +5,7 @@ #include "nix/store/parsed-derivations.hh" #include "nix/store/derivation-options.hh" #include "nix/store/build/derivation-building-misc.hh" +#include "nix/store/build/derivation-builder.hh" #include "nix/store/outputs-spec.hh" #include "nix/store/store-api.hh" #include "nix/store/pathlocks.hh" @@ -89,7 +90,7 @@ private: */ std::unique_ptr hook; - std::unique_ptr builder; + DerivationBuilderUnique builder; #endif BuildMode buildMode; diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index eb0bf5757..360b6a484 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -98,10 +98,13 @@ public: { } - ~DerivationBuilderImpl() + /** + * Cleanup code to run when destroying any DerivationBuilderImpl implementation. + */ + void cleanupOnDestruction() noexcept { /* Careful: we should never ever throw an exception from a - destructor. */ + noexcept function. */ try { killChild(); } catch (...) { @@ -1962,7 +1965,20 @@ StorePath DerivationBuilderImpl::makeFallbackPath(const StorePath & path) namespace nix { -std::unique_ptr makeDerivationBuilder( +void DerivationBuilderDeleter::operator()(DerivationBuilder * builder) noexcept +{ + if (!builder) /* Idempotent and handles nullptr as any deleter must. */ + return; + + if (auto builderImpl = dynamic_cast(builder)) + /* Note that this might call into virtual functions, which we can't do in a destructor of + the DerivationBuilderImpl itself. */ + builderImpl->cleanupOnDestruction(); + + delete builder; +} + +std::unique_ptr makeDerivationBuilder( LocalStore & store, std::unique_ptr miscMethods, DerivationBuilderParams params) { bool useSandbox = false; @@ -2013,17 +2029,19 @@ std::unique_ptr makeDerivationBuilder( throw Error("feature 'uid-range' is only supported in sandboxed builds"); #ifdef __APPLE__ - return std::make_unique(store, std::move(miscMethods), std::move(params), useSandbox); + return DerivationBuilderUnique( + new DarwinDerivationBuilder(store, std::move(miscMethods), std::move(params), useSandbox)); #elif defined(__linux__) if (useSandbox) - return std::make_unique(store, std::move(miscMethods), std::move(params)); + return DerivationBuilderUnique( + new ChrootLinuxDerivationBuilder(store, std::move(miscMethods), std::move(params))); - return std::make_unique(store, std::move(miscMethods), std::move(params)); + return DerivationBuilderUnique(new LinuxDerivationBuilder(store, std::move(miscMethods), std::move(params))); #else if (useSandbox) throw Error("sandboxing builds is not supported on this platform"); - return std::make_unique(store, std::move(miscMethods), std::move(params)); + return DerivationBuilderUnique(new DerivationBuilderImpl(store, std::move(miscMethods), std::move(params))); #endif } diff --git a/src/libstore/unix/build/external-derivation-builder.cc b/src/libstore/unix/build/external-derivation-builder.cc index 7ddb6e093..6c1a00f91 100644 --- a/src/libstore/unix/build/external-derivation-builder.cc +++ b/src/libstore/unix/build/external-derivation-builder.cc @@ -106,13 +106,14 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl } }; -std::unique_ptr makeExternalDerivationBuilder( +DerivationBuilderUnique makeExternalDerivationBuilder( LocalStore & store, std::unique_ptr miscMethods, DerivationBuilderParams params, const ExternalBuilder & handler) { - return std::make_unique(store, std::move(miscMethods), std::move(params), handler); + return DerivationBuilderUnique( + new ExternalDerivationBuilder(store, std::move(miscMethods), std::move(params), handler)); } } // namespace nix