[KBM] Fix for handle leak

* Ensure the lifetime of CPowerRenameProcessUI on the worker thread to prevent AV during shutdown

* Ensure worker thread progress dialog pointer is valid.  Also add a call to StopProgressDialog from the worker thread as the progress dialog can be particular about thread affinity for that method call.

Co-authored-by: Chris Davis (EDGE) <chrdavis@microsoft.com>

src/modules/keyboardmanager/KeyboardManagerEngineLibrary/KeyboardManager.cpp

@@ -51,6 +51,7 @@ KeyboardManager::KeyboardManager()
         loadingSettings = false;
     };
 
+    editorIsRunningEvent = CreateEvent(nullptr, true, false, KeyboardManagerConstants::EditorWindowEventName.c_str());
     settingsEventWaiter = EventWaiter(KeyboardManagerConstants::SettingsEventName, changeSettingsCallback);
 }
 
@@ -127,8 +128,7 @@ intptr_t KeyboardManager::HandleKeyboardHookEvent(LowlevelKeyboardEvent* data) n
     }
 
     // Suspend remapping if remap key/shortcut window is opened
-    auto h = CreateEvent(nullptr, true, false, KeyboardManagerConstants::EditorWindowEventName.c_str());
-    if (h != nullptr && WaitForSingleObject(h, 0) == WAIT_OBJECT_0)
+    if (editorIsRunningEvent != nullptr && WaitForSingleObject(editorIsRunningEvent, 0) == WAIT_OBJECT_0)
     {
         return 0;
     }

src/modules/keyboardmanager/KeyboardManagerEngineLibrary/KeyboardManager.h

@@ -9,6 +9,14 @@ public:
     // Constructor
     KeyboardManager();
 
+    ~KeyboardManager()
+    {
+        if (editorIsRunningEvent)
+        {
+            CloseHandle(editorIsRunningEvent);
+        }
+    }
+
     void StartLowlevelKeyboardHook();
     void StopLowlevelKeyboardHook();
 
@@ -43,6 +51,8 @@ private:
     // Load settings from the file.
     void LoadSettings();
 
+    HANDLE editorIsRunningEvent = nullptr;
+
     // Function called by the hook procedure to handle the events. This is the starting point function for remapping
     intptr_t HandleKeyboardHookEvent(LowlevelKeyboardEvent* data) noexcept;
 };

src/modules/powerrename/ui/PowerRenameUI.cpp

@@ -1524,7 +1524,12 @@ HRESULT CPowerRenameProgressUI::Start()
 {
     _Cleanup();
     m_canceled = false;
+    AddRef();
     m_workerThreadHandle = CreateThread(nullptr, 0, s_workerThread, this, 0, nullptr);
+    if (!m_workerThreadHandle)
+    {
+        Release();
+    }
     return (m_workerThreadHandle) ? S_OK : E_FAIL;
 }
 
@@ -1539,20 +1544,22 @@ DWORD WINAPI CPowerRenameProgressUI::s_workerThread(_In_ void* pv)
 
             SetTimer(hwndMessage, TIMERID_CHECKCANCELED, CANCEL_CHECK_INTERVAL, nullptr);
 
-            if (SUCCEEDED(CoCreateInstance(CLSID_ProgressDialog, NULL, CLSCTX_INPROC, IID_PPV_ARGS(&pThis->m_sppd))))
+            CComPtr<IProgressDialog> sppd;
+            if (SUCCEEDED(CoCreateInstance(CLSID_ProgressDialog, NULL, CLSCTX_INPROC, IID_PPV_ARGS(&sppd))))
             {
+                pThis->m_sppd = sppd;
                 wchar_t buff[100] = { 0 };
                 LoadString(g_hInst, IDS_LOADING, buff, ARRAYSIZE(buff));
-                pThis->m_sppd->SetLine(1, buff, FALSE, NULL);
+                sppd->SetLine(1, buff, FALSE, NULL);
                 LoadString(g_hInst, IDS_LOADING_MSG, buff, ARRAYSIZE(buff));
-                pThis->m_sppd->SetLine(2, buff, FALSE, NULL);
+                sppd->SetLine(2, buff, FALSE, NULL);
                 LoadString(g_hInst, IDS_APP_TITLE, buff, ARRAYSIZE(buff));
-                pThis->m_sppd->SetTitle(buff);
+                sppd->SetTitle(buff);
                 SetTimer(hwndMessage, TIMERID_CHECKCANCELED, CANCEL_CHECK_INTERVAL, nullptr);
-                pThis->m_sppd->StartProgressDialog(NULL, NULL, PROGDLG_MARQUEEPROGRESS, NULL);
+                sppd->StartProgressDialog(NULL, NULL, PROGDLG_MARQUEEPROGRESS, NULL);
             }
 
-            while (pThis->m_sppd && !pThis->m_canceled)
+            while (pThis->m_sppd && !sppd->HasUserCancelled())
             {
                 MSG msg;
                 while (PeekMessage(&msg, nullptr, 0, 0, PM_REMOVE))
@@ -1562,8 +1569,13 @@ DWORD WINAPI CPowerRenameProgressUI::s_workerThread(_In_ void* pv)
                 }
             }
 
+            // Ensure dialog is stopped
+            sppd->StopProgressDialog();
+
             KillTimer(hwndMessage, TIMERID_CHECKCANCELED);
             DestroyWindow(hwndMessage);
+
+            pThis->Release();
         }
 
         CoUninitialize();
@@ -1588,9 +1600,12 @@ void CPowerRenameProgressUI::_Cleanup()
 
     if (m_workerThreadHandle)
     {
+        // Wait for up to 5 seconds for worker thread to finish
+        WaitForSingleObject(m_workerThreadHandle, 5000);
         CloseHandle(m_workerThreadHandle);
         m_workerThreadHandle = nullptr;
     }
+    
 }
 
 void CPowerRenameProgressUI::_UpdateCancelState()
@@ -1635,6 +1650,7 @@ LRESULT CPowerRenameProgressUI::_WndProc(_In_ HWND hwnd, _In_ UINT msg, _In_ WPA
         break;
 
     case WM_DESTROY:
+        _UpdateCancelState();
         KillTimer(hwnd, TIMERID_CHECKCANCELED);
         break;