ControllerInterface: Allow hotplug callbacks to be unregistered and don't reload the entire config from the ini file on hotplug, just update the control references. This should fix a crash on shutdown on Android.

10 files changed, 62 insertions, 16 deletions

diff --git a/Source/Core/Core/Core.cpp b/Source/Core/Core/Core.cpp
index 4c8d47599f..cf1548314e 100644
--- a/Source/Core/Core/Core.cpp
+++ b/Source/Core/Core/Core.cpp
@@ -475,7 +475,6 @@ static void EmuThread(std::unique_ptr<BootParameters> boot, WindowSystemInfo wsi
   }
   else
   {
-    // Update references in case controllers were refreshed
     g_controller_interface.ChangeWindow(wsi.render_surface);
     Pad::LoadConfig();
     Keyboard::LoadConfig();
@@ -485,12 +484,14 @@ static void EmuThread(std::unique_ptr<BootParameters> boot, WindowSystemInfo wsi
   const bool delete_savestate = boot->delete_savestate;
 
   // Load and Init Wiimotes - only if we are booting in Wii mode
+  bool init_wiimotes = false;
   if (core_parameter.bWii && !SConfig::GetInstance().m_bt_passthrough_enabled)
   {
     if (init_controllers)
     {
       Wiimote::Initialize(savestate_path ? Wiimote::InitializeMode::DO_WAIT_FOR_WIIMOTES :
                                            Wiimote::InitializeMode::DO_NOT_WAIT_FOR_WIIMOTES);
+      init_wiimotes = true;
     }
     else
     {
@@ -501,11 +502,13 @@ static void EmuThread(std::unique_ptr<BootParameters> boot, WindowSystemInfo wsi
       NetPlay::SetupWiimotes();
   }
 
-  Common::ScopeGuard controller_guard{[init_controllers] {
+  Common::ScopeGuard controller_guard{[init_controllers, init_wiimotes] {
     if (!init_controllers)
       return;
 
-    Wiimote::Shutdown();
+    if (init_wiimotes)
+      Wiimote::Shutdown();
+
     Keyboard::Shutdown();
     Pad::Shutdown();
     g_controller_interface.Shutdown();
diff --git a/Source/Core/Core/HW/GCKeyboard.cpp b/Source/Core/Core/HW/GCKeyboard.cpp
index b5ec80bf53..b7134567a5 100644
--- a/Source/Core/Core/HW/GCKeyboard.cpp
+++ b/Source/Core/Core/HW/GCKeyboard.cpp
@@ -26,6 +26,8 @@ InputConfig* GetConfig()
 
 void Shutdown()
 {
+  s_config.UnregisterHotplugCallback();
+
   s_config.ClearControllers();
 }
 
@@ -37,7 +39,7 @@ void Initialize()
       s_config.CreateController<GCKeyboard>(i);
   }
 
-  g_controller_interface.RegisterDevicesChangedCallback(LoadConfig);
+  s_config.RegisterHotplugCallback();
 
   // Load the saved controller config
   s_config.LoadConfig(true);
@@ -57,4 +59,4 @@ KeyboardStatus GetStatus(int port)
 {
   return static_cast<GCKeyboard*>(s_config.GetController(port))->GetInput();
 }
-}
+}  // namespace Keyboard
diff --git a/Source/Core/Core/HW/GCPad.cpp b/Source/Core/Core/HW/GCPad.cpp
index 1ac2a6c3bb..ca1cfb17b8 100644
--- a/Source/Core/Core/HW/GCPad.cpp
+++ b/Source/Core/Core/HW/GCPad.cpp
@@ -23,6 +23,8 @@ InputConfig* GetConfig()
 
 void Shutdown()
 {
+  s_config.UnregisterHotplugCallback();
+
   s_config.ClearControllers();
 }
 
@@ -34,7 +36,7 @@ void Initialize()
       s_config.CreateController<GCPad>(i);
   }
 
-  g_controller_interface.RegisterDevicesChangedCallback(LoadConfig);
+  s_config.RegisterHotplugCallback();
 
   // Load the saved controller config
   s_config.LoadConfig(true);
@@ -74,4 +76,4 @@ bool GetMicButton(const int pad_num)
 {
   return static_cast<GCPad*>(s_config.GetController(pad_num))->GetMicButton();
 }
-}
+}  // namespace Pad
diff --git a/Source/Core/Core/HW/Wiimote.cpp b/Source/Core/Core/HW/Wiimote.cpp
index 190d0a2a9e..7f911b04b4 100644
--- a/Source/Core/Core/HW/Wiimote.cpp
+++ b/Source/Core/Core/HW/Wiimote.cpp
@@ -67,6 +67,8 @@ ControllerEmu::ControlGroup* GetTurntableGroup(int number, WiimoteEmu::Turntable
 
 void Shutdown()
 {
+  s_config.UnregisterHotplugCallback();
+
   s_config.ClearControllers();
 
   WiimoteReal::Stop();
@@ -80,7 +82,7 @@ void Initialize(InitializeMode init_mode)
       s_config.CreateController<WiimoteEmu::Wiimote>(i);
   }
 
-  g_controller_interface.RegisterDevicesChangedCallback(LoadConfig);
+  s_config.RegisterHotplugCallback();
 
   LoadConfig();
 
@@ -215,4 +217,4 @@ void DoState(PointerWrap& p)
   for (int i = 0; i < MAX_BBMOTES; ++i)
     static_cast<WiimoteEmu::Wiimote*>(s_config.GetController(i))->DoState(p);
 }
-}
+}  // namespace Wiimote
diff --git a/Source/Core/Core/HotkeyManager.cpp b/Source/Core/Core/HotkeyManager.cpp
index 96a0d37526..3f98ba67d1 100644
--- a/Source/Core/Core/HotkeyManager.cpp
+++ b/Source/Core/Core/HotkeyManager.cpp
@@ -238,7 +238,7 @@ void Initialize()
   if (s_config.ControllersNeedToBeCreated())
     s_config.CreateController<HotkeyManager>();
 
-  g_controller_interface.RegisterDevicesChangedCallback(LoadConfig);
+  s_config.RegisterHotplugCallback();
 
   // load the saved controller config
   s_config.LoadConfig(true);
@@ -260,9 +260,11 @@ ControllerEmu::ControlGroup* GetHotkeyGroup(HotkeyGroup group)
 
 void Shutdown()
 {
+  s_config.UnregisterHotplugCallback();
+
   s_config.ClearControllers();
 }
-}
+}  // namespace HotkeyManagerEmu
 
 struct HotkeyGroupInfo
 {
diff --git a/Source/Core/DolphinQt/MainWindow.cpp b/Source/Core/DolphinQt/MainWindow.cpp
index 1b57cbe96c..81a8955bd9 100644
--- a/Source/Core/DolphinQt/MainWindow.cpp
+++ b/Source/Core/DolphinQt/MainWindow.cpp
@@ -281,11 +281,11 @@ void MainWindow::ShutdownControllers()
 {
   m_hotkey_scheduler->Stop();
 
-  g_controller_interface.Shutdown();
   Pad::Shutdown();
   Keyboard::Shutdown();
   Wiimote::Shutdown();
   HotkeyManagerEmu::Shutdown();
+  g_controller_interface.Shutdown();
 
   m_hotkey_scheduler->deleteLater();
 }
diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp
index 318b288434..3da92e8b75 100644
--- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp
+++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp
@@ -249,10 +249,20 @@ void ControllerInterface::UpdateInput()
 
 // Register a callback to be called when a device is added or removed (as from the input backends'
 // hotplug thread), or when devices are refreshed
-void ControllerInterface::RegisterDevicesChangedCallback(std::function<void()> callback)
+// Returns a handle for later removing the callback.
+ControllerInterface::HotplugCallbackHandle
+ControllerInterface::RegisterDevicesChangedCallback(std::function<void()> callback)
 {
   std::lock_guard<std::mutex> lk(m_callbacks_mutex);
   m_devices_changed_callbacks.emplace_back(std::move(callback));
+  return std::prev(m_devices_changed_callbacks.end());
+}
+
+// Unregister a device callback.
+void ControllerInterface::UnregisterDevicesChangedCallback(const HotplugCallbackHandle& handle)
+{
+  std::lock_guard<std::mutex> lk(m_callbacks_mutex);
+  m_devices_changed_callbacks.erase(handle);
 }
 
 // Invoke all callbacks that were registered
diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h
index 0d712e5245..7330172421 100644
--- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h
+++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h
@@ -6,9 +6,9 @@
 
 #include <atomic>
 #include <functional>
+#include <list>
 #include <memory>
 #include <mutex>
-#include <vector>
 
 #include "Common/WindowSystemInfo.h"
 #include "InputCommon/ControllerInterface/Device.h"
@@ -40,6 +40,8 @@
 class ControllerInterface : public ciface::Core::DeviceContainer
 {
 public:
+  using HotplugCallbackHandle = std::list<std::function<void()>>::iterator;
+
   ControllerInterface() : m_is_init(false) {}
   void Initialize(const WindowSystemInfo& wsi);
   void ChangeWindow(void* hwnd);
@@ -50,11 +52,12 @@ public:
   bool IsInit() const { return m_is_init; }
   void UpdateInput();
 
-  void RegisterDevicesChangedCallback(std::function<void(void)> callback);
+  HotplugCallbackHandle RegisterDevicesChangedCallback(std::function<void(void)> callback);
+  void UnregisterDevicesChangedCallback(const HotplugCallbackHandle& handle);
   void InvokeDevicesChangedCallbacks() const;
 
 private:
-  std::vector<std::function<void()>> m_devices_changed_callbacks;
+  std::list<std::function<void()>> m_devices_changed_callbacks;
   mutable std::mutex m_callbacks_mutex;
   std::atomic<bool> m_is_init;
   std::atomic<bool> m_is_populating_devices{false};
diff --git a/Source/Core/InputCommon/InputConfig.cpp b/Source/Core/InputCommon/InputConfig.cpp
index dfc677d70a..65cfdef086 100644
--- a/Source/Core/InputCommon/InputConfig.cpp
+++ b/Source/Core/InputCommon/InputConfig.cpp
@@ -147,6 +147,21 @@ std::size_t InputConfig::GetControllerCount() const
   return m_controllers.size();
 }
 
+void InputConfig::RegisterHotplugCallback()
+{
+  // Update control references on all controllers
+  // as configured devices may have been added or removed.
+  m_hotplug_callback_handle = g_controller_interface.RegisterDevicesChangedCallback([this] {
+    for (auto& controller : m_controllers)
+      controller->UpdateReferences(g_controller_interface);
+  });
+}
+
+void InputConfig::UnregisterHotplugCallback()
+{
+  g_controller_interface.UnregisterDevicesChangedCallback(m_hotplug_callback_handle);
+}
+
 bool InputConfig::IsControllerControlledByGamepadDevice(int index) const
 {
   if (static_cast<size_t>(index) >= m_controllers.size())
diff --git a/Source/Core/InputCommon/InputConfig.h b/Source/Core/InputCommon/InputConfig.h
index 004e4be8bb..37d6e06b96 100644
--- a/Source/Core/InputCommon/InputConfig.h
+++ b/Source/Core/InputCommon/InputConfig.h
@@ -9,6 +9,8 @@
 #include <utility>
 #include <vector>
 
+#include "InputCommon/ControllerInterface/ControllerInterface.h"
+
 namespace ControllerEmu
 {
 class EmulatedController;
@@ -40,7 +42,12 @@ public:
   std::string GetProfileName() const { return m_profile_name; }
   std::size_t GetControllerCount() const;
 
+  // These should be used after creating all controllers and before clearing them, respectively.
+  void RegisterHotplugCallback();
+  void UnregisterHotplugCallback();
+
 private:
+  ControllerInterface::HotplugCallbackHandle m_hotplug_callback_handle;
   std::vector<std::unique_ptr<ControllerEmu::EmulatedController>> m_controllers;
   const std::string m_ini_name;
   const std::string m_gui_name;