Rework the HotkeyManager and KeyboardHook interop classes (#4710)

* Use GetAsyncKeyState calls and remove additional thread usage

* Removed Environment.Exit

src/common/interop/HotkeyManager.cpp

@@ -5,15 +5,14 @@ using namespace interop;
 
 HotkeyManager::HotkeyManager()
 {
-    keyboardEventCallback = gcnew KeyboardEventCallback(this, &HotkeyManager::KeyboardEventProc); 
+    keyboardEventCallback = gcnew KeyboardEventCallback(this, &HotkeyManager::KeyboardEventProc);
     isActiveCallback = gcnew IsActiveCallback(this, &HotkeyManager::IsActiveProc);
     filterKeyboardCallback = gcnew FilterKeyboardEvent(this, &HotkeyManager::FilterKeyboardProc);
 
     keyboardHook = gcnew KeyboardHook(
         keyboardEventCallback,
         isActiveCallback,
-        filterKeyboardCallback
+        filterKeyboardCallback);
-    );
     hotkeys = gcnew Dictionary<HOTKEY_HANDLE, HotkeyCallback ^>();
     pressedKeys = gcnew Hotkey();
     keyboardHook->Start();
@@ -25,8 +24,9 @@ HotkeyManager::~HotkeyManager()
 }
 
 // When all Shortcut keys are pressed, fire the HotkeyCallback event.
-void HotkeyManager::KeyboardEventProc(KeyboardEvent^ ev)
+void HotkeyManager::KeyboardEventProc(KeyboardEvent ^ ev)
 {
+    // pressedKeys always stores the latest keyboard state
     auto pressedKeysHandle = GetHotkeyHandle(pressedKeys);
     if (hotkeys->ContainsKey(pressedKeysHandle))
     {
@@ -42,22 +42,24 @@ bool HotkeyManager::IsActiveProc()
 }
 
 // KeyboardEvent callback is only fired for relevant key events.
-bool HotkeyManager::FilterKeyboardProc(KeyboardEvent^ ev)
+bool HotkeyManager::FilterKeyboardProc(KeyboardEvent ^ ev)
 {
-    auto oldHandle = GetHotkeyHandle(pressedKeys);
+    // Updating the pressed keys here so we know if the keypress event should be propagated or not.
-
+    pressedKeys->Win = (GetAsyncKeyState(VK_LWIN) & 0x8000) || (GetAsyncKeyState(VK_RWIN) & 0x8000);
-    // Updating the pressed keys here so we know if the keypress event 
+    pressedKeys->Ctrl = GetAsyncKeyState(VK_CONTROL) & 0x8000;
-    // should be propagated or not.
+    pressedKeys->Alt = GetAsyncKeyState(VK_MENU) & 0x8000;
-    UpdatePressedKeys(ev);
+    pressedKeys->Shift = GetAsyncKeyState(VK_SHIFT) & 0x8000;
+    pressedKeys->Key = ev->key;
 
+    // Convert to hotkey handle
     auto pressedKeysHandle = GetHotkeyHandle(pressedKeys);
 
-    // Check if the hotkey matches the pressed keys, and check if the pressed keys aren't duplicate
+    // Check if any hotkey matches the pressed keys if the current key event is a key down event
-    // (there shouldn't be auto repeating hotkeys)
+    if ((ev->message == WM_KEYDOWN || ev->message == WM_SYSKEYDOWN) && hotkeys->ContainsKey(pressedKeysHandle))
-    if (hotkeys->ContainsKey(pressedKeysHandle) && oldHandle != pressedKeysHandle)
     {
         return true;
     }
+
     return false;
 }
 
@@ -83,51 +85,3 @@ HOTKEY_HANDLE HotkeyManager::GetHotkeyHandle(Hotkey ^ hotkey)
     handle |= hotkey->Alt << 11;
     return handle;
 }
-
-void HotkeyManager::UpdatePressedKey(DWORD code, bool replaceWith, unsigned char replaceWithKey)
-{
-    switch (code)
-    {
-    case VK_LWIN:
-    case VK_RWIN:
-        pressedKeys->Win = replaceWith;
-        break;
-    case VK_CONTROL:
-    case VK_LCONTROL:
-    case VK_RCONTROL:
-        pressedKeys->Ctrl = replaceWith;
-        break;
-    case VK_SHIFT:
-    case VK_LSHIFT:
-    case VK_RSHIFT:
-        pressedKeys->Shift = replaceWith;
-        break;
-    case VK_MENU:
-    case VK_LMENU:
-    case VK_RMENU:
-        pressedKeys->Alt = replaceWith;
-        break;
-    default:
-        pressedKeys->Key = replaceWithKey;
-        break;
-    }
-}
-
-void HotkeyManager::UpdatePressedKeys(KeyboardEvent ^ ev)
-{
-    switch (ev->message)
-    {
-    case WM_KEYDOWN:
-    case WM_SYSKEYDOWN:
-    {
-        UpdatePressedKey(ev->key, true, ev->key);
-    }
-    break;
-    case WM_KEYUP:
-    case WM_SYSKEYUP:
-    {
-        UpdatePressedKey(ev->key, false, 0);
-    }
-    break;
-    }
-}

src/common/interop/HotkeyManager.h

@@ -26,7 +26,7 @@ public
 public
     delegate void HotkeyCallback();
 
-typedef unsigned short HOTKEY_HANDLE;
+    typedef unsigned short HOTKEY_HANDLE;
 
 public
     ref class HotkeyManager
@@ -46,12 +46,9 @@ public
         IsActiveCallback ^ isActiveCallback;
         FilterKeyboardEvent ^ filterKeyboardCallback;
 
-
         void KeyboardEventProc(KeyboardEvent ^ ev);
         bool IsActiveProc();
         bool FilterKeyboardProc(KeyboardEvent ^ ev);
         HOTKEY_HANDLE GetHotkeyHandle(Hotkey ^ hotkey);
-        void UpdatePressedKeys(KeyboardEvent ^ ev);
-        void UpdatePressedKey(DWORD code, bool replaceWith, unsigned char replaceWithKey);
     };
 }

src/common/interop/KeyboardHook.cpp

@@ -15,8 +15,6 @@ KeyboardHook::KeyboardHook(
     IsActiveCallback ^ isActiveCallback,
     FilterKeyboardEvent ^ filterKeyboardEvent)
 {
-    kbEventDispatch = gcnew Thread(gcnew ThreadStart(this, &KeyboardHook::DispatchProc));
-    queue = gcnew Queue<KeyboardEvent ^>();
     this->keyboardEventCallback = keyboardEventCallback;
     this->isActiveCallback = isActiveCallback;
     this->filterKeyboardEvent = filterKeyboardEvent;
@@ -24,44 +22,10 @@ KeyboardHook::KeyboardHook(
 
 KeyboardHook::~KeyboardHook()
 {
-    quit = true;
-
-    // Notify the DispatchProc thread so that it isn't stuck at the Wait step
-    Monitor::Enter(queue);
-    Monitor::Pulse(queue);
-    Monitor::Exit(queue);
-
-    kbEventDispatch->Join();
-
     // Unregister low level hook procedure
     UnhookWindowsHookEx(hookHandle);
 }
 
-void KeyboardHook::DispatchProc()
-{
-    Monitor::Enter(queue);
-    quit = false;
-    while (!quit)
-    {
-        if (queue->Count == 0)
-        {
-            Monitor::Wait(queue);
-            continue;
-        }
-        auto nextEv = queue->Dequeue();
-
-        // Release lock while callback is being invoked
-        Monitor::Exit(queue);
-
-        keyboardEventCallback->Invoke(nextEv);
-
-        // Re-aquire lock
-        Monitor::Enter(queue);
-    }
-
-    Monitor::Exit(queue);
-}
-
 void KeyboardHook::Start()
 {
     hookProc = gcnew HookProcDelegate(this, &KeyboardHook::HookProc);
@@ -85,8 +49,6 @@ void KeyboardHook::Start()
             throw std::exception("SetWindowsHookEx failed.");
         }
     }
-
-    kbEventDispatch->Start();
 }
 
 LRESULT CALLBACK KeyboardHook::HookProc(int nCode, WPARAM wParam, LPARAM lParam)
@@ -101,10 +63,7 @@ LRESULT CALLBACK KeyboardHook::HookProc(int nCode, WPARAM wParam, LPARAM lParam)
             return CallNextHookEx(hookHandle, nCode, wParam, lParam);
         }
 
-        Monitor::Enter(queue);
+        keyboardEventCallback->Invoke(ev);
-        queue->Enqueue(ev);
-        Monitor::Pulse(queue);
-        Monitor::Exit(queue);
         return 1;
     }
     return CallNextHookEx(hookHandle, nCode, wParam, lParam);

src/common/interop/KeyboardHook.h

@@ -33,16 +33,12 @@ public
 
     private:
         delegate LRESULT HookProcDelegate(int nCode, WPARAM wParam, LPARAM lParam);
-        Thread ^ kbEventDispatch;
-        Queue<KeyboardEvent ^> ^ queue;
         KeyboardEventCallback ^ keyboardEventCallback;
         IsActiveCallback ^ isActiveCallback;
         FilterKeyboardEvent ^ filterKeyboardEvent;
-        bool quit;
         HHOOK hookHandle;
         HookProcDelegate ^ hookProc;
 
-        void DispatchProc();
         LRESULT CALLBACK HookProc(int nCode, WPARAM wParam, LPARAM lParam);
     };
 

src/core/Microsoft.PowerToys.Settings.UI.Runner/Program.cs

@@ -74,9 +74,6 @@ namespace Microsoft.PowerToys.Settings.UI.Runner
                         MessageBoxButton.OK);
                     app.Shutdown();
                 }
-
-                // Terminate all threads of the process
-                Environment.Exit(0);
             }
         }