Merge pull request #13659 from jordan-woyak/ra-deadlock-fix

RetroAchievements: Fix potential deadlock on shutdown.

4 files changed, 44 insertions, 25 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<const char*>(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;
-        }
-
-        callback(&server_response, callback_data);
+        const auto response_code = http_request.GetLastResponseCode();
+
+        // 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<const char*>(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<void(const UpdatedItems&)>;
 
+  using AsyncCallback = Common::MoveOnlyFunction<void()>;
+  using AsyncCallbackHandler = Common::MoveOnlyFunction<void(AsyncCallback)>;
+
   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 <QLabel>
 #include <QLineEdit>
+#include <QPushButton>
 #include <QString>
 #include <QVBoxLayout>
 
 #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<void*>(winId()));
+  {
+    instance.Init(reinterpret_cast<void*>(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<BootParameters> boo
   NetPlayInit();
 
 #ifdef USE_RETRO_ACHIEVEMENTS
-  AchievementManager::GetInstance().Init(reinterpret_cast<void*>(winId()));
+  AchievementManager::GetInstance().Init(reinterpret_cast<void*>(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