From 51986e763d0b5149e0838300b4613975949cd861 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Wed, 17 Dec 2025 13:51:57 +1000 Subject: [PATCH] InputManager: Fix some low-risk races during reload --- src/duckstation-qt/mainwindow.cpp | 4 ++-- src/duckstation-qt/qthost.cpp | 33 ------------------------------ src/duckstation-qt/qthost.h | 3 --- src/util/input_manager.cpp | 34 +++++++++++++++++++++++-------- src/util/input_manager.h | 4 ---- 5 files changed, 27 insertions(+), 51 deletions(-) diff --git a/src/duckstation-qt/mainwindow.cpp b/src/duckstation-qt/mainwindow.cpp index 9195adbe0..d5938be3a 100644 --- a/src/duckstation-qt/mainwindow.cpp +++ b/src/duckstation-qt/mainwindow.cpp @@ -723,7 +723,7 @@ void MainWindow::recreate() show(); // We need to close input sources, because e.g. DInput uses our window handle. - g_emu_thread->closeInputSources(); + Host::RunOnCPUThread(&InputManager::CloseSources, true); // Ensure we don't get a display widget creation sent to us. const bool was_display_created = hasDisplayWidget(); @@ -760,7 +760,7 @@ void MainWindow::recreate() deleteLater(); // Reload the sources we just closed. - g_emu_thread->reloadInputSources(); + Host::RunOnCPUThread(&System::ReloadInputSources); if (controller_settings_window_pos.has_value()) { diff --git a/src/duckstation-qt/qthost.cpp b/src/duckstation-qt/qthost.cpp index 6ba968bb9..e889838be 100644 --- a/src/duckstation-qt/qthost.cpp +++ b/src/duckstation-qt/qthost.cpp @@ -1106,17 +1106,6 @@ void Host::OnGPUThreadRunIdleChanged(bool is_active) g_emu_thread->setGPUThreadRunIdle(is_active); } -void EmuThread::reloadInputSources() -{ - if (!isCurrentThread()) - { - QMetaObject::invokeMethod(this, &EmuThread::reloadInputSources, Qt::QueuedConnection); - return; - } - - System::ReloadInputSources(); -} - void EmuThread::reloadInputBindings() { if (!isCurrentThread()) @@ -1128,28 +1117,6 @@ void EmuThread::reloadInputBindings() System::ReloadInputBindings(); } -void EmuThread::reloadInputDevices() -{ - if (!isCurrentThread()) - { - QMetaObject::invokeMethod(this, &EmuThread::reloadInputDevices, Qt::QueuedConnection); - return; - } - - InputManager::ReloadDevices(); -} - -void EmuThread::closeInputSources() -{ - if (!isCurrentThread()) - { - QMetaObject::invokeMethod(this, &EmuThread::reloadInputDevices, Qt::BlockingQueuedConnection); - return; - } - - InputManager::CloseSources(); -} - void EmuThread::confirmActionIfMemoryCardBusy(const QString& action, bool cancel_resume_on_accept, std::function callback) const { diff --git a/src/duckstation-qt/qthost.h b/src/duckstation-qt/qthost.h index b39fea88b..e5a8a8826 100644 --- a/src/duckstation-qt/qthost.h +++ b/src/duckstation-qt/qthost.h @@ -141,10 +141,7 @@ public: void reloadCheats(bool reload_files, bool reload_enabled_list, bool verbose, bool verbose_if_changed); void updateEmuFolders(); void updateControllerSettings(); - void reloadInputSources(); void reloadInputBindings(); - void reloadInputDevices(); - void closeInputSources(); void startFullscreenUI(); void stopFullscreenUI(); void exitFullscreenUI(); diff --git a/src/util/input_manager.cpp b/src/util/input_manager.cpp index ac6a3edfc..373f025d2 100644 --- a/src/util/input_manager.cpp +++ b/src/util/input_manager.cpp @@ -151,7 +151,10 @@ static void AddPadBindings(const SettingsInterface& si, const std::string& secti static void InternalReloadBindings(const SettingsInterface& binding_si, const SettingsInterface& hotkey_binding_si); static void SynchronizePadEffectBindings(InputBindingKey key); static void UpdateContinuedVibration(); +static void InternalPauseVibration(); +static void InternalClearEffects(); static void GenerateRelativeMouseEvents(); +static void ReloadDevices(); static bool DoEventHook(InputBindingKey key, float value); static bool PreprocessEvent(InputBindingKey key, float value, GenericInputBinding generic_key); @@ -1889,7 +1892,7 @@ void InputManager::SetPadVibrationIntensity(u32 pad_index, u32 bind_index, float } } -void InputManager::PauseVibration() +void InputManager::InternalPauseVibration() { for (PadVibrationBinding& binding : s_state.pad_vibration_array) { @@ -1966,10 +1969,8 @@ void InputManager::SetPadLEDState(u32 pad_index, float intensity) } } -void InputManager::ClearEffects() +void InputManager::InternalClearEffects() { - PauseVibration(); - for (PadLEDBinding& pad : s_state.pad_led_array) { if (pad.last_intensity == 0.0f) @@ -2212,9 +2213,9 @@ void InputManager::InternalReloadBindings(const SettingsInterface& binding_si, void InputManager::ReloadBindings(const SettingsInterface& binding_si, const SettingsInterface& hotkey_binding_si) { - PauseVibration(); - std::unique_lock lock(s_state.mutex); + InternalPauseVibration(); + InternalClearEffects(); InternalReloadBindings(binding_si, hotkey_binding_si); UpdateRelativeMouseMode(); @@ -2224,12 +2225,23 @@ void InputManager::ReloadBindings(const SettingsInterface& binding_si, const Set // Source Management // ------------------------------------------------------------------------ -bool InputManager::ReloadDevices() +void InputManager::ClearEffects() { std::unique_lock lock(s_state.mutex); + InternalPauseVibration(); + InternalClearEffects(); +} +void InputManager::PauseVibration() +{ + std::unique_lock lock(s_state.mutex); + InternalPauseVibration(); +} + +void InputManager::ReloadDevices() +{ bool changed = false; - + std::unique_lock lock(s_state.mutex); for (u32 i = FIRST_EXTERNAL_INPUT_SOURCE; i < LAST_EXTERNAL_INPUT_SOURCE; i++) { if (s_state.input_sources[i]) @@ -2237,8 +2249,12 @@ bool InputManager::ReloadDevices() } UpdatePointerCount(); + if (!changed) + return; - return changed; + // need to release the lock, since otherwise we would risk a lock ordering issue + lock.unlock(); + System::ReloadInputBindings(); } void InputManager::CloseSources() diff --git a/src/util/input_manager.h b/src/util/input_manager.h index 58e7400c6..7d1611631 100644 --- a/src/util/input_manager.h +++ b/src/util/input_manager.h @@ -300,10 +300,6 @@ void ReloadBindings(const SettingsInterface& si, const SettingsInterface& hotkey void ReloadSourcesAndBindings(const SettingsInterface& si, const SettingsInterface& hotkey_binding_si, std::unique_lock& settings_lock); -/// Called when a device change is triggered by the system (DBT_DEVNODES_CHANGED on Windows). -/// Returns true if any device changes are detected. -bool ReloadDevices(); - /// Shuts down any enabled input sources. void CloseSources();