diff --git a/src/libexpr-tests/primops.cc b/src/libexpr-tests/primops.cc index 5b5898237..2bf726477 100644 --- a/src/libexpr-tests/primops.cc +++ b/src/libexpr-tests/primops.cc @@ -28,20 +28,15 @@ 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::move(logger); + logger = std::make_unique(); } ~CaptureLogging() { - logger = oldLogger; - } - - std::string get() const { - return tempLogger->get(); + logger = std::move(oldLogger); } }; @@ -113,7 +108,7 @@ namespace nix { CaptureLogging l; auto v = eval("builtins.trace \"test string 123\" 123"); ASSERT_THAT(v, IsIntEq(123)); - auto text = l.get(); + auto text = (dynamic_cast(logger.get()))->get(); ASSERT_NE(text.find("test string 123"), std::string::npos); } diff --git a/src/libmain/loggers.cc b/src/libmain/loggers.cc index 83a87bdf3..836ea3dc8 100644 --- a/src/libmain/loggers.cc +++ b/src/libmain/loggers.cc @@ -6,7 +6,8 @@ namespace nix { LogFormat defaultLogFormat = LogFormat::raw; -LogFormat parseLogFormat(const std::string & logFormatStr) { +LogFormat parseLogFormat(const std::string & logFormatStr) +{ if (logFormatStr == "raw" || getEnv("NIX_GET_COMPLETIONS")) return LogFormat::raw; else if (logFormatStr == "raw-with-logs") @@ -20,7 +21,8 @@ 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); @@ -40,16 +42,19 @@ Logger * makeDefaultLogger() { } } -void setLogFormat(const std::string & logFormatStr) { +void setLogFormat(const std::string & logFormatStr) +{ setLogFormat(parseLogFormat(logFormatStr)); } -void setLogFormat(const LogFormat & logFormat) { +void setLogFormat(const LogFormat & logFormat) +{ defaultLogFormat = logFormat; createDefaultLogger(); } -void createDefaultLogger() { +void createDefaultLogger() +{ logger = makeDefaultLogger(); } diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index 961850b58..17109b57e 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -117,13 +117,15 @@ public: { { auto state(state_.lock()); - if (!state->active) return; - state->active = false; - writeToStderr("\r\e[K"); - updateCV.notify_one(); - quitCV.notify_one(); + if (state->active) { + state->active = false; + writeToStderr("\r\e[K"); + updateCV.notify_one(); + quitCV.notify_one(); + } } - updateThread.join(); + if (updateThread.joinable()) + updateThread.join(); } void pause() override { @@ -553,9 +555,9 @@ public: } }; -Logger * makeProgressBar() +std::unique_ptr makeProgressBar() { - return new ProgressBar(isTTY()); + return std::make_unique(isTTY()); } void startProgressBar() @@ -565,9 +567,8 @@ void startProgressBar() void stopProgressBar() { - auto progressBar = dynamic_cast(logger); - if (progressBar) progressBar->stop(); - + if (auto progressBar = dynamic_cast(logger.get())) + progressBar->stop(); } } diff --git a/src/libmain/progress-bar.hh b/src/libmain/progress-bar.hh index c3c6e3833..83209e863 100644 --- a/src/libmain/progress-bar.hh +++ b/src/libmain/progress-bar.hh @@ -5,7 +5,7 @@ namespace nix { -Logger * makeProgressBar(); +std::unique_ptr makeProgressBar(); void startProgressBar(); diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index d6745f516..60cb64b7b 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -1041,11 +1041,15 @@ void processConnection( conn.protoVersion = protoVersion; conn.features = features; - auto tunnelLogger = new TunnelLogger(conn.to, protoVersion); - auto prevLogger = nix::logger; + auto tunnelLogger_ = std::make_unique(conn.to, protoVersion); + auto tunnelLogger = tunnelLogger_.get(); + std::unique_ptr prevLogger_; + auto prevLogger = logger.get(); // FIXME - if (!recursive) - logger = tunnelLogger; + if (!recursive) { + prevLogger_ = std::move(logger); + logger = std::move(tunnelLogger_); + } unsigned int opCount = 0; diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 9caa83efe..deeae120a 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -29,7 +29,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) { @@ -128,9 +128,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}; @@ -262,9 +262,9 @@ struct JSONLogger : Logger { } }; -Logger * makeJSONLogger(Descriptor fd) +std::unique_ptr makeJSONLogger(Descriptor fd) { - return new JSONLogger(fd); + return std::make_unique(fd); } static Logger::Fields getFields(nlohmann::json & json) diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index e8112c6b0..474283894 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -180,11 +180,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(Descriptor fd); +std::unique_ptr makeJSONLogger(Descriptor fd); /** * @param source A noun phrase describing the source of the message, e.g. "the builder". diff --git a/src/libutil/unix/processes.cc b/src/libutil/unix/processes.cc index 43d9179d9..a9df1be60 100644 --- a/src/libutil/unix/processes.cc +++ b/src/libutil/unix/processes.cc @@ -200,8 +200,15 @@ static int childEntry(void * arg) pid_t startProcess(std::function fun, const ProcessOptions & options) { ChildWrapperFunction wrapper = [&] { - if (!options.allowVfork) + if (!options.allowVfork) { + /* Set a simple logger, while releasing (not destroying) + the parent logger. We don't want to run the parent + logger's destructor since that will crash (e.g. when + ~ProgressBar() tries to join a thread that doesn't + exist. */ + logger.release(); logger = makeSimpleLogger(); + } try { #if __linux__ if (options.dieWithParent && prctl(PR_SET_PDEATHSIG, SIGKILL) == -1) diff --git a/src/nix/main.cc b/src/nix/main.cc index 80ef53084..a8e3ed7ac 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -388,8 +388,6 @@ void mainWrapped(int argc, char * * argv) } #endif - Finally f([] { logger->stop(); }); - programPath = argv[0]; auto programName = std::string(baseNameOf(programPath)); auto extensionPos = programName.find_last_of(".");