Fix crash if exception thrown during daemon startup

The "system" loggers have different lifecycle to the client loggers.
Client loggers register/unregister themselves with the
MultiplexingLogger on creation/destruction. However "system" loggers did
not, and would leave a dangling pointer in the MultiplexingLogger if
they were destroyed before it was.

Fix is to treat system logger ownership differently from client, i.e.
have MultiplexingLogger own the system logger.

Crash without this patch can be easily reproduced by adding a throw in
the QemuVirtualMachineFactory constructor.

Author: Gerry Boland

include/multipass/logging/multiplexing_logger.h

@@ -31,11 +31,13 @@ namespace logging
 class MultiplexingLogger : public Logger
 {
 public:
+    explicit MultiplexingLogger(UPtr system_logger);
     void log(Level level, CString category, CString message) const override;
     void add_logger(const Logger* logger);
     void remove_logger(const Logger* logger);
 
 private:
+    UPtr system_logger;
     mutable std::shared_timed_mutex mutex;
     std::vector<const Logger*> loggers;
 };

src/daemon/daemon_config.cpp

@@ -64,8 +64,7 @@ std::unique_ptr<const mp::DaemonConfig> mp::DaemonConfigBuilder::build()
     if (logger == nullptr)
         logger = std::make_unique<mpl::StandardLogger>(verbosity_level);
 
-    auto multiplexing_logger = std::make_shared<mpl::MultiplexingLogger>();
-    multiplexing_logger->add_logger(logger.get());
+    auto multiplexing_logger = std::make_shared<mpl::MultiplexingLogger>(std::move(logger));
     mpl::set_logger(multiplexing_logger);
 
     if (cache_directory.isEmpty())
@@ -113,6 +112,6 @@ std::unique_ptr<const mp::DaemonConfig> mp::DaemonConfigBuilder::build()
     return std::unique_ptr<const DaemonConfig>(
         new DaemonConfig{std::move(url_downloader), std::move(factory), std::move(image_hosts), std::move(vault),
                          std::move(name_generator), std::move(ssh_key_provider), std::move(cert_provider),
-                         std::move(client_cert_store), std::move(logger), multiplexing_logger, cache_directory,
+                         std::move(client_cert_store), multiplexing_logger, cache_directory,
                          data_directory, server_address, ssh_username, connection_type, image_refresh_timer});
 }

src/daemon/daemon_config.h

@@ -49,7 +49,6 @@ struct DaemonConfig
     const std::unique_ptr<SSHKeyProvider> ssh_key_provider;
     const std::unique_ptr<CertProvider> cert_provider;
     const std::unique_ptr<CertStore> client_cert_store;
-    const std::unique_ptr<logging::Logger> system_logger;
     const std::shared_ptr<logging::MultiplexingLogger> logger;
     const multipass::Path cache_directory;
     const multipass::Path data_directory;

src/logging/multiplexing_logger.cpp

@@ -21,9 +21,15 @@
 
 namespace mpl = multipass::logging;
 
+multipass::logging::MultiplexingLogger::MultiplexingLogger(UPtr system_logger)
+    : system_logger{std::move(system_logger)}
+{
+}
+
 void mpl::MultiplexingLogger::log(mpl::Level level, CString category, CString message) const
 {
     std::shared_lock<decltype(mutex)> lock{mutex};
+    system_logger->log(level, category, message);
     for (auto logger : loggers)
         logger->log(level, category, message);
 }
@@ -38,4 +44,4 @@ void mpl::MultiplexingLogger::remove_logger(const Logger* logger)
 {
     std::lock_guard<decltype(mutex)> lock{mutex};
     loggers.erase(std::remove(loggers.begin(), loggers.end(), logger), loggers.end());
-}
+}