From c8be8197118899c065d195c168956b8fa0e76d56 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Fri, 2 May 2025 13:56:44 +0200 Subject: [PATCH] LogManager: Stop using manual memory management This fixes a memory leak that would occur when the Android frontend calls LogManager::Init more than once in order to reload settings. Note that the log window listener is now owned by LogManager instead of by the frontend, making it consistent with the other log listeners. --- Source/Core/Common/Logging/LogManager.cpp | 25 +++++++++------------- Source/Core/Common/Logging/LogManager.h | 10 +++++---- Source/Core/DolphinQt/Config/LogWidget.cpp | 7 +++--- Source/Core/DolphinQt/Config/LogWidget.h | 21 ++++++++++++++++-- 4 files changed, 39 insertions(+), 24 deletions(-) diff --git a/Source/Core/Common/Logging/LogManager.cpp b/Source/Core/Common/Logging/LogManager.cpp index 283bd436e0..d0117ad07f 100644 --- a/Source/Core/Common/Logging/LogManager.cpp +++ b/Source/Core/Common/Logging/LogManager.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -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(File::GetUserPath(F_MAINLOG_IDX))); + RegisterListener(LogListener::CONSOLE_LISTENER, std::make_unique()); // 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 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 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(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 diff --git a/Source/Core/Common/Logging/LogManager.h b/Source/Core/Common/Logging/LogManager.h index 265c294465..d3a235634c 100644 --- a/Source/Core/Common/Logging/LogManager.h +++ b/Source/Core/Common/Logging/LogManager.h @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -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 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 m_log{}; - std::array m_listeners{}; + std::array, LogListener::NUMBER_OF_LISTENERS> m_listeners{}; BitSet32 m_listener_ids; size_t m_path_cutoff_point = 0; }; diff --git a/Source/Core/DolphinQt/Config/LogWidget.cpp b/Source/Core/DolphinQt/Config/LogWidget.cpp index 4b81701abb..9e53852e60 100644 --- a/Source/Core/DolphinQt/Config/LogWidget.cpp +++ b/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(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() diff --git a/Source/Core/DolphinQt/Config/LogWidget.h b/Source/Core/DolphinQt/Config/LogWidget.h index ecaf9ea824..e74c4610db 100644 --- a/Source/Core/DolphinQt/Config/LogWidget.h +++ b/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;