From 3710484e442b631bf92141a115f8626bbe90fcee Mon Sep 17 00:00:00 2001 From: Davide Beatrici Date: Fri, 30 Dec 2022 17:57:13 +0100 Subject: [PATCH] FIX(client): Use SPSC queue for global shortcuts on Windows We've been receiving quite a few reports about the program seemingly freezing, such as https://forums.mumble.info/topic/14467-14-conflict-with-flight-control-rudder-pedals. Turns out there are HID devices, namely advanced controllers, that constantly send packets. The reason behind the behavior is probably to prevent games from losing track of the physical state. Qt's event dispatcher blows up and consumes all resources, leading to: QEventDispatcherWin32::registerTimer: Failed to create a timer (The current process has used all of its system allowance of handles for Window Manager objects.) As a result, the UI completely freezes due to events not being processed anymore. This commit fixes the issue by using an SPSC (Single Producer Single Consumer) queue. The event loop and 20 ms timer are removed. The latter was strictly used for XInput and G-keys polling. Both backends still work, potentially better than before: no more polling! Whenever a HID message is received, the relevant code is executed. --- src/mumble/CMakeLists.txt | 7 +- src/mumble/GlobalShortcut_win.cpp | 115 ++++++++++++++++++------------ src/mumble/GlobalShortcut_win.h | 67 ++++++++++++++--- 3 files changed, 133 insertions(+), 56 deletions(-) diff --git a/src/mumble/CMakeLists.txt b/src/mumble/CMakeLists.txt index 883a3697f..56d6c0453 100644 --- a/src/mumble/CMakeLists.txt +++ b/src/mumble/CMakeLists.txt @@ -595,12 +595,17 @@ if(WIN32) target_link_libraries(mumble_client_object_lib PUBLIC Qt5::QWindowsIntegrationPlugin) endif() + add_subdirectory("${3RDPARTY_DIR}/SPSCQueue" "${CMAKE_CURRENT_BINARY_DIR}/SPSCQueue" EXCLUDE_FROM_ALL) add_subdirectory("${3RDPARTY_DIR}/xinputcheck-build" "${CMAKE_CURRENT_BINARY_DIR}/xinputcheck" EXCLUDE_FROM_ALL) # Disable all warnings that the xinputcheck code may emit disable_warnings_for_all_targets_in("${3RDPARTY_DIR}/xinputcheck-build") - target_link_libraries(mumble_client_object_lib PUBLIC xinputcheck) + target_link_libraries(mumble_client_object_lib + PUBLIC + SPSCQueue + xinputcheck + ) if(MSVC) target_link_libraries(mumble_client_object_lib diff --git a/src/mumble/GlobalShortcut_win.cpp b/src/mumble/GlobalShortcut_win.cpp index 9f3c43cc3..200d2aa2f 100644 --- a/src/mumble/GlobalShortcut_win.cpp +++ b/src/mumble/GlobalShortcut_win.cpp @@ -16,9 +16,9 @@ #include #include +#include #include -#include #include extern "C" { @@ -312,22 +312,21 @@ QList< Shortcut > GlobalShortcutWin::migrateSettings(const QList< Shortcut > &ol } GlobalShortcutWin::GlobalShortcutWin() + : m_msgQueue(64) #ifdef USE_XBOXINPUT - : m_xinputDevices(0), m_xinputLastPacket() + , + m_xinputDevices(0), m_xinputLastPacket() #endif { // Register the MetaTypes if they have not already been registered (e.g in Settings) registerMetaTypes(); - connect(this, &GlobalShortcutWin::hidMessage, this, &GlobalShortcutWin::on_hidMessage); - connect(this, &GlobalShortcutWin::keyboardMessage, this, &GlobalShortcutWin::on_keyboardMessage); - connect(this, &GlobalShortcutWin::mouseMessage, this, &GlobalShortcutWin::on_mouseMessage); - - start(QThread::LowestPriority); + start(); } GlobalShortcutWin::~GlobalShortcutWin() { - quit(); + requestInterruption(); + m_condVar.notify_all(); wait(); } @@ -395,11 +394,35 @@ void GlobalShortcutWin::run() { qWarning("GlobalShortcutWindows: RegisterRawInputDevices() failed with error %u!", GetLastError()); } - QTimer timer; - connect(&timer, &QTimer::timeout, this, &GlobalShortcutWin::timeTicked); - timer.start(20); + std::mutex mutex; + std::unique_lock< decltype(mutex) > lock(mutex); - exec(); + do { + m_condVar.wait(lock); + + if (bNeedRemap) { + remap(); + } + + while (const std::unique_ptr< MsgRaw > *item = m_msgQueue.front()) { + const std::unique_ptr< MsgRaw > &msg = *item; + + switch (msg->type()) { + case MsgRaw::Hid: + processOther(); + processMsgHid(static_cast< MsgHid & >(*msg)); + break; + case MsgRaw::Keyboard: + processMsgKeyboard(static_cast< MsgKeyboard & >(*msg)); + break; + case MsgRaw::Mouse: + processMsgMouse(static_cast< MsgMouse & >(*msg)); + break; + } + + m_msgQueue.pop(); + } + } while (!isInterruptionRequested()); } void GlobalShortcutWin::injectRawInputMessage(HRAWINPUT handle) { @@ -417,7 +440,7 @@ void GlobalShortcutWin::injectRawInputMessage(HRAWINPUT handle) { switch (input->header.dwType) { case RIM_TYPEMOUSE: { const RAWMOUSE &mouse = input->data.mouse; - emit mouseMessage(mouse.usButtonFlags, mouse.usButtonData); + m_msgQueue.emplace(std::make_unique< MsgMouse >(mouse.usButtonFlags)); break; } case RIM_TYPEKEYBOARD: { @@ -433,24 +456,26 @@ void GlobalShortcutWin::injectRawInputMessage(HRAWINPUT handle) { return; } - emit keyboardMessage(keyboard.Flags, keyboard.MakeCode, keyboard.VKey); + m_msgQueue.emplace(std::make_unique< MsgKeyboard >(keyboard.Flags, keyboard.MakeCode, keyboard.VKey)); break; } case RIM_TYPEHID: { const RAWHID &hid = input->data.hid; - std::vector< char > reports(hid.dwSizeHid * hid.dwCount); - memcpy(reports.data(), hid.bRawData, reports.size()); - - emit hidMessage(input->header.hDevice, std::move(reports), hid.dwSizeHid); + MsgHid::RawReports reports(hid.bRawData, hid.dwSizeHid * hid.dwCount); + m_msgQueue.emplace(std::make_unique< MsgHid >(input->header.hDevice, reports, hid.dwSizeHid)); + break; } + default: + return; } + + m_condVar.notify_all(); } -void GlobalShortcutWin::on_hidMessage(const HANDLE deviceHandle, std::vector< char > reports, - const uint32_t reportSize) { - auto iter = m_devices.find(deviceHandle); +void GlobalShortcutWin::processMsgHid(MsgHid &msg) { + auto iter = m_devices.find(msg.deviceHandle); if (iter == m_devices.end()) { - iter = addDevice(deviceHandle); + iter = addDevice(msg.deviceHandle); if (iter == m_devices.cend()) { return; } @@ -466,7 +491,8 @@ void GlobalShortcutWin::on_hidMessage(const HANDLE deviceHandle, std::vector< ch ULONG nUsages = device.buttons.size(); std::vector< USAGE > usages(nUsages); - if (HidP_GetUsages(HidP_Input, HID_USAGE_PAGE_BUTTON, 0, &usages[0], &nUsages, data, &reports[0], reportSize) + if (HidP_GetUsages(HidP_Input, HID_USAGE_PAGE_BUTTON, 0, &usages[0], &nUsages, data, &msg.reports[0], + msg.reportSize) != HIDP_STATUS_SUCCESS) { return; } @@ -486,69 +512,69 @@ void GlobalShortcutWin::on_hidMessage(const HANDLE deviceHandle, std::vector< ch } } -void GlobalShortcutWin::on_keyboardMessage(const uint16_t flags, uint16_t scanCode, const uint16_t virtualKey) { - if (virtualKey == VK_NUMLOCK) { +void GlobalShortcutWin::processMsgKeyboard(MsgKeyboard &msg) { + if (msg.virtualKey == VK_NUMLOCK) { // Keys like “Pause / Break” and “Numlock” act strangely, // sometimes like they are not even the same physical key. // They use the so called escaped sequences, which we have to decipher. - scanCode = MapVirtualKey(virtualKey, MAPVK_VK_TO_VSC) | 0x100; + msg.scanCode = MapVirtualKey(msg.virtualKey, MAPVK_VK_TO_VSC) | 0x100; } // E0 and E1 are escape sequences used for certain special keys, such as PRINT and PAUSE/BREAK. // See https://www.win.tue.nl/~aeb/linux/kbd/scancodes-1.html. - if (flags & RI_KEY_E1) { + if (msg.flags & RI_KEY_E1) { // For escaped sequences, turn the virtual key into the correct scan code using MapVirtualKey(). // MapVirtualKey() is unable to map VK_PAUSE (this is a known bug), hence we map that by hand. - scanCode = virtualKey != VK_PAUSE ? MapVirtualKey(virtualKey, MAPVK_VK_TO_VSC) : 0x45; + msg.scanCode = msg.virtualKey != VK_PAUSE ? MapVirtualKey(msg.virtualKey, MAPVK_VK_TO_VSC) : 0x45; } InputKeyboard input = {}; - input.code = scanCode; - input.e0 = flags & RI_KEY_E0; + input.code = msg.scanCode; + input.e0 = msg.flags & RI_KEY_E0; - handleButton(QVariant::fromValue(input), !(flags & RI_KEY_BREAK)); + handleButton(QVariant::fromValue(input), !(msg.flags & RI_KEY_BREAK)); } -void GlobalShortcutWin::on_mouseMessage(const uint16_t flags, const uint16_t data) { +void GlobalShortcutWin::processMsgMouse(const MsgMouse &msg) { // Multiple mouse transitions can be contained in a single message. // See https://docs.microsoft.com/en-us/windows/win32/api/winuser/ns-winuser-rawmouse. - if (flags & RI_MOUSE_BUTTON_1_DOWN) { + if (msg.buttonFlags & RI_MOUSE_BUTTON_1_DOWN) { handleButton(QVariant::fromValue(InputMouse::Left), true); } - if (flags & RI_MOUSE_BUTTON_1_UP) { + if (msg.buttonFlags & RI_MOUSE_BUTTON_1_UP) { handleButton(QVariant::fromValue(InputMouse::Left), false); } - if (flags & RI_MOUSE_BUTTON_2_DOWN) { + if (msg.buttonFlags & RI_MOUSE_BUTTON_2_DOWN) { handleButton(QVariant::fromValue(InputMouse::Right), true); } - if (flags & RI_MOUSE_BUTTON_2_UP) { + if (msg.buttonFlags & RI_MOUSE_BUTTON_2_UP) { handleButton(QVariant::fromValue(InputMouse::Right), false); } - if (flags & RI_MOUSE_BUTTON_3_DOWN) { + if (msg.buttonFlags & RI_MOUSE_BUTTON_3_DOWN) { handleButton(QVariant::fromValue(InputMouse::Middle), true); } - if (flags & RI_MOUSE_BUTTON_3_UP) { + if (msg.buttonFlags & RI_MOUSE_BUTTON_3_UP) { handleButton(QVariant::fromValue(InputMouse::Middle), false); } - if (flags & RI_MOUSE_BUTTON_4_DOWN) { + if (msg.buttonFlags & RI_MOUSE_BUTTON_4_DOWN) { handleButton(QVariant::fromValue(InputMouse::Side_1), true); } - if (flags & RI_MOUSE_BUTTON_4_UP) { + if (msg.buttonFlags & RI_MOUSE_BUTTON_4_UP) { handleButton(QVariant::fromValue(InputMouse::Side_1), false); } - if (flags & RI_MOUSE_BUTTON_5_DOWN) { + if (msg.buttonFlags & RI_MOUSE_BUTTON_5_DOWN) { handleButton(QVariant::fromValue(InputMouse::Side_2), true); } - if (flags & RI_MOUSE_BUTTON_5_UP) { + if (msg.buttonFlags & RI_MOUSE_BUTTON_5_UP) { handleButton(QVariant::fromValue(InputMouse::Side_2), false); } } @@ -691,10 +717,7 @@ bool GlobalShortcutWin::xinputIsPressed(const uint8_t bit, const XboxInputState } } #endif -void GlobalShortcutWin::timeTicked() { - if (bNeedRemap) { - remap(); - } +void GlobalShortcutWin::processOther() { #ifdef USE_XBOXINPUT if (m_xinput && m_xinputDevices > 0) { for (uint8_t i = 0; i < XBOXINPUT_MAX_DEVICES; ++i) { diff --git a/src/mumble/GlobalShortcut_win.h b/src/mumble/GlobalShortcut_win.h index 1d8696919..5446bf514 100644 --- a/src/mumble/GlobalShortcut_win.h +++ b/src/mumble/GlobalShortcut_win.h @@ -12,9 +12,14 @@ # include "XboxInput.h" #endif +#include #include #include +#include + +#include + #ifdef USE_GKEY class GKeyLibrary; #endif @@ -44,17 +49,52 @@ public: ~GlobalShortcutWin() override; public slots: void deviceRemoved(const HANDLE deviceHandle); - void timeTicked(); - - void on_hidMessage(const HANDLE deviceHandle, std::vector< char > reports, const uint32_t reportSize); - void on_keyboardMessage(const uint16_t flags, uint16_t scanCode, const uint16_t virtualKey); - void on_mouseMessage(const uint16_t flags, const uint16_t data); -signals: - void hidMessage(const HANDLE deviceHandle, std::vector< char > reports, const uint32_t reportSize); - void keyboardMessage(const uint16_t flags, uint16_t scanCode, const uint16_t virtualKey); - void mouseMessage(const uint16_t flags, const uint16_t data); protected: + class MsgRaw { + public: + enum Type : uint8_t { Hid, Keyboard, Mouse }; + + constexpr Type type() { return m_type; }; + + protected: + MsgRaw(const Type type) : m_type(type) {} + + private: + Type m_type; + }; + + class MsgHid : public MsgRaw { + public: + using RawReports = gsl::span< const BYTE >; + + MsgHid(HANDLE deviceHandle, const RawReports rawReports, const DWORD reportSize) + : MsgRaw(Hid), deviceHandle(deviceHandle), reports(rawReports.size()), reportSize(reportSize) { + memcpy(reports.data(), rawReports.data(), reports.size()); + } + + HANDLE deviceHandle; + std::vector< char > reports; + DWORD reportSize; + }; + + class MsgKeyboard : public MsgRaw { + public: + MsgKeyboard(const USHORT flags, const USHORT scanCode, const USHORT virtualKey) + : MsgRaw(Keyboard), flags(flags), scanCode(scanCode), virtualKey(virtualKey) {} + + USHORT flags; + USHORT scanCode; + USHORT virtualKey; + }; + + class MsgMouse : public MsgRaw { + public: + MsgMouse(const USHORT buttonFlags) : MsgRaw(Mouse), buttonFlags(buttonFlags) {} + + USHORT buttonFlags; + }; + struct Device { std::string name; std::string prefix; @@ -67,6 +107,15 @@ protected: #endif }; + void processMsgHid(MsgHid &msg); + void processMsgKeyboard(MsgKeyboard &msg); + void processMsgMouse(const MsgMouse &msg); + void processOther(); + + std::condition_variable m_condVar; + + rigtorp::SPSCQueue< std::unique_ptr< MsgRaw > > m_msgQueue; + typedef std::unordered_map< HANDLE, Device > DeviceMap; DeviceMap m_devices; #ifdef USE_XBOXINPUT