[KBM] Moved unregistering of key delays to always run on the dispatcher thread to avoid mutex re-entrancy (#6959)

* Moved unregistering of key delays to always run on the dispatcher thread

* Updated comments

src/modules/keyboardmanager/common/KeyDelay.cpp

@@ -1,6 +1,7 @@
 #include "pch.h"
 #include "KeyDelay.h"
 
+// NOTE: The destructor should never be called on the DelayThread, i.e. from any of shortPress, longPress or longPressReleased, as it will re-enter the mutex. Even if the mutex is removed it will deadlock because of the join statement
 KeyDelay::~KeyDelay()
 {
     std::unique_lock<std::mutex> l(_queueMutex);

src/modules/keyboardmanager/ui/ShortcutControl.cpp

@@ -355,6 +355,7 @@ void ShortcutControl::createDetectShortcutWindow(winrt::Windows::Foundation::IIn
         onAccept();
     });
 
+    // NOTE: UnregisterKeys should never be called on the DelayThread, as it will re-enter the mutex. To avoid this it is run on the dispatcher thread
     keyboardManagerState.RegisterKeyDelay(
         VK_RETURN,
         selectDetectedShortcutAndResetKeys,
@@ -367,19 +368,24 @@ void ShortcutControl::createDetectShortcutWindow(winrt::Windows::Foundation::IIn
                     onPressEnter();
                 });
         },
-        [onReleaseEnter](DWORD) {
+        [onReleaseEnter, detectShortcutBox](DWORD) {
-            onReleaseEnter();
+            detectShortcutBox.Dispatcher().RunAsync(
+                Windows::UI::Core::CoreDispatcherPriority::Normal,
+                [onReleaseEnter]() {
+                    onReleaseEnter();
+                });
         });
 
     TextBlock cancelButtonText;
     cancelButtonText.Text(GET_RESOURCE_STRING(IDS_CANCEL_BUTTON));
 
-    Button cancelButton;
+    auto onCancel = [&keyboardManagerState,
-    cancelButton.HorizontalAlignment(HorizontalAlignment::Stretch);
+                     detectShortcutBox,
-    cancelButton.Margin({ 2, 2, 2, 2 });
+                     unregisterKeys,
-    cancelButton.Content(cancelButtonText);
+                     isSingleKeyWindow,
-    // Cancel button
+                     parentWindow] {
-    cancelButton.Click([detectShortcutBox, unregisterKeys, &keyboardManagerState, isSingleKeyWindow, parentWindow](winrt::Windows::Foundation::IInspectable const& sender, RoutedEventArgs const&) {
+        detectShortcutBox.Hide();
+
         // Reset the keyboard manager UI state
         keyboardManagerState.ResetUIState();
         if (isSingleKeyWindow)
@@ -393,31 +399,27 @@ void ShortcutControl::createDetectShortcutWindow(winrt::Windows::Foundation::IIn
             keyboardManagerState.SetUIState(KeyboardManagerUIState::EditShortcutsWindowActivated, parentWindow);
         }
         unregisterKeys();
-        detectShortcutBox.Hide();
+    };
+
+    Button cancelButton;
+    cancelButton.HorizontalAlignment(HorizontalAlignment::Stretch);
+    cancelButton.Margin({ 2, 2, 2, 2 });
+    cancelButton.Content(cancelButtonText);
+    // Cancel button
+    cancelButton.Click([onCancel](winrt::Windows::Foundation::IInspectable const& sender, RoutedEventArgs const&) {
+        onCancel();
     });
 
+    // NOTE: UnregisterKeys should never be called on the DelayThread, as it will re-enter the mutex. To avoid this it is run on the dispatcher thread
     keyboardManagerState.RegisterKeyDelay(
         VK_ESCAPE,
         selectDetectedShortcutAndResetKeys,
-        [&keyboardManagerState, detectShortcutBox, unregisterKeys, isSingleKeyWindow, parentWindow](DWORD) {
+        [onCancel, detectShortcutBox](DWORD) {
             detectShortcutBox.Dispatcher().RunAsync(
                 Windows::UI::Core::CoreDispatcherPriority::Normal,
-                [detectShortcutBox] {
+                [onCancel] {
-                    detectShortcutBox.Hide();
+                    onCancel();
                 });
-
-            keyboardManagerState.ResetUIState();
-            if (isSingleKeyWindow)
-            {
-                // Revert UI state back to Edit Keyboard window
-                keyboardManagerState.SetUIState(KeyboardManagerUIState::EditKeyboardWindowActivated, parentWindow);
-            }
-            else
-            {
-                // Revert UI state back to Edit Shortcut window
-                keyboardManagerState.SetUIState(KeyboardManagerUIState::EditShortcutsWindowActivated, parentWindow);
-            }
-            unregisterKeys();
         },
         nullptr);
 

src/modules/keyboardmanager/ui/SingleKeyRemapControl.cpp

@@ -269,6 +269,7 @@ void SingleKeyRemapControl::createDetectKeyWindow(winrt::Windows::Foundation::II
         onAccept();
     });
 
+    // NOTE: UnregisterKeys should never be called on the DelayThread, as it will re-enter the mutex. To avoid this it is run on the dispatcher thread
     keyboardManagerState.RegisterKeyDelay(
         VK_RETURN,
         std::bind(&KeyboardManagerState::SelectDetectedRemapKey, &keyboardManagerState, std::placeholders::_1),
@@ -281,41 +282,48 @@ void SingleKeyRemapControl::createDetectKeyWindow(winrt::Windows::Foundation::II
                     onPressEnter();
                 });
         },
-        [onReleaseEnter](DWORD) {
+        [onReleaseEnter, detectRemapKeyBox](DWORD) {
-            onReleaseEnter();
+            detectRemapKeyBox.Dispatcher().RunAsync(
+                Windows::UI::Core::CoreDispatcherPriority::Normal,
+                [onReleaseEnter]() {
+                    onReleaseEnter();
+                });
         });
 
     TextBlock cancelButtonText;
     cancelButtonText.Text(GET_RESOURCE_STRING(IDS_CANCEL_BUTTON));
 
+    auto onCancel = [&keyboardManagerState,
+                     detectRemapKeyBox,
+                     unregisterKeys] {
+        detectRemapKeyBox.Hide();
+
+        // Reset the keyboard manager UI state
+        keyboardManagerState.ResetUIState();
+        // Revert UI state back to Edit Keyboard window
+        keyboardManagerState.SetUIState(KeyboardManagerUIState::EditKeyboardWindowActivated, EditKeyboardWindowHandle);
+        unregisterKeys();
+    };
+
     Button cancelButton;
     cancelButton.HorizontalAlignment(HorizontalAlignment::Stretch);
     cancelButton.Margin({ 2, 2, 2, 2 });
     cancelButton.Content(cancelButtonText);
     // Cancel button
-    cancelButton.Click([detectRemapKeyBox, unregisterKeys, &keyboardManagerState](winrt::Windows::Foundation::IInspectable const& sender, RoutedEventArgs const&) {
+    cancelButton.Click([onCancel](winrt::Windows::Foundation::IInspectable const& sender, RoutedEventArgs const&) {
-        // Reset the keyboard manager UI state
+        onCancel();
-        keyboardManagerState.ResetUIState();
-        // Revert UI state back to Edit Keyboard window
-        keyboardManagerState.SetUIState(KeyboardManagerUIState::EditKeyboardWindowActivated, EditKeyboardWindowHandle);
-        unregisterKeys();
-        detectRemapKeyBox.Hide();
     });
 
+    // NOTE: UnregisterKeys should never be called on the DelayThread, as it will re-enter the mutex. To avoid this it is run on the dispatcher thread
     keyboardManagerState.RegisterKeyDelay(
         VK_ESCAPE,
         std::bind(&KeyboardManagerState::SelectDetectedRemapKey, &keyboardManagerState, std::placeholders::_1),
-        [&keyboardManagerState, detectRemapKeyBox, unregisterKeys](DWORD) {
+        [onCancel, detectRemapKeyBox](DWORD) {
             detectRemapKeyBox.Dispatcher().RunAsync(
                 Windows::UI::Core::CoreDispatcherPriority::Normal,
-                [detectRemapKeyBox] {
+                [onCancel] {
-                    detectRemapKeyBox.Hide();
+                    onCancel();
                 });
-
-            keyboardManagerState.ResetUIState();
-            // Revert UI state back to Edit Keyboard window
-            keyboardManagerState.SetUIState(KeyboardManagerUIState::EditKeyboardWindowActivated, EditKeyboardWindowHandle);
-            unregisterKeys();
         },
         nullptr);