diff --git a/Source/Core/Common/Logging/Log.h b/Source/Core/Common/Logging/Log.h index 9e45881b1c..282b7ac8c4 100644 --- a/Source/Core/Common/Logging/Log.h +++ b/Source/Core/Common/Logging/Log.h @@ -54,7 +54,6 @@ enum LOG_TYPE OSHLE, OSREPORT, OSREPORT_HLE, - PAD, PIXELENGINE, PROCESSORINTERFACE, POWERPC, diff --git a/Source/Core/Common/Logging/LogManager.cpp b/Source/Core/Common/Logging/LogManager.cpp index 2c58148fb2..9cc984998e 100644 --- a/Source/Core/Common/Logging/LogManager.cpp +++ b/Source/Core/Common/Logging/LogManager.cpp @@ -157,7 +157,6 @@ LogManager::LogManager() m_log[OSHLE] = {"HLE", "OSHLE"}; m_log[OSREPORT] = {"OSREPORT", "OSReport EXI"}; m_log[OSREPORT_HLE] = {"OSREPORT_HLE", "OSReport HLE"}; - m_log[PAD] = {"PAD", "Pad"}; m_log[PIXELENGINE] = {"PE", "Pixel Engine"}; m_log[PROCESSORINTERFACE] = {"PI", "Processor Interface"}; m_log[POWERPC] = {"PowerPC", "PowerPC IBM CPU"}; diff --git a/Source/Core/Core/HW/WiimoteReal/WiimoteReal.cpp b/Source/Core/Core/HW/WiimoteReal/WiimoteReal.cpp index 680e85d61c..83d262ebeb 100644 --- a/Source/Core/Core/HW/WiimoteReal/WiimoteReal.cpp +++ b/Source/Core/Core/HW/WiimoteReal/WiimoteReal.cpp @@ -96,8 +96,15 @@ static void TryToFillWiimoteSlot(u32 index) s_wiimote_pool.erase(s_wiimote_pool.begin()); } +void PopulateDevices() +{ + // There is a very small chance of deadlocks if we didn't do it in another thread + s_wiimote_scanner.PopulateDevices(); +} + // Attempts to fill enabled real wiimote slots. // Push/pull wiimotes to/from ControllerInterface as needed. +// Should be called PopulateDevices() to be in line with other implementations. void ProcessWiimotePool() { std::lock_guard lk(g_wiimotes_mutex); @@ -545,7 +552,7 @@ void WiimoteScanner::StopThread() void WiimoteScanner::SetScanMode(WiimoteScanMode scan_mode) { m_scan_mode.store(scan_mode); - m_scan_mode_changed_event.Set(); + m_scan_mode_changed_or_population_event.Set(); } bool WiimoteScanner::IsReady() const @@ -610,6 +617,12 @@ void WiimoteScanner::PoolThreadFunc() } } +void WiimoteScanner::PopulateDevices() +{ + m_populate_devices.Set(); + m_scan_mode_changed_or_population_event.Set(); +} + void WiimoteScanner::ThreadFunc() { std::thread pool_thread(&WiimoteScanner::PoolThreadFunc, this); @@ -634,7 +647,12 @@ void WiimoteScanner::ThreadFunc() while (m_scan_thread_running.IsSet()) { - m_scan_mode_changed_event.WaitFor(std::chrono::milliseconds(500)); + m_scan_mode_changed_or_population_event.WaitFor(std::chrono::milliseconds(500)); + + if (m_populate_devices.TestAndClear()) + { + g_controller_interface.PlatformPopulateDevices([] { ProcessWiimotePool(); }); + } // Does stuff needed to detect disconnects on Windows for (const auto& backend : m_backends) @@ -675,7 +693,7 @@ void WiimoteScanner::ThreadFunc() } AddWiimoteToPool(std::unique_ptr(wiimote)); - ProcessWiimotePool(); + g_controller_interface.PlatformPopulateDevices([] { ProcessWiimotePool(); }); } if (found_board) @@ -956,12 +974,12 @@ void HandleWiimoteSourceChange(unsigned int index) if (auto removed_wiimote = std::move(g_wiimotes[index])) AddWiimoteToPool(std::move(removed_wiimote)); }); - ProcessWiimotePool(); + g_controller_interface.PlatformPopulateDevices([] { ProcessWiimotePool(); }); } void HandleWiimotesInControllerInterfaceSettingChange() { - ProcessWiimotePool(); + g_controller_interface.PlatformPopulateDevices([] { ProcessWiimotePool(); }); } } // namespace WiimoteReal diff --git a/Source/Core/Core/HW/WiimoteReal/WiimoteReal.h b/Source/Core/Core/HW/WiimoteReal/WiimoteReal.h index 44d6a22af4..701371d4bd 100644 --- a/Source/Core/Core/HW/WiimoteReal/WiimoteReal.h +++ b/Source/Core/Core/HW/WiimoteReal/WiimoteReal.h @@ -171,6 +171,7 @@ public: void StartThread(); void StopThread(); void SetScanMode(WiimoteScanMode scan_mode); + void PopulateDevices(); bool IsReady() const; @@ -183,7 +184,8 @@ private: std::thread m_scan_thread; Common::Flag m_scan_thread_running; - Common::Event m_scan_mode_changed_event; + Common::Flag m_populate_devices; + Common::Event m_scan_mode_changed_or_population_event; std::atomic m_scan_mode{WiimoteScanMode::DO_NOT_SCAN}; }; @@ -204,6 +206,7 @@ void InitAdapterClass(); #endif void HandleWiimotesInControllerInterfaceSettingChange(); +void PopulateDevices(); void ProcessWiimotePool(); } // namespace WiimoteReal diff --git a/Source/Core/DolphinQt/Config/Mapping/IOWindow.cpp b/Source/Core/DolphinQt/Config/Mapping/IOWindow.cpp index 27c17ce5fb..6d5c149024 100644 --- a/Source/Core/DolphinQt/Config/Mapping/IOWindow.cpp +++ b/Source/Core/DolphinQt/Config/Mapping/IOWindow.cpp @@ -9,8 +9,6 @@ #include #include -#include -#include #include #include #include @@ -31,6 +29,7 @@ #include "DolphinQt/Config/Mapping/MappingWindow.h" #include "DolphinQt/QtUtils/BlockUserInputFilter.h" #include "DolphinQt/QtUtils/ModalMessageBox.h" +#include "DolphinQt/Settings.h" #include "InputCommon/ControlReference/ControlReference.h" #include "InputCommon/ControlReference/ExpressionParser.h" @@ -220,6 +219,8 @@ IOWindow::IOWindow(MappingWidget* parent, ControllerEmu::EmulatedController* con CreateMainLayout(); connect(parent, &MappingWidget::Update, this, &IOWindow::Update); + connect(parent->GetParent(), &MappingWindow::ConfigChanged, this, &IOWindow::ConfigChanged); + connect(&Settings::Instance(), &Settings::ConfigChanged, this, &IOWindow::ConfigChanged); setWindowTitle(type == IOWindow::Type::Input ? tr("Configure Input") : tr("Configure Output")); setWindowFlags(windowFlags() & ~Qt::WindowContextHelpButtonHint); @@ -229,7 +230,7 @@ IOWindow::IOWindow(MappingWidget* parent, ControllerEmu::EmulatedController* con ConnectWidgets(); } -std::shared_ptr IOWindow::GetSelectedDevice() +std::shared_ptr IOWindow::GetSelectedDevice() const { return m_selected_device; } @@ -258,7 +259,7 @@ void IOWindow::CreateMainLayout() m_expression_text->setFont(QFontDatabase::systemFont(QFontDatabase::FixedFont)); new ControlExpressionSyntaxHighlighter(m_expression_text->document()); - m_operators_combo = new QComboBoxWithMouseWheelDisabled(); + m_operators_combo = new QComboBoxWithMouseWheelDisabled(this); m_operators_combo->addItem(tr("Operators")); m_operators_combo->insertSeparator(1); if (m_type == Type::Input) @@ -340,6 +341,7 @@ void IOWindow::CreateMainLayout() m_option_list->horizontalHeader()->setSectionResizeMode(1, QHeaderView::Fixed); m_option_list->setItemDelegate(new InputStateDelegate(this, 1, [&](int row) { + std::lock_guard lock(m_selected_device_mutex); // Clamp off negative values but allow greater than one in the text display. return std::max(GetSelectedDevice()->Inputs()[row]->GetState(), 0.0); })); @@ -400,7 +402,7 @@ void IOWindow::CreateMainLayout() void IOWindow::ConfigChanged() { const QSignalBlocker blocker(this); - const auto lock = m_controller->GetStateLock(); + const auto lock = ControllerEmu::EmulatedController::GetStateLock(); // ensure m_parse_text is in the right state UpdateExpression(m_reference->GetExpression(), UpdateMode::Force); @@ -410,10 +412,10 @@ void IOWindow::ConfigChanged() m_range_spinbox->setValue(m_reference->range * SLIDER_TICK_COUNT); m_range_slider->setValue(m_reference->range * SLIDER_TICK_COUNT); - m_devq = m_controller->GetDefaultDevice(); + if (m_devq.ToString().empty()) + m_devq = m_controller->GetDefaultDevice(); UpdateDeviceList(); - UpdateOptionList(); } void IOWindow::Update() @@ -425,6 +427,8 @@ void IOWindow::Update() void IOWindow::ConnectWidgets() { connect(m_select_button, &QPushButton::clicked, [this] { AppendSelectedOption(); }); + connect(&Settings::Instance(), &Settings::ReleaseDevices, this, &IOWindow::ReleaseDevices); + connect(&Settings::Instance(), &Settings::DevicesChanged, this, &IOWindow::UpdateDeviceList); connect(m_detect_button, &QPushButton::clicked, this, &IOWindow::OnDetectButtonPressed); connect(m_test_button, &QPushButton::clicked, this, &IOWindow::OnTestButtonPressed); @@ -479,16 +483,19 @@ void IOWindow::ConnectWidgets() void IOWindow::AppendSelectedOption() { - if (m_option_list->currentItem() == nullptr) + if (m_option_list->currentRow() < 0) return; m_expression_text->insertPlainText(MappingCommon::GetExpressionForControl( - m_option_list->currentItem()->text(), m_devq, m_controller->GetDefaultDevice())); + m_option_list->item(m_option_list->currentRow(), 0)->text(), m_devq, + m_controller->GetDefaultDevice())); } -void IOWindow::OnDeviceChanged(const QString& device) +void IOWindow::OnDeviceChanged() { - m_devq.FromString(device.toStdString()); + const std::string device_name = + m_devices_combo->count() > 0 ? m_devices_combo->currentData().toString().toStdString() : ""; + m_devq.FromString(device_name); UpdateOptionList(); } @@ -500,7 +507,7 @@ void IOWindow::OnDialogButtonPressed(QAbstractButton* button) return; } - const auto lock = m_controller->GetStateLock(); + const auto lock = ControllerEmu::EmulatedController::GetStateLock(); UpdateExpression(m_expression_text->toPlainText().toStdString()); m_original_expression = m_reference->GetExpression(); @@ -525,6 +532,7 @@ void IOWindow::OnDetectButtonPressed() const auto list = m_option_list->findItems(expression, Qt::MatchFixedString); + // Try to select the first. If this fails, the last selected item would still appear as such if (!list.empty()) m_option_list->setCurrentItem(list[0]); } @@ -541,8 +549,15 @@ void IOWindow::OnRangeChanged(int value) m_range_slider->setValue(m_reference->range * SLIDER_TICK_COUNT); } +void IOWindow::ReleaseDevices() +{ + std::lock_guard lock(m_selected_device_mutex); + m_selected_device = nullptr; +} + void IOWindow::UpdateOptionList() { + std::lock_guard lock(m_selected_device_mutex); m_selected_device = g_controller_interface.FindDevice(m_devq); m_option_list->setRowCount(0); @@ -575,13 +590,62 @@ void IOWindow::UpdateOptionList() void IOWindow::UpdateDeviceList() { + const QSignalBlocker blocker(m_devices_combo); + + const auto previous_device_name = m_devices_combo->currentData().toString().toStdString(); + m_devices_combo->clear(); + // Default to the default device or to the first device if there isn't a default. + // Try to the keep the previous selected device, mark it as disconnected if it's gone, as it could + // reconnect soon after if this is a devices refresh and it would be annoying to lose the value. + const auto default_device_name = m_controller->GetDefaultDevice().ToString(); + int default_device_index = -1; + int previous_device_index = -1; for (const auto& name : g_controller_interface.GetAllDeviceStrings()) - m_devices_combo->addItem(QString::fromStdString(name)); + { + QString qname = QString(); + if (name == default_device_name) + { + default_device_index = m_devices_combo->count(); + // Specify "default" even if we only have one device + qname.append(QLatin1Char{'['} + tr("default") + QStringLiteral("] ")); + } + if (name == previous_device_name) + { + previous_device_index = m_devices_combo->count(); + } + qname.append(QString::fromStdString(name)); + m_devices_combo->addItem(qname, QString::fromStdString(name)); + } - m_devices_combo->setCurrentText( - QString::fromStdString(m_controller->GetDefaultDevice().ToString())); + if (previous_device_index >= 0) + { + m_devices_combo->setCurrentIndex(previous_device_index); + } + else if (!previous_device_name.empty()) + { + const QString qname = QString::fromStdString(previous_device_name); + QString adjusted_qname; + if (previous_device_name == default_device_name) + { + adjusted_qname.append(QLatin1Char{'['} + tr("default") + QStringLiteral("] ")); + } + adjusted_qname.append(QLatin1Char{'['} + tr("disconnected") + QStringLiteral("] ")) + .append(qname); + m_devices_combo->addItem(adjusted_qname, qname); + m_devices_combo->setCurrentIndex(m_devices_combo->count() - 1); + } + else if (default_device_index >= 0) + { + m_devices_combo->setCurrentIndex(default_device_index); + } + else if (m_devices_combo->count() > 0) + { + m_devices_combo->setCurrentIndex(0); + } + // The device object might have changed so we need to always refresh it + OnDeviceChanged(); } void IOWindow::UpdateExpression(std::string new_expression, UpdateMode mode) diff --git a/Source/Core/DolphinQt/Config/Mapping/IOWindow.h b/Source/Core/DolphinQt/Config/Mapping/IOWindow.h index 320ebefde4..7d22ddbc0d 100644 --- a/Source/Core/DolphinQt/Config/Mapping/IOWindow.h +++ b/Source/Core/DolphinQt/Config/Mapping/IOWindow.h @@ -5,6 +5,7 @@ #pragma once #include +#include #include #include @@ -69,16 +70,16 @@ public: explicit IOWindow(MappingWidget* parent, ControllerEmu::EmulatedController* m_controller, ControlReference* ref, Type type); - std::shared_ptr GetSelectedDevice(); - private: + std::shared_ptr GetSelectedDevice() const; + void CreateMainLayout(); void ConnectWidgets(); void ConfigChanged(); void Update(); void OnDialogButtonPressed(QAbstractButton* button); - void OnDeviceChanged(const QString& device); + void OnDeviceChanged(); void OnDetectButtonPressed(); void OnTestButtonPressed(); void OnRangeChanged(int range); @@ -86,6 +87,7 @@ private: void AppendSelectedOption(); void UpdateOptionList(); void UpdateDeviceList(); + void ReleaseDevices(); enum class UpdateMode { @@ -135,4 +137,5 @@ private: ciface::Core::DeviceQualifier m_devq; Type m_type; std::shared_ptr m_selected_device; + std::mutex m_selected_device_mutex; }; diff --git a/Source/Core/DolphinQt/Config/Mapping/MappingWindow.cpp b/Source/Core/DolphinQt/Config/Mapping/MappingWindow.cpp index ac6230cec4..f3aed4eea0 100644 --- a/Source/Core/DolphinQt/Config/Mapping/MappingWindow.cpp +++ b/Source/Core/DolphinQt/Config/Mapping/MappingWindow.cpp @@ -328,7 +328,7 @@ bool MappingWindow::IsMappingAllDevices() const void MappingWindow::RefreshDevices() { - Core::RunAsCPUThread([&] { g_controller_interface.RefreshDevices(); }); + g_controller_interface.RefreshDevices(); } void MappingWindow::OnGlobalDevicesChanged() diff --git a/Source/Core/DolphinQt/Host.cpp b/Source/Core/DolphinQt/Host.cpp index b0cfb507bd..f85b0b7262 100644 --- a/Source/Core/DolphinQt/Host.cpp +++ b/Source/Core/DolphinQt/Host.cpp @@ -35,6 +35,8 @@ #include "VideoCommon/RenderBase.h" #include "VideoCommon/VideoConfig.h" +static thread_local bool tls_is_host_thread = false; + Host::Host() { State::SetOnAfterLoadCallback([] { Host_UpdateDisasmDialog(); }); @@ -51,6 +53,16 @@ Host* Host::GetInstance() return s_instance; } +void Host::DeclareAsHostThread() +{ + tls_is_host_thread = true; +} + +bool Host::IsHostThread() +{ + return tls_is_host_thread; +} + void Host::SetRenderHandle(void* handle) { m_render_to_main = Config::Get(Config::MAIN_RENDER_TO_MAIN); @@ -62,8 +74,7 @@ void Host::SetRenderHandle(void* handle) if (g_renderer) { g_renderer->ChangeSurface(handle); - if (g_controller_interface.IsInit()) - g_controller_interface.ChangeWindow(handle); + g_controller_interface.ChangeWindow(handle); } } diff --git a/Source/Core/DolphinQt/Host.h b/Source/Core/DolphinQt/Host.h index 6be28d5464..90c8b7042d 100644 --- a/Source/Core/DolphinQt/Host.h +++ b/Source/Core/DolphinQt/Host.h @@ -22,6 +22,9 @@ public: static Host* GetInstance(); + void DeclareAsHostThread(); + bool IsHostThread(); + bool GetRenderFocus(); bool GetRenderFullFocus(); bool GetRenderFullscreen(); diff --git a/Source/Core/DolphinQt/Main.cpp b/Source/Core/DolphinQt/Main.cpp index 9efad48c35..1399a40391 100644 --- a/Source/Core/DolphinQt/Main.cpp +++ b/Source/Core/DolphinQt/Main.cpp @@ -120,6 +120,8 @@ int WINAPI wWinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, PWSTR pCmdLine } #endif + Host::GetInstance()->DeclareAsHostThread(); + #ifdef __APPLE__ // On macOS, a command line option matching the format "-psn_X_XXXXXX" is passed when // the application is launched for the first time. This is to set the "ProcessSerialNumber", diff --git a/Source/Core/DolphinQt/MainWindow.cpp b/Source/Core/DolphinQt/MainWindow.cpp index 3b01fec11b..265155f2b9 100644 --- a/Source/Core/DolphinQt/MainWindow.cpp +++ b/Source/Core/DolphinQt/MainWindow.cpp @@ -789,7 +789,7 @@ void MainWindow::TogglePause() void MainWindow::OnStopComplete() { m_stop_requested = false; - HideRenderWidget(); + HideRenderWidget(true, m_exit_requested); #ifdef USE_DISCORD_PRESENCE if (!m_netplay_dialog->isVisible()) Discord::UpdateDiscordPresence(); @@ -1099,7 +1099,7 @@ void MainWindow::ShowRenderWidget() } } -void MainWindow::HideRenderWidget(bool reinit) +void MainWindow::HideRenderWidget(bool reinit, bool is_exit) { if (m_rendering_to_main) { @@ -1136,7 +1136,9 @@ void MainWindow::HideRenderWidget(bool reinit) // The controller interface will still be registered to the old render widget, if the core // has booted. Therefore, we should re-bind it to the main window for now. When the core // is next started, it will be swapped back to the new render widget. - g_controller_interface.ChangeWindow(GetWindowSystemInfo(windowHandle()).render_window); + g_controller_interface.ChangeWindow(GetWindowSystemInfo(windowHandle()).render_window, + is_exit ? ControllerInterface::WindowChangeReason::Exit : + ControllerInterface::WindowChangeReason::Other); } } diff --git a/Source/Core/DolphinQt/MainWindow.h b/Source/Core/DolphinQt/MainWindow.h index 79383b3541..7d9e97de28 100644 --- a/Source/Core/DolphinQt/MainWindow.h +++ b/Source/Core/DolphinQt/MainWindow.h @@ -142,7 +142,7 @@ private: const std::optional& savestate_path = {}); void StartGame(std::unique_ptr&& parameters); void ShowRenderWidget(); - void HideRenderWidget(bool reinit = true); + void HideRenderWidget(bool reinit = true, bool is_exit = false); void ShowSettingsWindow(); void ShowGeneralWindow(); diff --git a/Source/Core/DolphinQt/Settings.cpp b/Source/Core/DolphinQt/Settings.cpp index 7541a9087f..3f9fb663f0 100644 --- a/Source/Core/DolphinQt/Settings.cpp +++ b/Source/Core/DolphinQt/Settings.cpp @@ -25,6 +25,7 @@ #include "Core/NetPlayClient.h" #include "Core/NetPlayServer.h" +#include "DolphinQt/Host.h" #include "DolphinQt/QtUtils/QueueOnObject.h" #include "InputCommon/ControllerInterface/ControllerInterface.h" @@ -44,8 +45,24 @@ Settings::Settings() Config::AddConfigChangedCallback( [this] { QueueOnObject(this, [this] { emit ConfigChanged(); }); }); - g_controller_interface.RegisterDevicesChangedCallback( - [this] { QueueOnObject(this, [this] { emit DevicesChanged(); }); }); + g_controller_interface.RegisterDevicesChangedCallback([this] { + if (Host::GetInstance()->IsHostThread()) + { + emit DevicesChanged(); + } + else + { + // Any device shared_ptr in the host thread needs to be released immediately as otherwise + // they'd continue living until the queued event has run, but some devices can't be recreated + // until they are destroyed. + // This is safe from any thread. Devices will be refreshed and re-acquired and in + // DevicesChanged(). Waiting on QueueOnObject() to have finished running was not feasible as + // it would cause deadlocks without heavy workarounds. + emit ReleaseDevices(); + + QueueOnObject(this, [this] { emit DevicesChanged(); }); + } + }); } Settings::~Settings() = default; diff --git a/Source/Core/DolphinQt/Settings.h b/Source/Core/DolphinQt/Settings.h index d5fd90989c..e2c5dbde9e 100644 --- a/Source/Core/DolphinQt/Settings.h +++ b/Source/Core/DolphinQt/Settings.h @@ -192,6 +192,7 @@ signals: void AutoUpdateTrackChanged(const QString& mode); void FallbackRegionChanged(const DiscIO::Region& region); void AnalyticsToggled(bool enabled); + void ReleaseDevices(); void DevicesChanged(); void SDCardInsertionChanged(bool inserted); void USBKeyboardConnectionChanged(bool connected); 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 +#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 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 callback) @@ -147,12 +218,18 @@ void ControllerInterface::PlatformPopulateDevices(std::function 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_ptrs are freed up - // BEFORE we shutdown the backends. - InvokeDevicesChangedCallbacks(); + // Update control references so shared_ptrs 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::AddDevice(std::shared_ptr device) +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(); +} + +bool ControllerInterface::AddDevice(std::shared_ptr 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 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& a, + const std::shared_ptr& 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 callback) +void ControllerInterface::RemoveDevice(std::function 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::functionUpdateInput(); + } } } @@ -307,7 +441,7 @@ Common::Vec2 ControllerInterface::GetWindowInputScale() const ControllerInterface::HotplugCallbackHandle ControllerInterface::RegisterDevicesChangedCallback(std::function callback) { - std::lock_guard 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 callba // Unregister a device callback. void ControllerInterface::UnregisterDevicesChangedCallback(const HotplugCallbackHandle& handle) { - std::lock_guard 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 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>::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 device); - void RemoveDevice(std::function callback); + bool AddDevice(std::shared_ptr 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 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 callback); bool IsInit() const { return m_is_init; } void UpdateInput(); @@ -87,10 +112,17 @@ public: static ciface::InputChannel GetCurrentInputChannel(); private: + void ClearDevices(); + std::list> 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 m_is_init; - std::atomic 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 m_populating_devices_counter; WindowSystemInfo m_wsi; std::atomic 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 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& Inputs() const { return m_inputs; } const std::vector& 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> 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*)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. - 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))) + if (!s_idi8 && FAILED(DirectInput8Create(GetModuleHandle(nullptr), DIRECTINPUT_VERSION, + IID_IDirectInput8, (LPVOID*)&s_idi8, nullptr))) { - ERROR_LOG_FMT(PAD, "DirectInput8Create failed."); + ERROR_LOG_FMT(CONTROLLERINTERFACE, "DirectInput8Create failed."); return; } - InitKeyboardMouse(idi8, hwnd); - InitJoystick(idi8, hwnd); + // 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(); }); - idi8->Release(); + InitKeyboardMouse(s_idi8, hwnd); + InitJoystick(s_idi8, hwnd); +} + +void ChangeWindow(HWND hwnd) +{ + if (s_idi8) // Has init? Ignore if called before the first PopulateDevices() + { + // 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); + + SetKeyboardMouseWindow(hwnd); + InitJoystick(s_idi8, hwnd); + } +} + +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 #include +#include #include #include #include @@ -29,6 +30,7 @@ struct GUIDComparator }; static std::set 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(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 -#include +#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(kb_device, mo_device, hwnd)); + g_controller_interface.AddDevice(std::make_shared(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>; +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 #include #include -#include #include #include @@ -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 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 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 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 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(); 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( @@ -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; class Motor final : public Core::Device::Output { public: - Motor(ControlState* value) : m_value(*value) {} + Motor(std::atomic* 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& m_value; }; template @@ -128,13 +128,17 @@ void ReleaseDevices(std::optional 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; + // 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; - }); + ++removed_devices; + return true; + }, + true); } Device::Device(std::unique_ptr 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(rumble_period * m_rumble_level); + const auto on_time = + std::chrono::duration_cast(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 +#include #include #include #include @@ -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 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 #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 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); - 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); + 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> s_devnode_objects; static std::shared_ptr @@ -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(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(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 ptr; + g_controller_interface.PlatformPopulateDevices([&devnode] { + std::shared_ptr 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(device) == ptr.get(); + g_controller_interface.RemoveDevice([&ptr](const auto* device) { + return static_cast(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.