Merge pull request #13620 from JosJuice/logmanager-cleanup

LogManager: Stop using manual memory management

Source/Core/Common/Logging/LogManager.cpp

@@ -8,6 +8,7 @@
 #include <cstdarg>
 #include <cstring>
 #include <locale>
+#include <memory>
 #include <mutex>
 #include <ostream>
 #include <string>
@@ -151,8 +152,8 @@ LogManager::LogManager()
   m_log[LogType::WII_IPC] = {"WII_IPC", "WII IPC"};
 
   RegisterListener(LogListener::FILE_LISTENER,
-                   new FileLogListener(File::GetUserPath(F_MAINLOG_IDX)));
-  RegisterListener(LogListener::CONSOLE_LISTENER, new ConsoleListener());
+                   std::make_unique<FileLogListener>(File::GetUserPath(F_MAINLOG_IDX)));
+  RegisterListener(LogListener::CONSOLE_LISTENER, std::make_unique<ConsoleListener>());
 
   // Set up log listeners
   LogLevel verbosity = Config::Get(LOGGER_VERBOSITY);
@@ -171,12 +172,7 @@ LogManager::LogManager()
   m_path_cutoff_point = DeterminePathCutOffPoint();
 }
 
-LogManager::~LogManager()
-{
-  // The log window listener pointer is owned by the GUI code.
-  delete m_listeners[LogListener::CONSOLE_LISTENER];
-  delete m_listeners[LogListener::FILE_LISTENER];
-}
+LogManager::~LogManager() = default;
 
 void LogManager::SaveSettings()
 {
@@ -273,9 +269,9 @@ const char* LogManager::GetFullName(LogType type) const
   return m_log[type].m_full_name;
 }
 
-void LogManager::RegisterListener(LogListener::LISTENER id, LogListener* listener)
+void LogManager::RegisterListener(LogListener::LISTENER id, std::unique_ptr<LogListener> listener)
 {
-  m_listeners[id] = listener;
+  m_listeners[id] = std::move(listener);
 }
 
 void LogManager::EnableListener(LogListener::LISTENER id, bool enable)
@@ -289,23 +285,22 @@ bool LogManager::IsListenerEnabled(LogListener::LISTENER id) const
 }
 
 // Singleton. Ugh.
-static LogManager* s_log_manager;
+static std::unique_ptr<LogManager> s_log_manager;
 
 LogManager* LogManager::GetInstance()
 {
-  return s_log_manager;
+  return s_log_manager.get();
 }
 
 void LogManager::Init()
 {
-  s_log_manager = new LogManager();
+  s_log_manager = std::unique_ptr<LogManager>(new LogManager());
 }
 
 void LogManager::Shutdown()
 {
   if (s_log_manager)
     s_log_manager->SaveSettings();
-  delete s_log_manager;
-  s_log_manager = nullptr;
+  s_log_manager.reset();
 }
 }  // namespace Common::Log

Source/Core/Common/Logging/LogManager.h

@@ -5,6 +5,7 @@
 
 #include <array>
 #include <cstdarg>
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -31,7 +32,7 @@ public:
   };
 };
 
-class LogManager
+class LogManager final
 {
 public:
   struct LogContainer
@@ -41,6 +42,8 @@ public:
     bool m_enable = false;
   };
 
+  ~LogManager();
+
   static LogManager* GetInstance();
   static void Init();
   static void Shutdown();
@@ -60,7 +63,7 @@ public:
   const char* GetShortName(LogType type) const;
   const char* GetFullName(LogType type) const;
 
-  void RegisterListener(LogListener::LISTENER id, LogListener* listener);
+  void RegisterListener(LogListener::LISTENER id, std::unique_ptr<LogListener> listener);
   void EnableListener(LogListener::LISTENER id, bool enable);
   bool IsListenerEnabled(LogListener::LISTENER id) const;
 
@@ -68,7 +71,6 @@ public:
 
 private:
   LogManager();
-  ~LogManager();
 
   LogManager(const LogManager&) = delete;
   LogManager& operator=(const LogManager&) = delete;
@@ -79,7 +81,7 @@ private:
 
   LogLevel m_level;
   EnumMap<LogContainer, LAST_LOG_TYPE> m_log{};
-  std::array<LogListener*, LogListener::NUMBER_OF_LISTENERS> m_listeners{};
+  std::array<std::unique_ptr<LogListener>, LogListener::NUMBER_OF_LISTENERS> m_listeners{};
   BitSet32 m_listener_ids;
   size_t m_path_cutoff_point = 0;
 };

Source/Core/DolphinQt/Config/LogWidget.cpp

@@ -52,15 +52,16 @@ LogWidget::LogWidget(QWidget* parent) : QDockWidget(parent), m_timer(new QTimer(
 
   connect(&Settings::Instance(), &Settings::DebugFontChanged, this, &LogWidget::UpdateFont);
 
-  Common::Log::LogManager::GetInstance()->RegisterListener(LogListener::LOG_WINDOW_LISTENER, this);
+  Common::Log::LogManager::GetInstance()->RegisterListener(
+      Common::Log::LogListener::LOG_WINDOW_LISTENER, std::make_unique<LogListenerImpl>(this));
 }
 
 LogWidget::~LogWidget()
 {
   SaveSettings();
 
-  Common::Log::LogManager::GetInstance()->RegisterListener(LogListener::LOG_WINDOW_LISTENER,
-                                                           nullptr);
+  Common::Log::LogManager::GetInstance()->RegisterListener(
+      Common::Log::LogListener::LOG_WINDOW_LISTENER, nullptr);
 }
 
 void LogWidget::UpdateLog()

Source/Core/DolphinQt/Config/LogWidget.h

@@ -18,7 +18,7 @@ class QPlainTextEdit;
 class QPushButton;
 class QTimer;
 
-class LogWidget final : public QDockWidget, Common::Log::LogListener
+class LogWidget final : public QDockWidget
 {
   Q_OBJECT
 public:
@@ -29,6 +29,23 @@ protected:
   void closeEvent(QCloseEvent*) override;
 
 private:
+  // LogListener instances are owned by LogManager, so we can't make LogWidget inherit from
+  // LogListener, since Qt should be in control of LogWidget's lifetime. Instead we have
+  // this LogListenerImpl class to act as an adapter.
+  class LogListenerImpl final : public Common::Log::LogListener
+  {
+  public:
+    explicit LogListenerImpl(LogWidget* log_widget) : m_log_widget(log_widget) {}
+
+  private:
+    void Log(Common::Log::LogLevel level, const char* text) override
+    {
+      m_log_widget->Log(level, text);
+    }
+
+    LogWidget* m_log_widget;
+  };
+
   void UpdateLog();
   void UpdateFont();
   void CreateWidgets();
@@ -36,7 +53,7 @@ private:
   void LoadSettings();
   void SaveSettings();
 
-  void Log(Common::Log::LogLevel level, const char* text) override;
+  void Log(Common::Log::LogLevel level, const char* text);
 
   // Log
   QCheckBox* m_log_wrap;