From 826f04d06c89011fa7e47531111ee90b27344c8a Mon Sep 17 00:00:00 2001 From: Jordan Woyak Date: Wed, 14 May 2025 00:15:30 -0500 Subject: [PATCH] RetroAchievements: Fix potential deadlock on shutdown. --- Source/Core/Core/AchievementManager.cpp | 42 +++++++++++-------- Source/Core/Core/AchievementManager.h | 9 +++- .../AchievementSettingsWidget.cpp | 12 ++++-- Source/Core/DolphinQt/MainWindow.cpp | 4 +- 4 files changed, 43 insertions(+), 24 deletions(-) diff --git a/Source/Core/Core/AchievementManager.cpp b/Source/Core/Core/AchievementManager.cpp index 35cbe24d54..b6fc4e0cc3 100644 --- a/Source/Core/Core/AchievementManager.cpp +++ b/Source/Core/Core/AchievementManager.cpp @@ -58,13 +58,14 @@ AchievementManager& AchievementManager::GetInstance() return s_instance; } -void AchievementManager::Init(void* hwnd) +void AchievementManager::Init(void* hwnd, AsyncCallbackHandler async_callback_handler) { LoadDefaultBadges(); if (!m_client && Config::Get(Config::RA_ENABLED)) { { std::lock_guard lg{m_lock}; + m_async_callback_handler = std::move(async_callback_handler); m_client = rc_client_create(MemoryVerifier, Request); } std::string host_url = Config::Get(Config::RA_HOST_URL); @@ -1269,8 +1270,7 @@ void AchievementManager::Request(const rc_api_request_t* request, std::string url = request->url; std::string post_data = request->post_data; AchievementManager::GetInstance().m_queue.Push( - [url = std::move(url), post_data = std::move(post_data), callback = std::move(callback), - callback_data = std::move(callback_data)] { + [url = std::move(url), post_data = std::move(post_data), callback, callback_data] { Common::HttpRequest http_request; Common::HttpRequest::Response http_response; if (!post_data.empty()) @@ -1284,22 +1284,28 @@ void AchievementManager::Request(const rc_api_request_t* request, Common::HttpRequest::AllowedReturnCodes::All); } - rc_api_server_response_t server_response; - if (http_response.has_value() && http_response->size() > 0) - { - server_response.body = reinterpret_cast(http_response->data()); - server_response.body_length = http_response->size(); - server_response.http_status_code = http_request.GetLastResponseCode(); - } - else - { - static constexpr char error_message[] = "Failed HTTP request."; - server_response.body = error_message; - server_response.body_length = sizeof(error_message); - server_response.http_status_code = RC_API_SERVER_RESPONSE_RETRYABLE_CLIENT_ERROR; - } + const auto response_code = http_request.GetLastResponseCode(); - callback(&server_response, callback_data); + // The callback needs to be invoked inside our async callback handler + // as it may trigger a shutdown which will wait on this very job to complete. + AchievementManager::GetInstance().m_async_callback_handler( + [callback, callback_data, http_response = std::move(http_response), response_code] { + rc_api_server_response_t server_response; + if (http_response.has_value() && http_response->size() > 0) + { + server_response.body = reinterpret_cast(http_response->data()); + server_response.body_length = http_response->size(); + server_response.http_status_code = response_code; + } + else + { + static constexpr char error_message[] = "Failed HTTP request."; + server_response.body = error_message; + server_response.body_length = sizeof(error_message); + server_response.http_status_code = RC_API_SERVER_RESPONSE_RETRYABLE_CLIENT_ERROR; + } + callback(&server_response, callback_data); + }); }); } diff --git a/Source/Core/Core/AchievementManager.h b/Source/Core/Core/AchievementManager.h index 90f406772c..a8639899d3 100644 --- a/Source/Core/Core/AchievementManager.h +++ b/Source/Core/Core/AchievementManager.h @@ -29,6 +29,7 @@ #include "Common/CommonTypes.h" #include "Common/Config/Config.h" #include "Common/Event.h" +#include "Common/Functional.h" #include "Common/HttpRequest.h" #include "Common/JsonUtil.h" #include "Common/Lazy.h" @@ -121,8 +122,12 @@ public: }; using UpdateCallback = std::function; + using AsyncCallback = Common::MoveOnlyFunction; + using AsyncCallbackHandler = Common::MoveOnlyFunction; + static AchievementManager& GetInstance(); - void Init(void* hwnd); + + void Init(void* hwnd, AsyncCallbackHandler async_callback_handler); void SetUpdateCallback(UpdateCallback callback); void Login(const std::string& password); bool HasAPIToken() const; @@ -308,6 +313,8 @@ private: Common::AsyncWorkThread m_image_queue; mutable std::recursive_mutex m_lock; std::recursive_mutex m_filereader_lock; + + AsyncCallbackHandler m_async_callback_handler; }; // class AchievementManager #else // USE_RETRO_ACHIEVEMENTS diff --git a/Source/Core/DolphinQt/Achievements/AchievementSettingsWidget.cpp b/Source/Core/DolphinQt/Achievements/AchievementSettingsWidget.cpp index 6d3ed55ebd..4628f14336 100644 --- a/Source/Core/DolphinQt/Achievements/AchievementSettingsWidget.cpp +++ b/Source/Core/DolphinQt/Achievements/AchievementSettingsWidget.cpp @@ -6,13 +6,12 @@ #include #include +#include #include #include #include "Core/AchievementManager.h" #include "Core/Config/AchievementSettings.h" -#include "Core/Config/FreeLookSettings.h" -#include "Core/Config/MainSettings.h" #include "Core/Config/UISettings.h" #include "Core/Core.h" #include "Core/Movie.h" @@ -22,7 +21,7 @@ #include "DolphinQt/Config/ControllerInterface/ControllerInterfaceWindow.h" #include "DolphinQt/Config/ToolTipControls/ToolTipCheckBox.h" #include "DolphinQt/QtUtils/ModalMessageBox.h" -#include "DolphinQt/QtUtils/NonDefaultQPushButton.h" +#include "DolphinQt/QtUtils/QueueOnObject.h" #include "DolphinQt/QtUtils/SignalBlocking.h" #include "DolphinQt/Settings.h" @@ -252,9 +251,14 @@ void AchievementSettingsWidget::ToggleRAIntegration() auto& instance = AchievementManager::GetInstance(); if (Config::Get(Config::RA_ENABLED)) - instance.Init(reinterpret_cast(winId())); + { + instance.Init(reinterpret_cast(winId()), + [this](auto func) { QueueOnObject(this, std::move(func)); }); + } else + { instance.Shutdown(); + } } void AchievementSettingsWidget::Login() diff --git a/Source/Core/DolphinQt/MainWindow.cpp b/Source/Core/DolphinQt/MainWindow.cpp index 99d38e4d6d..7ed9a3948c 100644 --- a/Source/Core/DolphinQt/MainWindow.cpp +++ b/Source/Core/DolphinQt/MainWindow.cpp @@ -274,7 +274,9 @@ MainWindow::MainWindow(Core::System& system, std::unique_ptr boo NetPlayInit(); #ifdef USE_RETRO_ACHIEVEMENTS - AchievementManager::GetInstance().Init(reinterpret_cast(winId())); + AchievementManager::GetInstance().Init(reinterpret_cast(winId()), [this](auto func) { + QueueOnObject(this, std::move(func)); + }); if (AchievementManager::GetInstance().IsHardcoreModeActive()) Settings::Instance().SetDebugModeEnabled(false); // This needs to trigger on both RA_HARDCORE_ENABLED and RA_ENABLED