Merge pull request #9702 from Filoppi/controller_interface_fixes

Controller Interface refactor

17 files changed, 512 insertions, 166 deletions

diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp
index 0cae47c741..7c17a61448 100644
--- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp
+++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp
@@ -6,6 +6,7 @@
 
 #include <algorithm>
 
+#include "Common/Assert.h"
 #include "Common/Logging/Log.h"
 #include "Core/HW/WiimoteReal/WiimoteReal.h"
 
@@ -37,6 +38,10 @@
 
 ControllerInterface g_controller_interface;
 
+// We need to save which input channel we are in by thread, so we can access the correct input
+// update values in different threads by input channel. We start from InputChannel::Host on all
+// threads as hotkeys are updated from a worker thread, but UI can read from the main thread. This
+// will never interfere with game threads.
 static thread_local ciface::InputChannel tls_input_channel = ciface::InputChannel::Host;
 
 void ControllerInterface::Initialize(const WindowSystemInfo& wsi)
@@ -44,12 +49,13 @@ void ControllerInterface::Initialize(const WindowSystemInfo& wsi)
   if (m_is_init)
     return;
 
+  std::lock_guard lk_population(m_devices_population_mutex);
+
   m_wsi = wsi;
 
-  // Allow backends to add devices as soon as they are initialized.
-  m_is_init = true;
+  m_populating_devices_counter = 1;
 
-  m_is_populating_devices = true;
+  m_devices_mutex.lock();
 
 #ifdef CIFACE_USE_WIN32
   ciface::Win32::Init(wsi.render_window);
@@ -58,9 +64,7 @@ void ControllerInterface::Initialize(const WindowSystemInfo& wsi)
 // nothing needed
 #endif
 #ifdef CIFACE_USE_OSX
-  if (m_wsi.type == WindowSystemType::MacOS)
-    ciface::OSX::Init(wsi.render_window);
-// nothing needed for Quartz
+// nothing needed for OSX and Quartz
 #endif
 #ifdef CIFACE_USE_SDL
   ciface::SDL::Init();
@@ -78,33 +82,95 @@ void ControllerInterface::Initialize(const WindowSystemInfo& wsi)
   ciface::DualShockUDPClient::Init();
 #endif
 
+  // Don't allow backends to add devices before the first RefreshDevices() as they will be cleaned
+  // there. Or they'd end up waiting on the devices mutex if populated from another thread.
+  m_is_init = true;
+
   RefreshDevices();
+
+  const bool devices_empty = m_devices.empty();
+
+  m_devices_mutex.unlock();
+
+  if (m_populating_devices_counter.fetch_sub(1) == 1 && !devices_empty)
+    InvokeDevicesChangedCallbacks();
 }
 
-void ControllerInterface::ChangeWindow(void* hwnd)
+void ControllerInterface::ChangeWindow(void* hwnd, WindowChangeReason reason)
 {
   if (!m_is_init)
     return;
 
   // This shouldn't use render_surface so no need to update it.
   m_wsi.render_window = hwnd;
-  RefreshDevices();
+
+  // No need to re-add devices if this is an application exit request
+  if (reason == WindowChangeReason::Exit)
+    ClearDevices();
+  else
+    RefreshDevices(RefreshReason::WindowChangeOnly);
 }
 
-void ControllerInterface::RefreshDevices()
+void ControllerInterface::RefreshDevices(RefreshReason reason)
 {
   if (!m_is_init)
     return;
 
+#ifdef CIFACE_USE_OSX
+  if (m_wsi.type == WindowSystemType::MacOS)
   {
-    std::lock_guard lk(m_devices_mutex);
-    m_devices.clear();
+    std::lock_guard lk_pre_population(m_pre_population_mutex);
+    // This is needed to stop its threads before locking our mutexes, to avoid deadlocks
+    // (in case it tried to add a device after we had locked m_devices_population_mutex).
+    // There doesn't seem to be an easy to way to repopulate OSX devices without restarting
+    // its hotplug thread. This will not release its devices, that's still done below.
+    ciface::OSX::DeInit();
   }
+#endif
 
-  m_is_populating_devices = true;
+  // This lock has two main functions:
+  // -Avoid a deadlock between m_devices_mutex and ControllerEmu::s_state_mutex when
+  // InvokeDevicesChangedCallbacks() is called concurrently by two different threads.
+  // -Avoid devices being destroyed while others of the same type are being created.
+  // This wasn't thread safe in multiple device sources.
+  std::lock_guard lk_population(m_devices_population_mutex);
+
+#if defined(CIFACE_USE_WIN32) && !defined(CIFACE_USE_XLIB) && !defined(CIFACE_USE_OSX)
+  // If only the window changed, avoid removing and re-adding all devices.
+  // Instead only refresh devices that require the window handle.
+  if (reason == RefreshReason::WindowChangeOnly)
+  {
+    m_populating_devices_counter.fetch_add(1);
+
+    {
+      std::lock_guard lk(m_devices_mutex);
+      // No need to do anything else in this case.
+      // Only (Win32) DInput needs the window handle to be updated.
+      ciface::Win32::ChangeWindow(m_wsi.render_window);
+    }
+
+    if (m_populating_devices_counter.fetch_sub(1) == 1)
+      InvokeDevicesChangedCallbacks();
+    return;
+  }
+#endif
+
+  m_populating_devices_counter.fetch_add(1);
+
+  // We lock m_devices_mutex here to make everything simpler.
+  // Multiple devices classes have their own "hotplug" thread, and can add/remove devices at any
+  // time, while actual writes to "m_devices" are safe, the order in which they happen is not. That
+  // means a thread could be adding devices while we are removing them, or removing them as we are
+  // populating them (causing missing or duplicate devices).
+  m_devices_mutex.lock();
 
   // Make sure shared_ptr<Device> objects are released before repopulating.
-  InvokeDevicesChangedCallbacks();
+  ClearDevices();
+
+  // Some of these calls won't immediately populate devices, but will do it async
+  // with their own PlatformPopulateDevices().
+  // This means that devices might end up in different order, unless we override their priority.
+  // It also means they might appear as "disconnected" in the Qt UI for a tiny bit of time.
 
 #ifdef CIFACE_USE_WIN32
   ciface::Win32::PopulateDevices(m_wsi.render_window);
@@ -116,7 +182,10 @@ void ControllerInterface::RefreshDevices()
 #ifdef CIFACE_USE_OSX
   if (m_wsi.type == WindowSystemType::MacOS)
   {
-    ciface::OSX::PopulateDevices(m_wsi.render_window);
+    {
+      std::lock_guard lk_pre_population(m_pre_population_mutex);
+      ciface::OSX::Init();
+    }
     ciface::Quartz::PopulateDevices(m_wsi.render_window);
   }
 #endif
@@ -136,10 +205,12 @@ void ControllerInterface::RefreshDevices()
   ciface::DualShockUDPClient::PopulateDevices();
 #endif
 
-  WiimoteReal::ProcessWiimotePool();
+  WiimoteReal::PopulateDevices();
 
-  m_is_populating_devices = false;
-  InvokeDevicesChangedCallbacks();
+  m_devices_mutex.unlock();
+
+  if (m_populating_devices_counter.fetch_sub(1) == 1)
+    InvokeDevicesChangedCallbacks();
 }
 
 void ControllerInterface::PlatformPopulateDevices(std::function<void()> callback)
@@ -147,12 +218,18 @@ void ControllerInterface::PlatformPopulateDevices(std::function<void()> callback
   if (!m_is_init)
     return;
 
-  m_is_populating_devices = true;
+  std::lock_guard lk_population(m_devices_population_mutex);
 
-  callback();
+  m_populating_devices_counter.fetch_add(1);
 
-  m_is_populating_devices = false;
-  InvokeDevicesChangedCallbacks();
+  {
+    std::lock_guard lk(m_devices_mutex);
+
+    callback();
+  }
+
+  if (m_populating_devices_counter.fetch_sub(1) == 1)
+    InvokeDevicesChangedCallbacks();
 }
 
 // Remove all devices and call library cleanup functions
@@ -163,23 +240,11 @@ void ControllerInterface::Shutdown()
 
   // Prevent additional devices from being added during shutdown.
   m_is_init = false;
+  // Additional safety measure to avoid InvokeDevicesChangedCallbacks()
+  m_populating_devices_counter = 1;
 
-  {
-    std::lock_guard lk(m_devices_mutex);
-
-    for (const auto& d : m_devices)
-    {
-      // Set outputs to ZERO before destroying device
-      for (ciface::Core::Device::Output* o : d->Outputs())
-        o->SetState(0);
-    }
-
-    m_devices.clear();
-  }
-
-  // This will update control references so shared_ptr<Device>s are freed up
-  // BEFORE we shutdown the backends.
-  InvokeDevicesChangedCallbacks();
+  // Update control references so shared_ptr<Device>s are freed up BEFORE we shutdown the backends.
+  ClearDevices();
 
 #ifdef CIFACE_USE_WIN32
   ciface::Win32::DeInit();
@@ -203,13 +268,48 @@ void ControllerInterface::Shutdown()
 #ifdef CIFACE_USE_DUALSHOCKUDPCLIENT
   ciface::DualShockUDPClient::DeInit();
 #endif
+
+  // Make sure no devices had been added within Shutdown() in the time
+  // between checking they checked atomic m_is_init bool and we changed it.
+  // We couldn't have locked m_devices_mutex nor m_devices_population_mutex for the whole Shutdown()
+  // as they could cause deadlocks. Note that this is still not 100% safe as some backends are
+  // shut down in other places, possibly adding devices after we have shut down, but the chances of
+  // that happening are basically zero.
+  ClearDevices();
+}
+
+void ControllerInterface::ClearDevices()
+{
+  std::lock_guard lk_population(m_devices_population_mutex);
+
+  {
+    std::lock_guard lk(m_devices_mutex);
+
+    if (m_devices.empty())
+      return;
+
+    for (const auto& d : m_devices)
+    {
+      // Set outputs to ZERO before destroying device
+      for (ciface::Core::Device::Output* o : d->Outputs())
+        o->SetState(0);
+    }
+
+    // Devices will still be alive after this: there are shared ptrs around the code holding them,
+    // but InvokeDevicesChangedCallbacks() will clean all of them.
+    m_devices.clear();
+  }
+
+  InvokeDevicesChangedCallbacks();
 }
 
-void ControllerInterface::AddDevice(std::shared_ptr<ciface::Core::Device> device)
+bool ControllerInterface::AddDevice(std::shared_ptr<ciface::Core::Device> device)
 {
   // If we are shutdown (or in process of shutting down) ignore this request:
   if (!m_is_init)
-    return;
+    return false;
+
+  std::lock_guard lk_population(m_devices_population_mutex);
 
   {
     std::lock_guard lk(m_devices_mutex);
@@ -239,14 +339,36 @@ void ControllerInterface::AddDevice(std::shared_ptr<ciface::Core::Device> device
 
     NOTICE_LOG_FMT(CONTROLLERINTERFACE, "Added device: {}", device->GetQualifiedName());
     m_devices.emplace_back(std::move(device));
+
+    // We can't (and don't want) to control the order in which devices are added, but we
+    // need their order to be consistent, and we need the same one to always be the first, where
+    // present (the keyboard and mouse device usually). This is because when defaulting a
+    // controller profile, it will automatically select the first device in the list as its default.
+    std::stable_sort(m_devices.begin(), m_devices.end(),
+                     [](const std::shared_ptr<ciface::Core::Device>& a,
+                        const std::shared_ptr<ciface::Core::Device>& b) {
+                       // It would be nice to sort devices by Source then Name then ID but it's
+                       // better to leave them sorted by the add order, which also avoids breaking
+                       // the order on other platforms that are less tested.
+                       return a->GetSortPriority() > b->GetSortPriority();
+                     });
   }
 
-  if (!m_is_populating_devices)
+  if (!m_populating_devices_counter)
     InvokeDevicesChangedCallbacks();
+  return true;
 }
 
-void ControllerInterface::RemoveDevice(std::function<bool(const ciface::Core::Device*)> callback)
+void ControllerInterface::RemoveDevice(std::function<bool(const ciface::Core::Device*)> callback,
+                                       bool force_devices_release)
 {
+  // If we are shutdown (or in process of shutting down) ignore this request:
+  if (!m_is_init)
+    return;
+
+  std::lock_guard lk_population(m_devices_population_mutex);
+
+  bool any_removed;
   {
     std::lock_guard lk(m_devices_mutex);
     auto it = std::remove_if(m_devices.begin(), m_devices.end(), [&callback](const auto& dev) {
@@ -257,22 +379,34 @@ void ControllerInterface::RemoveDevice(std::function<bool(const ciface::Core::De
       }
       return false;
     });
+    const size_t prev_size = m_devices.size();
     m_devices.erase(it, m_devices.end());
+    any_removed = m_devices.size() != prev_size;
   }
 
-  if (!m_is_populating_devices)
+  if (any_removed && (!m_populating_devices_counter || force_devices_release))
     InvokeDevicesChangedCallbacks();
 }
 
 // Update input for all devices if lock can be acquired without waiting.
 void ControllerInterface::UpdateInput()
 {
-  // Don't block the UI or CPU thread (to avoid a short but noticeable frame drop)
+  // This should never happen
+  ASSERT(m_is_init);
+  if (!m_is_init)
+    return;
+
+  // TODO: if we are an emulation input channel, we should probably always lock
+  // Prefer outdated values over blocking UI or CPU thread (avoids short but noticeable frame drop)
   if (m_devices_mutex.try_lock())
   {
     std::lock_guard lk(m_devices_mutex, std::adopt_lock);
     for (const auto& d : m_devices)
+    {
+      // Theoretically we could avoid updating input on devices that don't have any references to
+      // them, but in practice a few devices types could break in different ways, so we don't
       d->UpdateInput();
+    }
   }
 }
 
@@ -307,7 +441,7 @@ Common::Vec2 ControllerInterface::GetWindowInputScale() const
 ControllerInterface::HotplugCallbackHandle
 ControllerInterface::RegisterDevicesChangedCallback(std::function<void()> callback)
 {
-  std::lock_guard<std::mutex> lk(m_callbacks_mutex);
+  std::lock_guard lk(m_callbacks_mutex);
   m_devices_changed_callbacks.emplace_back(std::move(callback));
   return std::prev(m_devices_changed_callbacks.end());
 }
@@ -315,14 +449,16 @@ ControllerInterface::RegisterDevicesChangedCallback(std::function<void()> callba
 // Unregister a device callback.
 void ControllerInterface::UnregisterDevicesChangedCallback(const HotplugCallbackHandle& handle)
 {
-  std::lock_guard<std::mutex> lk(m_callbacks_mutex);
+  std::lock_guard lk(m_callbacks_mutex);
   m_devices_changed_callbacks.erase(handle);
 }
 
 // Invoke all callbacks that were registered
 void ControllerInterface::InvokeDevicesChangedCallbacks() const
 {
-  std::lock_guard<std::mutex> lk(m_callbacks_mutex);
-  for (const auto& callback : m_devices_changed_callbacks)
+  m_callbacks_mutex.lock();
+  const auto devices_changed_callbacks = m_devices_changed_callbacks;
+  m_callbacks_mutex.unlock();
+  for (const auto& callback : devices_changed_callbacks)
     callback();
 }
diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h
index 12d17614da..9c91d853e7 100644
--- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h
+++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h
@@ -60,13 +60,38 @@ class ControllerInterface : public ciface::Core::DeviceContainer
 public:
   using HotplugCallbackHandle = std::list<std::function<void()>>::iterator;
 
+  enum class WindowChangeReason
+  {
+    // Application is shutting down
+    Exit,
+    Other
+  };
+
+  enum class RefreshReason
+  {
+    // Only the window changed.
+    WindowChangeOnly,
+    // User requested, or any other internal reason (e.g. init).
+    // The window might have changed anyway.
+    Other
+  };
+
   ControllerInterface() : m_is_init(false) {}
   void Initialize(const WindowSystemInfo& wsi);
-  void ChangeWindow(void* hwnd);
-  void RefreshDevices();
+  // Only call from one thread at a time.
+  void ChangeWindow(void* hwnd, WindowChangeReason reason = WindowChangeReason::Other);
+  // Can be called by any thread at any time (when initialized).
+  void RefreshDevices(RefreshReason reason = RefreshReason::Other);
   void Shutdown();
-  void AddDevice(std::shared_ptr<ciface::Core::Device> device);
-  void RemoveDevice(std::function<bool(const ciface::Core::Device*)> callback);
+  bool AddDevice(std::shared_ptr<ciface::Core::Device> device);
+  // Removes all the devices the function returns true to.
+  // If all the devices shared ptrs need to be destroyed immediately,
+  // set force_devices_release to true.
+  void RemoveDevice(std::function<bool(const ciface::Core::Device*)> callback,
+                    bool force_devices_release = false);
+  // This is mandatory to use on device populations functions that can be called concurrently by
+  // more than one thread, or that are called by a single other thread.
+  // Without this, our devices list might end up in a mixed state.
   void PlatformPopulateDevices(std::function<void()> callback);
   bool IsInit() const { return m_is_init; }
   void UpdateInput();
@@ -87,10 +112,17 @@ public:
   static ciface::InputChannel GetCurrentInputChannel();
 
 private:
+  void ClearDevices();
+
   std::list<std::function<void()>> m_devices_changed_callbacks;
+  mutable std::recursive_mutex m_devices_population_mutex;
+  mutable std::mutex m_pre_population_mutex;
   mutable std::mutex m_callbacks_mutex;
   std::atomic<bool> m_is_init;
-  std::atomic<bool> m_is_populating_devices{false};
+  // This is now always protected by m_devices_population_mutex, so
+  // it doesn't really need to be a counter or atomic anymore (it could be a raw bool),
+  // but we keep it so for simplicity, in case we changed the design.
+  std::atomic<int> m_populating_devices_counter;
   WindowSystemInfo m_wsi;
   std::atomic<float> m_aspect_ratio_adjustment = 1;
 };
diff --git a/Source/Core/InputCommon/ControllerInterface/CoreDevice.h b/Source/Core/InputCommon/ControllerInterface/CoreDevice.h
index c368578949..d3312568b2 100644
--- a/Source/Core/InputCommon/ControllerInterface/CoreDevice.h
+++ b/Source/Core/InputCommon/ControllerInterface/CoreDevice.h
@@ -125,9 +125,20 @@ public:
   // Currently handled on a per-backend basis but this could change.
   virtual bool IsValid() const { return true; }
 
+  // Returns true whether this device is "virtual/emulated", not linked
+  // to any actual physical device. Mostly used by keyboard and mouse devices,
+  // and to avoid uselessly recreating the device unless really necessary.
+  // Doesn't necessarily need to be set to true if the device is virtual.
+  virtual bool IsVirtualDevice() const { return false; }
+
   // (e.g. Xbox 360 controllers have controller number LEDs which should match the ID we use.)
   virtual std::optional<int> GetPreferredId() const;
 
+  // Use this to change the order in which devices are sorted in their list.
+  // A higher priority means it will be one of the first ones (smaller index), making it more
+  // likely to be index 0, which is automatically set as the default device when there isn't one.
+  virtual int GetSortPriority() const { return 0; }
+
   const std::vector<Input*>& Inputs() const { return m_inputs; }
   const std::vector<Output*>& Outputs() const { return m_outputs; }
 
@@ -227,6 +238,7 @@ public:
                                           std::chrono::milliseconds maximum_wait) const;
 
 protected:
+  // Exclusively needed when reading/writing "m_devices"
   mutable std::recursive_mutex m_devices_mutex;
   std::vector<std::shared_ptr<Device>> m_devices;
 };
diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInput.cpp b/Source/Core/InputCommon/ControllerInterface/DInput/DInput.cpp
index d508de613a..f5e105dc45 100644
--- a/Source/Core/InputCommon/ControllerInterface/DInput/DInput.cpp
+++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInput.cpp
@@ -16,6 +16,8 @@
 
 namespace ciface::DInput
 {
+static IDirectInput8* s_idi8 = nullptr;
+
 BOOL CALLBACK DIEnumDeviceObjectsCallback(LPCDIDEVICEOBJECTINSTANCE lpddoi, LPVOID pvRef)
 {
   ((std::list<DIDEVICEOBJECTINSTANCE>*)pvRef)->push_back(*lpddoi);
@@ -42,29 +44,57 @@ std::string GetDeviceName(const LPDIRECTINPUTDEVICE8 device)
   }
   else
   {
-    ERROR_LOG_FMT(PAD, "GetProperty(DIPROP_PRODUCTNAME) failed.");
+    ERROR_LOG_FMT(CONTROLLERINTERFACE, "GetProperty(DIPROP_PRODUCTNAME) failed.");
   }
 
   return result;
 }
 
+// Assumes hwnd had not changed from the previous call
 void PopulateDevices(HWND hwnd)
 {
-  // Remove unplugged devices.
+  if (!s_idi8 && FAILED(DirectInput8Create(GetModuleHandle(nullptr), DIRECTINPUT_VERSION,
+                                           IID_IDirectInput8, (LPVOID*)&s_idi8, nullptr)))
+  {
+    ERROR_LOG_FMT(CONTROLLERINTERFACE, "DirectInput8Create failed.");
+    return;
+  }
+
+  // Remove old (invalid) devices. No need to ever remove the KeyboardMouse device.
+  // Note that if we have 2+ DInput controllers, not fully repopulating devices
+  // will mean that a device with index "2" could persist while there is no device with index "0".
+  // This is slightly inconsistent as when we refresh all devices, they will instead reset, and
+  // that happens a lot (for uncontrolled reasons, like starting/stopping the emulation).
   g_controller_interface.RemoveDevice(
       [](const auto* dev) { return dev->GetSource() == DINPUT_SOURCE_NAME && !dev->IsValid(); });
 
-  IDirectInput8* idi8;
-  if (FAILED(DirectInput8Create(GetModuleHandle(nullptr), DIRECTINPUT_VERSION, IID_IDirectInput8,
-                                (LPVOID*)&idi8, nullptr)))
+  InitKeyboardMouse(s_idi8, hwnd);
+  InitJoystick(s_idi8, hwnd);
+}
+
+void ChangeWindow(HWND hwnd)
+{
+  if (s_idi8)  // Has init? Ignore if called before the first PopulateDevices()
   {
-    ERROR_LOG_FMT(PAD, "DirectInput8Create failed.");
-    return;
-  }
+    // The KeyboardMouse device is marked as virtual device, so we avoid removing it.
+    // We need to force all the DInput joysticks to be destroyed now, or recreation would fail.
+    g_controller_interface.RemoveDevice(
+        [](const auto* dev) {
+          return dev->GetSource() == DINPUT_SOURCE_NAME && !dev->IsVirtualDevice();
+        },
+        true);
 
-  InitKeyboardMouse(idi8, hwnd);
-  InitJoystick(idi8, hwnd);
+    SetKeyboardMouseWindow(hwnd);
+    InitJoystick(s_idi8, hwnd);
+  }
+}
 
-  idi8->Release();
+void DeInit()
+{
+  if (s_idi8)
+  {
+    s_idi8->Release();
+    s_idi8 = nullptr;
+  }
 }
 }  // namespace ciface::DInput
diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInput.h b/Source/Core/InputCommon/ControllerInterface/DInput/DInput.h
index ed159089e0..8be064a1f1 100644
--- a/Source/Core/InputCommon/ControllerInterface/DInput/DInput.h
+++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInput.h
@@ -20,4 +20,6 @@ BOOL CALLBACK DIEnumDevicesCallback(LPCDIDEVICEINSTANCE lpddi, LPVOID pvRef);
 std::string GetDeviceName(const LPDIRECTINPUTDEVICE8 device);
 
 void PopulateDevices(HWND hwnd);
+void ChangeWindow(HWND hwnd);
+void DeInit();
 }  // namespace ciface::DInput
diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.cpp b/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.cpp
index 67fd33186b..3d51d9c043 100644
--- a/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.cpp
+++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.cpp
@@ -4,6 +4,7 @@
 
 #include <algorithm>
 #include <limits>
+#include <mutex>
 #include <set>
 #include <sstream>
 #include <type_traits>
@@ -29,6 +30,7 @@ struct GUIDComparator
 };
 
 static std::set<GUID, GUIDComparator> s_guids_in_use;
+static std::mutex s_guids_mutex;
 
 void InitJoystick(IDirectInput8* const idi8, HWND hwnd)
 {
@@ -46,12 +48,16 @@ void InitJoystick(IDirectInput8* const idi8, HWND hwnd)
     }
 
     // Skip devices we are already using.
-    if (s_guids_in_use.count(joystick.guidInstance))
     {
-      continue;
+      std::lock_guard lk(s_guids_mutex);
+      if (s_guids_in_use.count(joystick.guidInstance))
+      {
+        continue;
+      }
     }
 
     LPDIRECTINPUTDEVICE8 js_device;
+    // Don't print any warnings on failure
     if (SUCCEEDED(idi8->CreateDevice(joystick.guidInstance, &js_device, nullptr)))
     {
       if (SUCCEEDED(js_device->SetDataFormat(&c_dfDIJoystick)))
@@ -60,37 +66,40 @@ void InitJoystick(IDirectInput8* const idi8, HWND hwnd)
                                                   DISCL_BACKGROUND | DISCL_EXCLUSIVE)))
         {
           WARN_LOG_FMT(
-              PAD,
+              CONTROLLERINTERFACE,
               "DInput: Failed to acquire device exclusively. Force feedback will be unavailable.");
           // Fall back to non-exclusive mode, with no rumble
           if (FAILED(
                   js_device->SetCooperativeLevel(nullptr, DISCL_BACKGROUND | DISCL_NONEXCLUSIVE)))
           {
-            // PanicAlert("SetCooperativeLevel failed!");
             js_device->Release();
             continue;
           }
         }
 
-        s_guids_in_use.insert(joystick.guidInstance);
         auto js = std::make_shared<Joystick>(js_device);
-
-        // only add if it has some inputs/outputs
+        // only add if it has some inputs/outputs.
+        // Don't even add it to our static list in case we first created it without a window handle,
+        // failing to get exclusive mode, and then later managed to obtain it, which mean it
+        // could now have some outputs if it didn't before.
         if (js->Inputs().size() || js->Outputs().size())
-          g_controller_interface.AddDevice(std::move(js));
+        {
+          if (g_controller_interface.AddDevice(std::move(js)))
+          {
+            std::lock_guard lk(s_guids_mutex);
+            s_guids_in_use.insert(joystick.guidInstance);
+          }
+        }
       }
       else
       {
-        // PanicAlert("SetDataFormat failed!");
         js_device->Release();
       }
     }
   }
 }
 
-Joystick::Joystick(/*const LPCDIDEVICEINSTANCE lpddi, */ const LPDIRECTINPUTDEVICE8 device)
-    : m_device(device)
-//, m_name(TStringToString(lpddi->tszInstanceName))
+Joystick::Joystick(const LPDIRECTINPUTDEVICE8 device) : m_device(device)
 {
   // seems this needs to be done before GetCapabilities
   // polled or buffered data
@@ -183,11 +192,12 @@ Joystick::~Joystick()
   info.dwSize = sizeof(info);
   if (SUCCEEDED(m_device->GetDeviceInfo(&info)))
   {
+    std::lock_guard lk(s_guids_mutex);
     s_guids_in_use.erase(info.guidInstance);
   }
   else
   {
-    ERROR_LOG_FMT(PAD, "DInputJoystick: GetDeviceInfo failed.");
+    ERROR_LOG_FMT(CONTROLLERINTERFACE, "DInputJoystick: GetDeviceInfo failed.");
   }
 
   DeInitForceFeedback();
diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.cpp b/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.cpp
index ebe2e84286..aaa723849e 100644
--- a/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.cpp
+++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.cpp
@@ -6,7 +6,8 @@
 
 #include <algorithm>
 
-#include <fmt/format.h>
+#include "Common/Logging/Log.h"
+#include "Core/Core.h"
 
 #include "InputCommon/ControllerInterface/ControllerInterface.h"
 #include "InputCommon/ControllerInterface/DInput/DInput.h"
@@ -54,15 +55,18 @@ static const struct
 #include "InputCommon/ControllerInterface/DInput/NamedKeys.h"  // NOLINT
 };
 
-// Prevent duplicate keyboard/mouse devices.
-static bool s_keyboard_mouse_exists = false;
+// Prevent duplicate keyboard/mouse devices. Modified by more threads.
+static bool s_keyboard_mouse_exists;
+static HWND s_hwnd;
 
 void InitKeyboardMouse(IDirectInput8* const idi8, HWND hwnd)
 {
   if (s_keyboard_mouse_exists)
     return;
 
-  // mouse and keyboard are a combined device, to allow shift+click and stuff
+  s_hwnd = hwnd;
+
+  // Mouse and keyboard are a combined device, to allow shift+click and stuff
   // if that's dumb, I will make a VirtualDevice class that just uses ranges of inputs/outputs from
   // other devices
   // so there can be a separated Keyboard and mouse, as well as combined KeyboardMouse
@@ -70,6 +74,8 @@ void InitKeyboardMouse(IDirectInput8* const idi8, HWND hwnd)
   LPDIRECTINPUTDEVICE8 kb_device = nullptr;
   LPDIRECTINPUTDEVICE8 mo_device = nullptr;
 
+  // These are "virtual" system devices, so they are always there even if we have no physical
+  // mouse and keyboard plugged into the computer
   if (SUCCEEDED(idi8->CreateDevice(GUID_SysKeyboard, &kb_device, nullptr)) &&
       SUCCEEDED(kb_device->SetDataFormat(&c_dfDIKeyboard)) &&
       SUCCEEDED(kb_device->SetCooperativeLevel(nullptr, DISCL_BACKGROUND | DISCL_NONEXCLUSIVE)) &&
@@ -77,20 +83,32 @@ void InitKeyboardMouse(IDirectInput8* const idi8, HWND hwnd)
       SUCCEEDED(mo_device->SetDataFormat(&c_dfDIMouse2)) &&
       SUCCEEDED(mo_device->SetCooperativeLevel(nullptr, DISCL_BACKGROUND | DISCL_NONEXCLUSIVE)))
   {
-    g_controller_interface.AddDevice(std::make_shared<KeyboardMouse>(kb_device, mo_device, hwnd));
+    g_controller_interface.AddDevice(std::make_shared<KeyboardMouse>(kb_device, mo_device));
     return;
   }
 
+  ERROR_LOG_FMT(CONTROLLERINTERFACE, "KeyboardMouse device failed to be created");
+
   if (kb_device)
     kb_device->Release();
   if (mo_device)
     mo_device->Release();
 }
 
+void SetKeyboardMouseWindow(HWND hwnd)
+{
+  s_hwnd = hwnd;
+}
+
 KeyboardMouse::~KeyboardMouse()
 {
   s_keyboard_mouse_exists = false;
 
+  // Independently of the order in which we do these, if we put a breakpoint on Unacquire() (or in
+  // any place in the call stack before this), when refreshing devices from the UI, on the second
+  // attempt, it will get stuck in an infinite (while) loop inside dinput8.dll. Given that it can't
+  // be otherwise be reproduced (not even with sleeps), we can just ignore the problem.
+
   // kb
   m_kb_device->Unacquire();
   m_kb_device->Release();
@@ -100,14 +118,15 @@ KeyboardMouse::~KeyboardMouse()
 }
 
 KeyboardMouse::KeyboardMouse(const LPDIRECTINPUTDEVICE8 kb_device,
-                             const LPDIRECTINPUTDEVICE8 mo_device, HWND hwnd)
-    : m_kb_device(kb_device), m_mo_device(mo_device), m_hwnd(hwnd), m_last_update(GetTickCount()),
-      m_state_in()
+                             const LPDIRECTINPUTDEVICE8 mo_device)
+    : m_kb_device(kb_device), m_mo_device(mo_device), m_last_update(GetTickCount()), m_state_in()
 {
   s_keyboard_mouse_exists = true;
 
-  m_kb_device->Acquire();
-  m_mo_device->Acquire();
+  if (FAILED(m_kb_device->Acquire()))
+    WARN_LOG_FMT(CONTROLLERINTERFACE, "Keyboard device failed to acquire. We'll retry later");
+  if (FAILED(m_mo_device->Acquire()))
+    WARN_LOG_FMT(CONTROLLERINTERFACE, "Mouse device failed to acquire. We'll retry later");
 
   // KEYBOARD
   // add keys
@@ -155,11 +174,11 @@ void KeyboardMouse::UpdateCursorInput()
 
   // Get the cursor position relative to the upper left corner of the current window
   // (separate or render to main)
-  ScreenToClient(m_hwnd, &point);
+  ScreenToClient(s_hwnd, &point);
 
-  // Get the size of the current window. (In my case Rect.top and Rect.left was zero.)
+  // Get the size of the current window (in my case Rect.top and Rect.left was zero).
   RECT rect;
-  GetClientRect(m_hwnd, &rect);
+  GetClientRect(s_hwnd, &rect);
 
   // Width and height are the size of the rendering window. They could be 0
   const auto win_width = std::max(rect.right - rect.left, 1l);
@@ -174,6 +193,8 @@ void KeyboardMouse::UpdateCursorInput()
 
 void KeyboardMouse::UpdateInput()
 {
+  UpdateCursorInput();
+
   DIMOUSESTATE2 tmp_mouse;
 
   // if mouse position hasn't been updated in a short while, skip a dev state
@@ -190,16 +211,14 @@ void KeyboardMouse::UpdateInput()
 
   m_last_update = cur_time;
 
-  HRESULT kb_hr = m_kb_device->GetDeviceState(sizeof(m_state_in.keyboard), &m_state_in.keyboard);
   HRESULT mo_hr = m_mo_device->GetDeviceState(sizeof(tmp_mouse), &tmp_mouse);
-
-  if (DIERR_INPUTLOST == kb_hr || DIERR_NOTACQUIRED == kb_hr)
-    m_kb_device->Acquire();
-
   if (DIERR_INPUTLOST == mo_hr || DIERR_NOTACQUIRED == mo_hr)
-    m_mo_device->Acquire();
-
-  if (SUCCEEDED(mo_hr))
+  {
+    INFO_LOG_FMT(CONTROLLERINTERFACE, "Mouse device failed to get state");
+    if (FAILED(m_mo_device->Acquire()))
+      INFO_LOG_FMT(CONTROLLERINTERFACE, "Mouse device failed to re-acquire, we'll retry later");
+  }
+  else if (SUCCEEDED(mo_hr))
   {
     m_state_in.relative_mouse.Move({tmp_mouse.lX, tmp_mouse.lY, tmp_mouse.lZ});
     m_state_in.relative_mouse.Update();
@@ -210,8 +229,16 @@ void KeyboardMouse::UpdateInput()
 
     // copy over the buttons
     std::copy_n(tmp_mouse.rgbButtons, std::size(tmp_mouse.rgbButtons), m_state_in.mouse.rgbButtons);
+  }
 
-    UpdateCursorInput();
+  HRESULT kb_hr = m_kb_device->GetDeviceState(sizeof(m_state_in.keyboard), &m_state_in.keyboard);
+  if (kb_hr == DIERR_INPUTLOST || kb_hr == DIERR_NOTACQUIRED)
+  {
+    INFO_LOG_FMT(CONTROLLERINTERFACE, "Keyboard device failed to get state");
+    if (SUCCEEDED(m_kb_device->Acquire()))
+      m_kb_device->GetDeviceState(sizeof(m_state_in.keyboard), &m_state_in.keyboard);
+    else
+      INFO_LOG_FMT(CONTROLLERINTERFACE, "Keyboard device failed to re-acquire, we'll retry later");
   }
 }
 
@@ -225,6 +252,17 @@ std::string KeyboardMouse::GetSource() const
   return DINPUT_SOURCE_NAME;
 }
 
+// Give this device a higher priority to make sure it shows first
+int KeyboardMouse::GetSortPriority() const
+{
+  return 5;
+}
+
+bool KeyboardMouse::IsVirtualDevice() const
+{
+  return true;
+}
+
 // names
 std::string KeyboardMouse::Key::GetName() const
 {
diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.h b/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.h
index ceda59d02c..b0fd39c00d 100644
--- a/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.h
+++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.h
@@ -16,6 +16,7 @@ namespace ciface::DInput
 void InitKeyboardMouse(IDirectInput8* const idi8, HWND hwnd);
 
 using RelativeMouseState = RelativeInputState<Common::TVec3<LONG>>;
+void SetKeyboardMouseWindow(HWND hwnd);
 
 class KeyboardMouse : public Core::Device
 {
@@ -34,6 +35,7 @@ private:
     RelativeMouseState relative_mouse;
   };
 
+  // Keyboard key
   class Key : public Input
   {
   public:
@@ -46,6 +48,7 @@ private:
     const u8 m_index;
   };
 
+  // Mouse button
   class Button : public Input
   {
   public:
@@ -58,6 +61,7 @@ private:
     const u8 m_index;
   };
 
+  // Mouse movement offset axis. Includes mouse wheel
   class Axis : public Input
   {
   public:
@@ -72,6 +76,7 @@ private:
     const u8 m_index;
   };
 
+  // Mouse from window center
   class Cursor : public Input
   {
   public:
@@ -92,12 +97,13 @@ private:
 public:
   void UpdateInput() override;
 
-  KeyboardMouse(const LPDIRECTINPUTDEVICE8 kb_device, const LPDIRECTINPUTDEVICE8 mo_device,
-                HWND hwnd);
+  KeyboardMouse(const LPDIRECTINPUTDEVICE8 kb_device, const LPDIRECTINPUTDEVICE8 mo_device);
   ~KeyboardMouse();
 
   std::string GetName() const override;
   std::string GetSource() const override;
+  int GetSortPriority() const override;
+  bool IsVirtualDevice() const override;
 
 private:
   void UpdateCursorInput();
@@ -105,8 +111,6 @@ private:
   const LPDIRECTINPUTDEVICE8 m_kb_device;
   const LPDIRECTINPUTDEVICE8 m_mo_device;
 
-  const HWND m_hwnd;
-
   DWORD m_last_update;
   State m_state_in;
 };
diff --git a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp
index 6511af49c1..f8347aa51a 100644
--- a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp
+++ b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp
@@ -7,7 +7,6 @@
 #include <algorithm>
 #include <array>
 #include <chrono>
-#include <mutex>
 #include <tuple>
 
 #include <SFML/Network/SocketSelector.hpp>
@@ -19,6 +18,7 @@
 #include "Common/Logging/Log.h"
 #include "Common/MathUtil.h"
 #include "Common/Random.h"
+#include "Common/ScopeGuard.h"
 #include "Common/StringUtil.h"
 #include "Common/Thread.h"
 #include "Core/CoreTiming.h"
@@ -136,6 +136,8 @@ public:
   std::string GetName() const final override;
   std::string GetSource() const final override;
   std::optional<int> GetPreferredId() const final override;
+  // Always add these at the end, given their hotplug nature
+  int GetSortPriority() const override { return -2; }
 
 private:
   void ResetPadData();
@@ -190,12 +192,12 @@ struct Server
   std::string m_description;
   std::string m_address;
   u16 m_port;
-  std::mutex m_port_info_mutex;
   std::array<Proto::MessageType::PortInfo, Proto::PORT_COUNT> m_port_info;
   sf::UdpSocket m_socket;
   SteadyClock::time_point m_disconnect_time = SteadyClock::now();
 };
 
+static bool s_has_init;
 static bool s_servers_enabled;
 static std::vector<Server> s_servers;
 static u32 s_client_uid;
@@ -215,6 +217,8 @@ static void HotplugThreadFunc()
 {
   Common::SetCurrentThreadName("DualShockUDPClient Hotplug Thread");
   INFO_LOG_FMT(CONTROLLERINTERFACE, "DualShockUDPClient hotplug thread started");
+  Common::ScopeGuard thread_stop_guard{
+      [] { INFO_LOG_FMT(CONTROLLERINTERFACE, "DualShockUDPClient hotplug thread stopped"); }};
 
   std::vector<bool> timed_out_servers(s_servers.size(), false);
 
@@ -329,7 +333,6 @@ static void HotplugThreadFunc()
       }
     }
   }
-  INFO_LOG_FMT(CONTROLLERINTERFACE, "DualShockUDPClient hotplug thread stopped");
 }
 
 static void StartHotplugThread()
@@ -353,13 +356,15 @@ static void StopHotplugThread()
     return;
   }
 
+  s_hotplug_thread.join();
+
   for (auto& server : s_servers)
   {
     server.m_socket.unbind();  // interrupt blocking socket
   }
-  s_hotplug_thread.join();
 }
 
+// Also just start
 static void Restart()
 {
   INFO_LOG_FMT(CONTROLLERINTERFACE, "DualShockUDPClient Restart");
@@ -375,7 +380,8 @@ static void Restart()
     }
   }
 
-  PopulateDevices();  // Only removes devices
+  // Only removes devices as servers have been cleaned
+  g_controller_interface.PlatformPopulateDevices([] { PopulateDevices(); });
 
   s_client_uid = Common::Random::GenerateValue<u32>();
   s_next_listports_time = SteadyClock::now();
@@ -386,6 +392,9 @@ static void Restart()
 
 static void ConfigChanged()
 {
+  if (!s_has_init)
+    return;
+
   const bool servers_enabled = Config::Get(Settings::SERVERS_ENABLED);
   const std::string servers_setting = Config::Get(Settings::SERVERS);
 
@@ -398,6 +407,9 @@ static void ConfigChanged()
 
   if (servers_enabled != s_servers_enabled || servers_setting != new_servers_setting)
   {
+    // Stop the thread before writing to s_servers
+    StopHotplugThread();
+
     s_servers_enabled = servers_enabled;
     s_servers.clear();
 
@@ -425,6 +437,9 @@ static void ConfigChanged()
 
 void Init()
 {
+  // Does not support multiple init calls
+  s_has_init = true;
+
   // The following is added for backwards compatibility
   const auto server_address_setting = Config::Get(Settings::SERVER_ADDRESS);
   const auto server_port_setting = Config::Get(Settings::SERVER_PORT);
@@ -445,6 +460,10 @@ void Init()
   ConfigChanged();  // Call it immediately to load settings
 }
 
+// This can be called by the host thread as well as the hotplug thread, concurrently.
+// So use PlatformPopulateDevices().
+// s_servers is already safe because it can only be modified when the DSU thread is not running,
+// from the main thread
 void PopulateDevices()
 {
   INFO_LOG_FMT(CONTROLLERINTERFACE, "DualShockUDPClient PopulateDevices");
@@ -455,9 +474,11 @@ void PopulateDevices()
   g_controller_interface.RemoveDevice(
       [](const auto* dev) { return dev->GetSource() == DUALSHOCKUDP_SOURCE_NAME; });
 
-  for (auto& server : s_servers)
+  // Users might have created more than one server on the same IP/Port.
+  // Devices might end up being duplicated (if the server responds two all requests)
+  // but they won't conflict.
+  for (const auto& server : s_servers)
   {
-    std::lock_guard lock{server.m_port_info_mutex};
     for (size_t port_index = 0; port_index < server.m_port_info.size(); port_index++)
     {
       const Proto::MessageType::PortInfo& port_info = server.m_port_info[port_index];
@@ -473,6 +494,10 @@ void PopulateDevices()
 void DeInit()
 {
   StopHotplugThread();
+
+  s_has_init = false;
+  s_servers_enabled = false;
+  s_servers.clear();
 }
 
 Device::Device(std::string name, int index, std::string server_address, u16 server_port)
diff --git a/Source/Core/InputCommon/ControllerInterface/OSX/OSX.h b/Source/Core/InputCommon/ControllerInterface/OSX/OSX.h
index 955b426f5e..f4ebc1a911 100644
--- a/Source/Core/InputCommon/ControllerInterface/OSX/OSX.h
+++ b/Source/Core/InputCommon/ControllerInterface/OSX/OSX.h
@@ -6,8 +6,7 @@
 
 namespace ciface::OSX
 {
-void Init(void* window);
-void PopulateDevices(void* window);
+void Init();
 void DeInit();
 
 void DeviceElementDebugPrint(const void*, void*);
diff --git a/Source/Core/InputCommon/ControllerInterface/OSX/OSX.mm b/Source/Core/InputCommon/ControllerInterface/OSX/OSX.mm
index 63bddf499e..8c80e9f529 100644
--- a/Source/Core/InputCommon/ControllerInterface/OSX/OSX.mm
+++ b/Source/Core/InputCommon/ControllerInterface/OSX/OSX.mm
@@ -135,8 +135,6 @@ static void DeviceDebugPrint(IOHIDDeviceRef device)
 #endif
 }
 
-static void* g_window;
-
 static std::string GetDeviceRefName(IOHIDDeviceRef inIOHIDDeviceRef)
 {
   const NSString* name = reinterpret_cast<const NSString*>(
@@ -172,10 +170,8 @@ static void DeviceMatchingCallback(void* inContext, IOReturn inResult, void* inS
   }
 }
 
-void Init(void* window)
+void Init()
 {
-  g_window = window;
-
   HIDManager = IOHIDManagerCreate(kCFAllocatorDefault, kIOHIDOptionsTypeNone);
   if (!HIDManager)
     ERROR_LOG_FMT(CONTROLLERINTERFACE, "Failed to create HID Manager reference");
@@ -210,19 +206,17 @@ void Init(void* window)
   });
 }
 
-void PopulateDevices(void* window)
-{
-  DeInit();
-  Init(window);
-}
-
 void DeInit()
 {
-  s_stopper.Signal();
-  s_hotplug_thread.join();
+  if (HIDManager)
+  {
+    s_stopper.Signal();
+    s_hotplug_thread.join();
 
-  // This closes all devices as well
-  IOHIDManagerClose(HIDManager, kIOHIDOptionsTypeNone);
-  CFRelease(HIDManager);
+    // This closes all devices as well
+    IOHIDManagerClose(HIDManager, kIOHIDOptionsTypeNone);
+    CFRelease(HIDManager);
+    HIDManager = nullptr;
+  }
 }
 }  // namespace ciface::OSX
diff --git a/Source/Core/InputCommon/ControllerInterface/SDL/SDL.cpp b/Source/Core/InputCommon/ControllerInterface/SDL/SDL.cpp
index f908181c8d..71d75f786c 100644
--- a/Source/Core/InputCommon/ControllerInterface/SDL/SDL.cpp
+++ b/Source/Core/InputCommon/ControllerInterface/SDL/SDL.cpp
@@ -46,7 +46,6 @@ static void OpenAndAddDevice(int index)
 static Common::Event s_init_event;
 static Uint32 s_stop_event_type;
 static Uint32 s_populate_event_type;
-static Common::Event s_populated_event;
 static std::thread s_hotplug_thread;
 
 static bool HandleEventAndContinue(const SDL_Event& e)
@@ -64,9 +63,10 @@ static bool HandleEventAndContinue(const SDL_Event& e)
   }
   else if (e.type == s_populate_event_type)
   {
-    for (int i = 0; i < SDL_NumJoysticks(); ++i)
-      OpenAndAddDevice(i);
-    s_populated_event.Set();
+    g_controller_interface.PlatformPopulateDevices([] {
+      for (int i = 0; i < SDL_NumJoysticks(); ++i)
+        OpenAndAddDevice(i);
+    });
   }
   else if (e.type == s_stop_event_type)
   {
@@ -111,7 +111,8 @@ void Init()
       // Drain all of the events and add the initial joysticks before returning. Otherwise, the
       // individual joystick events as well as the custom populate event will be handled _after_
       // ControllerInterface::Init/RefreshDevices has cleared its list of devices, resulting in
-      // duplicate devices.
+      // duplicate devices. Adding devices will actually "fail" here, as the ControllerInterface
+      // hasn't finished initializing yet.
       SDL_Event e;
       while (SDL_PollEvent(&e) != 0)
       {
@@ -161,8 +162,6 @@ void PopulateDevices()
 
   SDL_Event populate_event{s_populate_event_type};
   SDL_PushEvent(&populate_event);
-
-  s_populated_event.Wait();
 #endif
 }
 
diff --git a/Source/Core/InputCommon/ControllerInterface/Wiimote/WiimoteController.cpp b/Source/Core/InputCommon/ControllerInterface/Wiimote/WiimoteController.cpp
index eaa56ef449..82e2cfedda 100644
--- a/Source/Core/InputCommon/ControllerInterface/Wiimote/WiimoteController.cpp
+++ b/Source/Core/InputCommon/ControllerInterface/Wiimote/WiimoteController.cpp
@@ -82,14 +82,14 @@ using UndetectableSignedAnalogInput = SignedInput<false>;
 class Motor final : public Core::Device::Output
 {
 public:
-  Motor(ControlState* value) : m_value(*value) {}
+  Motor(std::atomic<ControlState>* value) : m_value(*value) {}
 
   std::string GetName() const override { return "Motor"; }
 
   void SetState(ControlState state) override { m_value = state; }
 
 private:
-  ControlState& m_value;
+  std::atomic<ControlState>& m_value;
 };
 
 template <typename T>
@@ -128,13 +128,17 @@ void ReleaseDevices(std::optional<u32> count)
 
   // Remove up to "count" remotes (or all of them if nullopt).
   // Real wiimotes will be added to the pool.
-  g_controller_interface.RemoveDevice([&](const Core::Device* device) {
-    if (device->GetSource() != SOURCE_NAME || count == removed_devices)
-      return false;
-
-    ++removed_devices;
-    return true;
-  });
+  // Make sure to force the device removal immediately (as they are shared ptrs and
+  // they could be kept alive, preventing us from re-creating the device)
+  g_controller_interface.RemoveDevice(
+      [&](const Core::Device* device) {
+        if (device->GetSource() != SOURCE_NAME || count == removed_devices)
+          return false;
+
+        ++removed_devices;
+        return true;
+      },
+      true);
 }
 
 Device::Device(std::unique_ptr<WiimoteReal::Wiimote> wiimote) : m_wiimote(std::move(wiimote))
@@ -317,6 +321,12 @@ std::string Device::GetSource() const
   return SOURCE_NAME;
 }
 
+// Always add these at the end, given their hotplug nature
+int Device::GetSortPriority() const
+{
+  return -1;
+}
+
 void Device::RunTasks()
 {
   if (IsPerformingTask())
@@ -1367,7 +1377,8 @@ void Device::UpdateRumble()
 {
   static constexpr auto rumble_period = std::chrono::milliseconds(100);
 
-  const auto on_time = std::chrono::duration_cast<Clock::duration>(rumble_period * m_rumble_level);
+  const auto on_time =
+      std::chrono::duration_cast<Clock::duration>(rumble_period * m_rumble_level.load());
   const auto off_time = rumble_period - on_time;
 
   const auto now = Clock::now();
diff --git a/Source/Core/InputCommon/ControllerInterface/Wiimote/WiimoteController.h b/Source/Core/InputCommon/ControllerInterface/Wiimote/WiimoteController.h
index 4864e0abb3..4d93584a67 100644
--- a/Source/Core/InputCommon/ControllerInterface/Wiimote/WiimoteController.h
+++ b/Source/Core/InputCommon/ControllerInterface/Wiimote/WiimoteController.h
@@ -5,6 +5,7 @@
 #pragma once
 
 #include <array>
+#include <atomic>
 #include <chrono>
 #include <memory>
 #include <vector>
@@ -33,6 +34,7 @@ public:
 
   std::string GetName() const override;
   std::string GetSource() const override;
+  int GetSortPriority() const override;
 
   void UpdateInput() override;
 
@@ -261,7 +263,7 @@ private:
   bool m_rumble = false;
 
   // For pulse of rumble motor to simulate multiple levels.
-  ControlState m_rumble_level = 0;
+  std::atomic<ControlState> m_rumble_level;
   Clock::time_point m_last_rumble_change = Clock::now();
 
   // Assume mode is disabled so one gets set.
diff --git a/Source/Core/InputCommon/ControllerInterface/Win32/Win32.cpp b/Source/Core/InputCommon/ControllerInterface/Win32/Win32.cpp
index b7cf19a13d..066c50a693 100644
--- a/Source/Core/InputCommon/ControllerInterface/Win32/Win32.cpp
+++ b/Source/Core/InputCommon/ControllerInterface/Win32/Win32.cpp
@@ -11,6 +11,7 @@
 #include <thread>
 
 #include "Common/Event.h"
+#include "Common/Flag.h"
 #include "Common/Logging/Log.h"
 #include "Common/ScopeGuard.h"
 #include "Common/Thread.h"
@@ -19,20 +20,30 @@
 
 constexpr UINT WM_DOLPHIN_STOP = WM_USER;
 
-static Common::Event s_done_populating;
-static std::atomic<HWND> s_hwnd;
+static Common::Event s_received_device_change_event;
+// Dolphin's render window
+static HWND s_hwnd;
+// Windows messaging window (hidden)
 static HWND s_message_window;
 static std::thread s_thread;
+static Common::Flag s_first_populate_devices_asked;
 
 static LRESULT CALLBACK WindowProc(HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam)
 {
   if (message == WM_INPUT_DEVICE_CHANGE)
   {
-    g_controller_interface.PlatformPopulateDevices([] {
-      ciface::DInput::PopulateDevices(s_hwnd);
-      ciface::XInput::PopulateDevices();
-    });
-    s_done_populating.Set();
+    // Windows automatically sends this message before we ask for it and before we are "ready" to
+    // listen for it.
+    if (s_first_populate_devices_asked.IsSet())
+    {
+      s_received_device_change_event.Set();
+      // TODO: we could easily use the message passed alongside this event, which tells
+      // whether a device was added or removed, to avoid removing old, still connected, devices
+      g_controller_interface.PlatformPopulateDevices([] {
+        ciface::DInput::PopulateDevices(s_hwnd);
+        ciface::XInput::PopulateDevices();
+      });
+    }
   }
 
   return DefWindowProc(hwnd, message, wparam, lparam);
@@ -125,10 +136,14 @@ void ciface::Win32::PopulateDevices(void* hwnd)
   if (s_thread.joinable())
   {
     s_hwnd = static_cast<HWND>(hwnd);
-    s_done_populating.Reset();
+    s_first_populate_devices_asked.Set();
+    s_received_device_change_event.Reset();
+    // Do this forced devices refresh in the messaging thread so it won't cause any race conditions
     PostMessage(s_message_window, WM_INPUT_DEVICE_CHANGE, 0, 0);
-    if (!s_done_populating.WaitFor(std::chrono::seconds(10)))
-      ERROR_LOG_FMT(CONTROLLERINTERFACE, "win32 timed out when trying to populate devices");
+    std::thread([] {
+      if (!s_received_device_change_event.WaitFor(std::chrono::seconds(5)))
+        ERROR_LOG_FMT(CONTROLLERINTERFACE, "win32 timed out when trying to populate devices");
+    }).detach();
   }
   else
   {
@@ -137,6 +152,15 @@ void ciface::Win32::PopulateDevices(void* hwnd)
   }
 }
 
+void ciface::Win32::ChangeWindow(void* hwnd)
+{
+  if (s_thread.joinable())  // "Has init?"
+  {
+    s_hwnd = static_cast<HWND>(hwnd);
+    ciface::DInput::ChangeWindow(s_hwnd);
+  }
+}
+
 void ciface::Win32::DeInit()
 {
   NOTICE_LOG_FMT(CONTROLLERINTERFACE, "win32 DeInit");
@@ -145,7 +169,11 @@ void ciface::Win32::DeInit()
     PostMessage(s_message_window, WM_DOLPHIN_STOP, 0, 0);
     s_thread.join();
     s_message_window = nullptr;
+    s_received_device_change_event.Reset();
+    s_first_populate_devices_asked.Clear();
+    DInput::DeInit();
   }
+  s_hwnd = nullptr;
 
   XInput::DeInit();
 }
diff --git a/Source/Core/InputCommon/ControllerInterface/Win32/Win32.h b/Source/Core/InputCommon/ControllerInterface/Win32/Win32.h
index acf72d76a0..8d390022a4 100644
--- a/Source/Core/InputCommon/ControllerInterface/Win32/Win32.h
+++ b/Source/Core/InputCommon/ControllerInterface/Win32/Win32.h
@@ -8,5 +8,6 @@ namespace ciface::Win32
 {
 void Init(void* hwnd);
 void PopulateDevices(void* hwnd);
+void ChangeWindow(void* hwnd);
 void DeInit();
 }  // namespace ciface::Win32
diff --git a/Source/Core/InputCommon/ControllerInterface/evdev/evdev.cpp b/Source/Core/InputCommon/ControllerInterface/evdev/evdev.cpp
index 265d81ad40..cb13c65a1e 100644
--- a/Source/Core/InputCommon/ControllerInterface/evdev/evdev.cpp
+++ b/Source/Core/InputCommon/ControllerInterface/evdev/evdev.cpp
@@ -202,6 +202,9 @@ static int s_wakeup_eventfd;
 // There is no easy way to get the device name from only a dev node
 // during a device removed event, since libevdev can't work on removed devices;
 // sysfs is not stable, so this is probably the easiest way to get a name for a node.
+// This can, and will be modified by different thread, possibly concurrently,
+// as devices can be destroyed by any thread at any time. As of now it's protected
+// by ControllerInterface::m_devices_population_mutex.
 static std::map<std::string, std::weak_ptr<evdevDevice>> s_devnode_objects;
 
 static std::shared_ptr<evdevDevice>
@@ -260,9 +263,13 @@ static void AddDeviceNode(const char* devnode)
 
     // Remove and re-add device as naming and inputs may have changed.
     // This will also give it the correct index and invoke device change callbacks.
-    g_controller_interface.RemoveDevice([&evdev_device](const auto* device) {
-      return static_cast<const evdevDevice*>(device) == evdev_device.get();
-    });
+    // Make sure to force the device removal immediately (as they are shared ptrs and
+    // they could be kept alive, preventing us from re-creating the device)
+    g_controller_interface.RemoveDevice(
+        [&evdev_device](const auto* device) {
+          return static_cast<const evdevDevice*>(device) == evdev_device.get();
+        },
+        true);
 
     g_controller_interface.AddDevice(evdev_device);
   }
@@ -276,6 +283,8 @@ static void AddDeviceNode(const char* devnode)
       g_controller_interface.AddDevice(evdev_device);
   }
 
+  // If the devices failed to be added to g_controller_interface, it will be added here but then
+  // immediately removed in its destructor due to the shared ptr not having any references left
   s_devnode_objects.emplace(devnode, std::move(evdev_device));
 }
 
@@ -318,23 +327,30 @@ static void HotplugThreadFunc()
     if (!devnode)
       continue;
 
+    // Use g_controller_interface.PlatformPopulateDevices() to protect access around
+    // s_devnode_objects. Note that even if we get these events at the same time as a
+    // a PopulateDevices() request (e.g. on start up, we might get all the add events
+    // for connected devices), this won't ever cause duplicate devices as AddDeviceNode()
+    // automatically removes the old one if it already existed
     if (strcmp(action, "remove") == 0)
     {
-      std::shared_ptr<evdevDevice> ptr;
+      g_controller_interface.PlatformPopulateDevices([&devnode] {
+        std::shared_ptr<evdevDevice> ptr;
 
-      const auto it = s_devnode_objects.find(devnode);
-      if (it != s_devnode_objects.end())
-        ptr = it->second.lock();
+        const auto it = s_devnode_objects.find(devnode);
+        if (it != s_devnode_objects.end())
+          ptr = it->second.lock();
 
-      // If we don't recognize this device, ptr will be null and no device will be removed.
+        // If we don't recognize this device, ptr will be null and no device will be removed.
 
-      g_controller_interface.RemoveDevice([&ptr](const auto* device) {
-        return static_cast<const evdevDevice*>(device) == ptr.get();
+        g_controller_interface.RemoveDevice([&ptr](const auto* device) {
+          return static_cast<const evdevDevice*>(device) == ptr.get();
+        });
       });
     }
     else if (strcmp(action, "add") == 0)
     {
-      AddDeviceNode(devnode);
+      g_controller_interface.PlatformPopulateDevices([&devnode] { AddDeviceNode(devnode); });
     }
   }
   NOTICE_LOG_FMT(CONTROLLERINTERFACE, "evdev hotplug thread stopped");
@@ -376,8 +392,15 @@ void Init()
   StartHotplugThread();
 }
 
+// Only call this when ControllerInterface::m_devices_population_mutex is locked
 void PopulateDevices()
 {
+  // Don't run if we are not initialized
+  if (!s_hotplug_thread_running.IsSet())
+  {
+    return;
+  }
+
   // We use udev to iterate over all /dev/input/event* devices.
   // Note: the Linux kernel is currently limited to just 32 event devices. If
   // this ever changes, hopefully udev will take care of this.