ShuriZma
/
dolphin
mirror of https://github.com/dolphin-emu/dolphin.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

ControllerInterface: Fix Windows deadlock 

Remove the redundant s_populate_mutex and only use
ControllerInterface::m_devices_population_mutex instead to prevent a
deadlock caused by locking them in opposite orders.

The device population functions in the win32 InputBackend previously
locked s_populate_mutex first before calling various functions that
locked m_devices_population_mutex. This normally worked but
ControllerInterface::RefreshDevices locks m_devices_population_mutex
first and then calls HandleWindowChange which then locked
s_populate_mutex, potentially causing the deadlock.

Fix this by using PlatformPopulateDevices to lock
m_devices_population_mutex before running the code that was previously
protected by s_populate_mutex. The functions in question lock
m_devices_population_mutex anyway, so this shouldn't meaningfully
increase contention on the lock.

Reproduction steps:

* Let Dolphin finish startup.
* In Win32.cpp::OnDevicesChanged set a breakpoint on the call to
  PlatformPopulateDevices. When the breakpoint is triggered the function
  will have locked s_populate_mutex, but since PlatformPopulateDevices
  won't have run yet m_devices_population_mutex will still be unlocked.
* Unplug a device from your computer.
* Wait for the breakpoint to trigger. (At this point you can plug the
  device back in).
* Freeze the ntdll.dll!TppWorkerThread() that triggered the breakpoint.
* Resume Dolphin and start a game.
* Core::EmuThread will call ControllerInterface::ChangeWindow which
  calls RefreshDevices. It locks m_devices_population_mutex, then calls
  InputBackend::HandleWindowChange, which tries to lock
  s_populate_mutex.
* Unfreeze ntdll.dll!TppWorkerThread().

At this point EmuThread and TppWorkerThread are deadlocked. The UI is
still responsive since the Host thread is unaffected, but trying to stop
the game or close Dolphin normally will fail since EmuThread is unable
to stop.
This commit is contained in:
Dentomologist 2025-06-28 12:33:34 -07:00
parent 3a32013740
commit c613d3ca10
1 changed files with 8 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
Source/Core/InputCommon/ControllerInterface/Win32/Win32.cpp
View File

@ -10,8 +10,6 @@
#include <hidclass.h>
#include <mutex>
#include "Common/Flag.h"
#include "Common/Logging/Log.h"
#include "InputCommon/ControllerInterface/DInput/DInput.h"
@ -20,7 +18,6 @@
#pragma comment(lib, "OneCoreUAP.Lib")
static std::mutex s_populate_mutex;
// TODO is this really needed?
static Common::Flag s_first_populate_devices_asked;
static HCMNOTIFICATION s_notify_handle;
@ -52,7 +49,6 @@ _Pre_satisfies_(EventDataSize >= sizeof(CM_NOTIFY_EVENT_DATA)) static DWORD CALL
// listen for it.
if (s_first_populate_devices_asked.IsSet())
{
std::lock_guard lk_population(s_populate_mutex);
// 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([&] {
@ -96,17 +92,18 @@ InputBackend::InputBackend(ControllerInterface* controller_interface)
void InputBackend::PopulateDevices()
{
std::lock_guard lk_population(s_populate_mutex);
s_first_populate_devices_asked.Set();
ciface::DInput::PopulateDevices(GetHWND());
ciface::XInput::PopulateDevices();
ciface::WGInput::PopulateDevices();
g_controller_interface.PlatformPopulateDevices([this] {
s_first_populate_devices_asked.Set();
ciface::DInput::PopulateDevices(GetHWND());
ciface::XInput::PopulateDevices();
ciface::WGInput::PopulateDevices();
});
}
void InputBackend::HandleWindowChange()
{
std::lock_guard lk_population(s_populate_mutex);
ciface::DInput::ChangeWindow(GetHWND());
g_controller_interface.PlatformPopulateDevices(
[this] { ciface::DInput::ChangeWindow(GetHWND()); });
}
InputBackend::~InputBackend()