From ccc9d0e5ea12e33c06be1a695873b0fce56e1bff Mon Sep 17 00:00:00 2001 From: LillyJadeKatrin Date: Fri, 9 Jun 2023 17:05:52 -0400 Subject: [PATCH] Synchronized Achievement Window Expanded the use of the lock mutex already used for loading the player's existing unlock status to guard against races involving the Achievements dialog window reading from data AchievementManager might be in the process of updating. The lock has been exposed publicly and the AchievementsWindow uses it in its UpdateData method, and anywhere else that might modify data used to render that window has also been wrapped with it. --- Source/Core/Core/AchievementManager.cpp | 67 ++++++++++++------- Source/Core/Core/AchievementManager.h | 1 + .../Achievements/AchievementsWindow.cpp | 15 +++-- 3 files changed, 54 insertions(+), 29 deletions(-) diff --git a/Source/Core/Core/AchievementManager.cpp b/Source/Core/Core/AchievementManager.cpp index 407e0bed58..ef23dcf163 100644 --- a/Source/Core/Core/AchievementManager.cpp +++ b/Source/Core/Core/AchievementManager.cpp @@ -48,7 +48,11 @@ AchievementManager::ResponseType AchievementManager::Login(const std::string& pa { if (!m_is_runtime_initialized) return AchievementManager::ResponseType::MANAGER_NOT_INITIALIZED; - AchievementManager::ResponseType r_type = VerifyCredentials(password); + AchievementManager::ResponseType r_type = AchievementManager::ResponseType::UNKNOWN_FAILURE; + { + std::lock_guard lg{m_lock}; + r_type = VerifyCredentials(password); + } if (m_update_callback) m_update_callback(); return r_type; @@ -62,7 +66,10 @@ void AchievementManager::LoginAsync(const std::string& password, const ResponseC return; } m_queue.EmplaceItem([this, password, callback] { + { + std::lock_guard lg{m_lock}; callback(VerifyCredentials(password)); + } if (m_update_callback) m_update_callback(); }); @@ -154,11 +161,11 @@ void AchievementManager::LoadGameByFilenameAsync(const std::string& iso_path, } const auto fetch_game_data_response = FetchGameData(); - m_is_game_loaded = fetch_game_data_response == ResponseType::SUCCESS; - if (!m_is_game_loaded) + if (fetch_game_data_response != ResponseType::SUCCESS) { OSD::AddMessage("Unable to retrieve data from RetroAchievements server.", OSD::Duration::VERY_LONG, OSD::Color::RED); + return; } // Claim the lock, then queue the fetch unlock data calls, then initialize the unlock map in @@ -167,6 +174,7 @@ void AchievementManager::LoadGameByFilenameAsync(const std::string& iso_path, // it. { std::lock_guard lg{m_lock}; + m_is_game_loaded = true; LoadUnlockData([](ResponseType r_type) {}); ActivateDeactivateAchievements(); PointSpread spread = TallyScore(); @@ -318,25 +326,33 @@ u32 AchievementManager::MemoryPeeker(u32 address, u32 num_bytes, void* ud) void AchievementManager::AchievementEventHandler(const rc_runtime_event_t* runtime_event) { - switch (runtime_event->type) { - case RC_RUNTIME_EVENT_ACHIEVEMENT_TRIGGERED: - HandleAchievementTriggeredEvent(runtime_event); - break; - case RC_RUNTIME_EVENT_LBOARD_STARTED: - HandleLeaderboardStartedEvent(runtime_event); - break; - case RC_RUNTIME_EVENT_LBOARD_CANCELED: - HandleLeaderboardCanceledEvent(runtime_event); - break; - case RC_RUNTIME_EVENT_LBOARD_TRIGGERED: - HandleLeaderboardTriggeredEvent(runtime_event); - break; + std::lock_guard lg{m_lock}; + switch (runtime_event->type) + { + case RC_RUNTIME_EVENT_ACHIEVEMENT_TRIGGERED: + HandleAchievementTriggeredEvent(runtime_event); + break; + case RC_RUNTIME_EVENT_LBOARD_STARTED: + HandleLeaderboardStartedEvent(runtime_event); + break; + case RC_RUNTIME_EVENT_LBOARD_CANCELED: + HandleLeaderboardCanceledEvent(runtime_event); + break; + case RC_RUNTIME_EVENT_LBOARD_TRIGGERED: + HandleLeaderboardTriggeredEvent(runtime_event); + break; + } } if (m_update_callback) m_update_callback(); } +std::recursive_mutex* AchievementManager::GetLock() +{ + return &m_lock; +} + std::string AchievementManager::GetPlayerDisplayName() const { return IsLoggedIn() ? m_display_name : ""; @@ -397,14 +413,17 @@ void AchievementManager::GetAchievementProgress(AchievementId achievement_id, u3 void AchievementManager::CloseGame() { - m_is_game_loaded = false; - m_game_id = 0; - m_queue.Cancel(); - m_unlock_map.clear(); - m_system = nullptr; - ActivateDeactivateAchievements(); - ActivateDeactivateLeaderboards(); - ActivateDeactivateRichPresence(); + { + std::lock_guard lg{m_lock}; + m_is_game_loaded = false; + m_game_id = 0; + m_queue.Cancel(); + m_unlock_map.clear(); + m_system = nullptr; + ActivateDeactivateAchievements(); + ActivateDeactivateLeaderboards(); + ActivateDeactivateRichPresence(); + } if (m_update_callback) m_update_callback(); } diff --git a/Source/Core/Core/AchievementManager.h b/Source/Core/Core/AchievementManager.h index 9a4289612d..14b6b0ec77 100644 --- a/Source/Core/Core/AchievementManager.h +++ b/Source/Core/Core/AchievementManager.h @@ -84,6 +84,7 @@ public: u32 MemoryPeeker(u32 address, u32 num_bytes, void* ud); void AchievementEventHandler(const rc_runtime_event_t* runtime_event); + std::recursive_mutex* GetLock(); std::string GetPlayerDisplayName() const; u32 GetPlayerScore() const; std::string GetGameDisplayName() const; diff --git a/Source/Core/DolphinQt/Achievements/AchievementsWindow.cpp b/Source/Core/DolphinQt/Achievements/AchievementsWindow.cpp index 40de94ee92..d791d1b107 100644 --- a/Source/Core/DolphinQt/Achievements/AchievementsWindow.cpp +++ b/Source/Core/DolphinQt/Achievements/AchievementsWindow.cpp @@ -4,6 +4,8 @@ #ifdef USE_RETRO_ACHIEVEMENTS #include "DolphinQt/Achievements/AchievementsWindow.h" +#include + #include #include #include @@ -60,11 +62,14 @@ void AchievementsWindow::ConnectWidgets() void AchievementsWindow::UpdateData() { - m_header_widget->UpdateData(); - m_header_widget->setVisible(AchievementManager::GetInstance()->IsLoggedIn()); - // Settings tab handles its own updates ... indeed, that calls this - m_progress_widget->UpdateData(); - m_tab_widget->setTabVisible(1, AchievementManager::GetInstance()->IsGameLoaded()); + { + std::lock_guard lg{*AchievementManager::GetInstance()->GetLock()}; + m_header_widget->UpdateData(); + m_header_widget->setVisible(AchievementManager::GetInstance()->IsLoggedIn()); + // Settings tab handles its own updates ... indeed, that calls this + m_progress_widget->UpdateData(); + m_tab_widget->setTabVisible(1, AchievementManager::GetInstance()->IsGameLoaded()); + } update(); }