diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index fb90403a0..88a829cc0 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -60,7 +60,7 @@ static bool allSupportedLocally(Store & store, const std::set& requ static int main_build_remote(int argc, char * * argv) { { - logger = makeJSONLogger(*logger); + logger = makeJSONLogger(std::move(logger)); /* Ensure we don't get any SSH passphrase or host key popups. */ unsetenv("DISPLAY"); diff --git a/src/libmain/loggers.cc b/src/libmain/loggers.cc index 8c3c4e355..85ffe4a8a 100644 --- a/src/libmain/loggers.cc +++ b/src/libmain/loggers.cc @@ -24,14 +24,14 @@ LogFormat parseLogFormat(const std::string & logFormatStr) { throw Error("option 'log-format' has an invalid value '%s'", logFormatStr); } -Logger * makeDefaultLogger() { +std::unique_ptr makeDefaultLogger() { switch (defaultLogFormat) { case LogFormat::raw: return makeSimpleLogger(false); case LogFormat::rawWithLogs: return makeSimpleLogger(true); case LogFormat::internalJSON: - return makeJSONLogger(*makeSimpleLogger(true)); + return makeJSONLogger(makeSimpleLogger(true)); case LogFormat::bar: return makeProgressBar(); case LogFormat::barWithLogs: { diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index e36bc0b01..3151ac9ec 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -44,17 +44,20 @@ static std::string_view storePathToName(std::string_view path) ProgressBar::ProgressBar(bool isTTY) : isTTY(isTTY) { - state_.lock()->active = isTTY; - updateThread = std::thread([&]() { - auto state(state_.lock()); - auto nextWakeup = A_LONG_TIME; - while (state->active) { - if (!state->haveUpdate) - state.wait_for(updateCV, nextWakeup); - nextWakeup = draw(*state, {}); - state.wait_for(quitCV, std::chrono::milliseconds(50)); - } - }); + if ((state_.lock()->active = isTTY)) { + // Only start the update thread when the logger is set to active. + // Otherwise, the destructor will std::terminate trying to destroy a joinable thread. + updateThread = std::thread([&]() { + auto state(state_.lock()); + auto nextWakeup = A_LONG_TIME; + while (state->active) { + if (!state->haveUpdate) + state.wait_for(updateCV, nextWakeup); + nextWakeup = draw(*state, {}); + state.wait_for(quitCV, std::chrono::milliseconds(50)); + } + }); + } } ProgressBar::~ProgressBar() @@ -554,9 +557,9 @@ void ProgressBar::setPrintMultiline(bool printMultiline) this->printMultiline = printMultiline; } -Logger * makeProgressBar() +std::unique_ptr makeProgressBar() { - return new ProgressBar(shouldANSI()); + return std::make_unique(shouldANSI()); } void startProgressBar() @@ -566,7 +569,7 @@ void startProgressBar() void stopProgressBar() { - auto progressBar = dynamic_cast(logger); + auto progressBar = dynamic_cast(logger.get()); if (progressBar) progressBar->stop(); } diff --git a/src/libmain/progress-bar.hh b/src/libmain/progress-bar.hh index e682d75fe..bf05c89de 100644 --- a/src/libmain/progress-bar.hh +++ b/src/libmain/progress-bar.hh @@ -111,7 +111,7 @@ struct ProgressBar : public Logger void setPrintMultiline(bool printMultiline) override; }; -Logger * makeProgressBar(); +std::unique_ptr makeProgressBar(); void startProgressBar(); diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index c7d7d06d6..acc8f7cf1 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2636,7 +2636,7 @@ void LocalDerivationGoal::runChild() /* Execute the program. This should not return. */ if (drv->isBuiltin()) { try { - logger = makeJSONLogger(*logger); + logger = makeJSONLogger(std::move(logger)); BasicDerivation & drv2(*drv); for (auto & e : drv2.env) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 6d64644d1..d96d92d01 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -991,11 +991,13 @@ void processConnection( if (clientVersion < MIN_SUPPORTED_WORKER_PROTO_VERSION) throw Error("the Nix client version is too old"); - auto tunnelLogger = new TunnelLogger(to, clientVersion); - auto prevLogger = nix::logger; + std::unique_ptr savedPrevLogger; + auto prevLogger = nix::logger.get(); + auto ownedTunnelLogger = std::make_unique(to, clientVersion); + auto tunnelLogger = ownedTunnelLogger.get(); // FIXME if (!recursive) - logger = tunnelLogger; + savedPrevLogger = std::exchange(logger, std::move(ownedTunnelLogger)); unsigned int opCount = 0; diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index fecbc89ac..ccd1f1ea7 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -27,7 +27,7 @@ void setCurActivity(const ActivityId activityId) curActivity = activityId; } -Logger * logger = makeSimpleLogger(true); +std::unique_ptr logger = makeSimpleLogger(true); void Logger::warn(const std::string & msg) { @@ -129,9 +129,9 @@ void writeToStderr(std::string_view s) } } -Logger * makeSimpleLogger(bool printBuildLogs) +std::unique_ptr makeSimpleLogger(bool printBuildLogs) { - return new SimpleLogger(printBuildLogs); + return std::make_unique(printBuildLogs); } std::atomic nextId{0}; @@ -159,9 +159,9 @@ void to_json(nlohmann::json & json, std::shared_ptr pos) } struct JSONLogger : Logger { - Logger & prevLogger; + std::unique_ptr prevLogger; - JSONLogger(Logger & prevLogger) : prevLogger(prevLogger) { } + JSONLogger(std::unique_ptr prevLogger) : prevLogger(std::move(prevLogger)) { } bool isVerbose() override { return true; @@ -182,7 +182,7 @@ struct JSONLogger : Logger { void write(const nlohmann::json & json) { - prevLogger.log(lvlError, "@nix " + json.dump(-1, ' ', false, nlohmann::json::error_handler_t::replace)); + prevLogger->log(lvlError, "@nix " + json.dump(-1, ' ', false, nlohmann::json::error_handler_t::replace)); } void log(Verbosity lvl, std::string_view s) override @@ -254,9 +254,9 @@ struct JSONLogger : Logger { } }; -Logger * makeJSONLogger(Logger & prevLogger) +std::unique_ptr makeJSONLogger(std::unique_ptr prevLogger) { - return new JSONLogger(prevLogger); + return std::make_unique(std::move(prevLogger)); } static Logger::Fields getFields(nlohmann::json & json) diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index 3cead4296..6089f1585 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -227,11 +227,11 @@ struct PushActivity ~PushActivity() { setCurActivity(prevAct); } }; -extern Logger * logger; +extern std::unique_ptr logger; -Logger * makeSimpleLogger(bool printBuildLogs = true); +std::unique_ptr makeSimpleLogger(bool printBuildLogs = true); -Logger * makeJSONLogger(Logger & prevLogger); +std::unique_ptr makeJSONLogger(std::unique_ptr prevLogger); /** * suppress msgs > this diff --git a/tests/unit/libexpr/primops.cc b/tests/unit/libexpr/primops.cc index bd174a6c0..f623cc896 100644 --- a/tests/unit/libexpr/primops.cc +++ b/tests/unit/libexpr/primops.cc @@ -27,20 +27,17 @@ namespace nix { }; class CaptureLogging { - Logger * oldLogger; - std::unique_ptr tempLogger; + std::unique_ptr oldLogger; public: - CaptureLogging() : tempLogger(std::make_unique()) { - oldLogger = logger; - logger = tempLogger.get(); - } + CaptureLogging() : oldLogger(std::exchange(logger, std::make_unique())) {} ~CaptureLogging() { - logger = oldLogger; + logger = std::move(oldLogger); } std::string get() const { - return tempLogger->get(); + auto captureLogger = dynamic_cast(logger.get()); + return captureLogger ? captureLogger->get() : ""; } }; diff --git a/tests/unit/libmain/progress-bar.cc b/tests/unit/libmain/progress-bar.cc index e44a8b37e..778256d0b 100644 --- a/tests/unit/libmain/progress-bar.cc +++ b/tests/unit/libmain/progress-bar.cc @@ -24,7 +24,7 @@ namespace nix initGC(); startProgressBar(); - ASSERT_NE(dynamic_cast(logger), nullptr); + ASSERT_NE(dynamic_cast(logger.get()), nullptr); ProgressBar & progressBar = dynamic_cast(*logger); Activity act(