From 137e50dc25d67879ebf2827e3fd5cc754d4d19d7 Mon Sep 17 00:00:00 2001 From: LillyJadeKatrin Date: Sat, 17 May 2025 08:12:43 -0400 Subject: [PATCH 1/3] AchievementManager: Remove CloseGame from LoadGameCallback This was causing deadlocks when a game didn't load (including if RetroAchievements does not yet support it) because it was attempting to close the queue the the callback was currently running on, forcing LoadGameCallback to wait for LoadGameCallback to finish. However, it appears that recent changes to the queue have independently resolved the reason CloseGame was being called here in the first place. --- Source/Core/Core/AchievementManager.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Source/Core/Core/AchievementManager.cpp b/Source/Core/Core/AchievementManager.cpp index b6fc4e0cc3..dba87f2864 100644 --- a/Source/Core/Core/AchievementManager.cpp +++ b/Source/Core/Core/AchievementManager.cpp @@ -994,22 +994,22 @@ void AchievementManager::LoadGameCallback(int result, const char* error_message, OSD::Duration::VERY_LONG, OSD::Color::RED); OSD::AddMessage("Please update Dolphin to a newer version.", OSD::Duration::VERY_LONG, OSD::Color::RED); - instance.CloseGame(); return; } + if (result == RC_NO_GAME_LOADED && instance.m_dll_found) + { + // Allow developer tools for unidentified games + rc_client_set_read_memory_function(instance.m_client, MemoryPeeker); + instance.m_system.store(&Core::System::GetInstance(), std::memory_order_release); + WARN_LOG_FMT(ACHIEVEMENTS, "Unrecognized title ready for development."); + OSD::AddMessage("Unrecognized title loaded for development.", OSD::Duration::VERY_LONG, + OSD::Color::YELLOW); + } if (result != RC_OK) { WARN_LOG_FMT(ACHIEVEMENTS, "Failed to load data for current game."); OSD::AddMessage("Achievements are not supported for this title.", OSD::Duration::VERY_LONG, OSD::Color::RED); - if (instance.m_dll_found && result == RC_NO_GAME_LOADED) - { - // Allow developer tools for unidentified games - rc_client_set_read_memory_function(instance.m_client, MemoryPeeker); - instance.m_system.store(&Core::System::GetInstance(), std::memory_order_release); - return; - } - instance.CloseGame(); return; } From b6803d00fecd599e4d7e29ca9757e793f79afc09 Mon Sep 17 00:00:00 2001 From: LillyJadeKatrin Date: Sat, 17 May 2025 08:27:22 -0400 Subject: [PATCH 2/3] Revert "RetroAchievements: Fix potential deadlock on shutdown." This reverts commit 826f04d06c89011fa7e47531111ee90b27344c8a. --- Source/Core/Core/AchievementManager.cpp | 42 ++++++++----------- Source/Core/Core/AchievementManager.h | 9 +--- .../AchievementSettingsWidget.cpp | 12 ++---- Source/Core/DolphinQt/MainWindow.cpp | 4 +- 4 files changed, 24 insertions(+), 43 deletions(-) diff --git a/Source/Core/Core/AchievementManager.cpp b/Source/Core/Core/AchievementManager.cpp index dba87f2864..40667f6bd8 100644 --- a/Source/Core/Core/AchievementManager.cpp +++ b/Source/Core/Core/AchievementManager.cpp @@ -58,14 +58,13 @@ AchievementManager& AchievementManager::GetInstance() return s_instance; } -void AchievementManager::Init(void* hwnd, AsyncCallbackHandler async_callback_handler) +void AchievementManager::Init(void* hwnd) { 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); @@ -1270,7 +1269,8 @@ 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, callback_data] { + [url = std::move(url), post_data = std::move(post_data), callback = std::move(callback), + callback_data = std::move(callback_data)] { Common::HttpRequest http_request; Common::HttpRequest::Response http_response; if (!post_data.empty()) @@ -1284,28 +1284,22 @@ void AchievementManager::Request(const rc_api_request_t* request, Common::HttpRequest::AllowedReturnCodes::All); } - const auto response_code = http_request.GetLastResponseCode(); + 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; + } - // 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); - }); + callback(&server_response, callback_data); }); } diff --git a/Source/Core/Core/AchievementManager.h b/Source/Core/Core/AchievementManager.h index a8639899d3..90f406772c 100644 --- a/Source/Core/Core/AchievementManager.h +++ b/Source/Core/Core/AchievementManager.h @@ -29,7 +29,6 @@ #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" @@ -122,12 +121,8 @@ public: }; using UpdateCallback = std::function; - using AsyncCallback = Common::MoveOnlyFunction; - using AsyncCallbackHandler = Common::MoveOnlyFunction; - static AchievementManager& GetInstance(); - - void Init(void* hwnd, AsyncCallbackHandler async_callback_handler); + void Init(void* hwnd); void SetUpdateCallback(UpdateCallback callback); void Login(const std::string& password); bool HasAPIToken() const; @@ -313,8 +308,6 @@ 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 4628f14336..6d3ed55ebd 100644 --- a/Source/Core/DolphinQt/Achievements/AchievementSettingsWidget.cpp +++ b/Source/Core/DolphinQt/Achievements/AchievementSettingsWidget.cpp @@ -6,12 +6,13 @@ #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" @@ -21,7 +22,7 @@ #include "DolphinQt/Config/ControllerInterface/ControllerInterfaceWindow.h" #include "DolphinQt/Config/ToolTipControls/ToolTipCheckBox.h" #include "DolphinQt/QtUtils/ModalMessageBox.h" -#include "DolphinQt/QtUtils/QueueOnObject.h" +#include "DolphinQt/QtUtils/NonDefaultQPushButton.h" #include "DolphinQt/QtUtils/SignalBlocking.h" #include "DolphinQt/Settings.h" @@ -251,14 +252,9 @@ void AchievementSettingsWidget::ToggleRAIntegration() auto& instance = AchievementManager::GetInstance(); if (Config::Get(Config::RA_ENABLED)) - { - instance.Init(reinterpret_cast(winId()), - [this](auto func) { QueueOnObject(this, std::move(func)); }); - } + instance.Init(reinterpret_cast(winId())); else - { instance.Shutdown(); - } } void AchievementSettingsWidget::Login() diff --git a/Source/Core/DolphinQt/MainWindow.cpp b/Source/Core/DolphinQt/MainWindow.cpp index 7ed9a3948c..99d38e4d6d 100644 --- a/Source/Core/DolphinQt/MainWindow.cpp +++ b/Source/Core/DolphinQt/MainWindow.cpp @@ -274,9 +274,7 @@ MainWindow::MainWindow(Core::System& system, std::unique_ptr boo NetPlayInit(); #ifdef USE_RETRO_ACHIEVEMENTS - AchievementManager::GetInstance().Init(reinterpret_cast(winId()), [this](auto func) { - QueueOnObject(this, std::move(func)); - }); + AchievementManager::GetInstance().Init(reinterpret_cast(winId())); if (AchievementManager::GetInstance().IsHardcoreModeActive()) Settings::Instance().SetDebugModeEnabled(false); // This needs to trigger on both RA_HARDCORE_ENABLED and RA_ENABLED From dac023af15cbcf966ae5edfb2ea05cd475b0150d Mon Sep 17 00:00:00 2001 From: LillyJadeKatrin Date: Sat, 17 May 2025 23:28:22 -0400 Subject: [PATCH 3/3] Resolve clang-tidy violation --- Source/Core/Core/AchievementManager.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Source/Core/Core/AchievementManager.cpp b/Source/Core/Core/AchievementManager.cpp index 40667f6bd8..f7428c3a3c 100644 --- a/Source/Core/Core/AchievementManager.cpp +++ b/Source/Core/Core/AchievementManager.cpp @@ -1269,8 +1269,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())