diff --git a/installer/PowerToysSetupCustomActionsVNext/CustomAction.cpp b/installer/PowerToysSetupCustomActionsVNext/CustomAction.cpp index 4a83582dc2..11221c43aa 100644 --- a/installer/PowerToysSetupCustomActionsVNext/CustomAction.cpp +++ b/installer/PowerToysSetupCustomActionsVNext/CustomAction.cpp @@ -1579,7 +1579,7 @@ UINT __stdcall TerminateProcessesCA(MSIHANDLE hInstall) } processes.resize(bytes / sizeof(processes[0])); - std::array processesToTerminate = { + std::array processesToTerminate = { L"PowerToys.PowerLauncher.exe", L"PowerToys.Settings.exe", L"PowerToys.AdvancedPaste.exe", @@ -1623,6 +1623,7 @@ UINT __stdcall TerminateProcessesCA(MSIHANDLE hInstall) L"PowerToys.WorkspacesWindowArranger.exe", L"Microsoft.CmdPal.UI.exe", L"Microsoft.CmdPal.Ext.PowerToys.exe", + L"PowerToys.ShortcutGuide.exe", L"PowerToys.ZoomIt.exe", L"PowerToys.exe", }; diff --git a/src/common/interop/Constants.cpp b/src/common/interop/Constants.cpp index 1df141d733..c71c9a9ba3 100644 --- a/src/common/interop/Constants.cpp +++ b/src/common/interop/Constants.cpp @@ -163,6 +163,12 @@ namespace winrt::PowerToys::Interop::implementation { return CommonSharedConstants::POWERACCENT_EXIT_EVENT; } + + hstring Constants::ShortcutGuideExitEvent() + { + return CommonSharedConstants::SHORTCUT_GUIDE_EXIT_EVENT; + } + hstring Constants::ShortcutGuideTriggerEvent() { return CommonSharedConstants::SHORTCUT_GUIDE_TRIGGER_EVENT; @@ -317,4 +323,3 @@ namespace winrt::PowerToys::Interop::implementation return CommonSharedConstants::KEYBOARD_MANAGER_ENGINE_INSTANCE_MUTEX; } } - diff --git a/src/common/interop/Constants.h b/src/common/interop/Constants.h index 964cf3f444..05a5e4747c 100644 --- a/src/common/interop/Constants.h +++ b/src/common/interop/Constants.h @@ -45,6 +45,7 @@ namespace winrt::PowerToys::Interop::implementation static hstring ShowPeekEvent(); static hstring TerminatePeekEvent(); static hstring PowerAccentExitEvent(); + static hstring ShortcutGuideExitEvent(); static hstring ShortcutGuideTriggerEvent(); static hstring ShortcutGuideWinKeyHoldEvent(); static hstring RegistryPreviewTriggerEvent(); @@ -91,4 +92,3 @@ namespace winrt::PowerToys::Interop::factory_implementation { }; } - diff --git a/src/common/interop/Constants.idl b/src/common/interop/Constants.idl index ec990faf66..a2fe504279 100644 --- a/src/common/interop/Constants.idl +++ b/src/common/interop/Constants.idl @@ -41,6 +41,7 @@ namespace PowerToys static String ShowPeekEvent(); static String TerminatePeekEvent(); static String PowerAccentExitEvent(); + static String ShortcutGuideExitEvent(); static String ShortcutGuideTriggerEvent(); static String ShortcutGuideWinKeyHoldEvent(); static String RegistryPreviewTriggerEvent(); @@ -82,4 +83,3 @@ namespace PowerToys } } } - diff --git a/src/modules/ShortcutGuide/ShortcutGuide.Ui/Program.cs b/src/modules/ShortcutGuide/ShortcutGuide.Ui/Program.cs index 3474cf1e0d..1fc52a0ce8 100644 --- a/src/modules/ShortcutGuide/ShortcutGuide.Ui/Program.cs +++ b/src/modules/ShortcutGuide/ShortcutGuide.Ui/Program.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using System.ComponentModel; using System.Diagnostics; using System.IO; using System.Threading; @@ -19,10 +20,14 @@ namespace ShortcutGuide { public sealed class Program { + private static readonly ManualResetEvent _runnerExitEvent = new(false); + public static Thread CopyAndIndexGenerationThread { get; private set; } = null!; public static nint ForegroundWindowHandle { get; set; } = nint.Zero; + internal static WaitHandle RunnerExitEvent => _runnerExitEvent; + [STAThread] public static void Main(string[] args) { @@ -38,21 +43,17 @@ namespace ShortcutGuide return; } - if (args.Length >= 1 && int.TryParse(args[0], out int runnerPID)) - { - RunnerHelper.WaitForPowerToysRunner(runnerPID, () => - { - Logger.LogInfo($"PowerToys runner process (PID={runnerPID}) exited. Exiting ShortcutGuide."); - Environment.Exit(0); - }); - } - if (PowerToys.GPOWrapper.GPOWrapper.GetConfiguredShortcutGuideEnabledValue() == PowerToys.GPOWrapper.GpoRuleConfigured.Disabled) { Logger.LogWarning("Tried to start with a GPO policy setting the utility to always be disabled. Please contact your systems administrator."); return; } + if (args.Length >= 1 && int.TryParse(args[0], out int runnerPID)) + { + MonitorPowerToysRunner(runnerPID); + } + Directory.CreateDirectory(ManifestInterpreter.PathOfManifestFiles); // Copy every shipped manifest from the install directory to the per-user manifest folder. @@ -136,9 +137,48 @@ namespace ShortcutGuide { Logger.LogWarning("Another instance of ShortcutGuide is running. Exiting ShortcutGuide"); } + } - // The WinRT/WinUI dispatcher thread doesn't terminate cleanly; force exit. - Environment.Exit(0); + private static void MonitorPowerToysRunner(int runnerPID) + { + Process runnerProcess; + try + { + runnerProcess = Process.GetProcessById(runnerPID); + + // Force the process handle to open synchronously so a Runner exit + // during WinUI initialization cannot be missed or confused with PID reuse. + _ = runnerProcess.Handle; + } + catch (Exception ex) when (ex is ArgumentException or InvalidOperationException or Win32Exception) + { + Logger.LogWarning($"PowerToys runner process (PID={runnerPID}) is no longer available. Exiting ShortcutGuide."); + _runnerExitEvent.Set(); + return; + } + + var runnerWatcher = new Thread(() => + { + try + { + runnerProcess.WaitForExit(); + Logger.LogInfo($"PowerToys runner process (PID={runnerPID}) exited. Exiting ShortcutGuide."); + } + catch (Exception ex) when (ex is InvalidOperationException or Win32Exception) + { + Logger.LogWarning($"Failed while waiting for PowerToys runner process (PID={runnerPID}): {ex.Message}"); + } + finally + { + runnerProcess.Dispose(); + _runnerExitEvent.Set(); + } + }) + { + IsBackground = true, + Name = "ShortcutGuide-RunnerWatcher", + }; + runnerWatcher.Start(); } private static void SendSettingsTelemetry() diff --git a/src/modules/ShortcutGuide/ShortcutGuide.Ui/ShortcutGuideXAML/App.xaml.cs b/src/modules/ShortcutGuide/ShortcutGuide.Ui/ShortcutGuideXAML/App.xaml.cs index d6b06005b3..03d1e72445 100644 --- a/src/modules/ShortcutGuide/ShortcutGuide.Ui/ShortcutGuideXAML/App.xaml.cs +++ b/src/modules/ShortcutGuide/ShortcutGuide.Ui/ShortcutGuideXAML/App.xaml.cs @@ -38,16 +38,21 @@ namespace ShortcutGuide /// internal static OverlayWindow OverlayWindow { get; private set; } = null!; - private HotkeySettingsControlHook _winKeyUpKeyboardHook = null!; + private HotkeySettingsControlHook? _winKeyUpKeyboardHook; internal static string CurrentAppName { get; set; } = string.Empty; private readonly SemaphoreSlim _activationGate = new(1, 1); + private readonly ManualResetEvent _listenerShutdownEvent = new(false); private EventWaitHandle? _regularHotkeyEvent; private EventWaitHandle? _winKeyHoldEvent; + private EventWaitHandle? _exitEvent; + private RegisteredWaitHandle? _runnerExitRegistration; private Thread? _listenForActivationEventsThread; private int _activeSource = (int)ShortcutGuideActivationSource.None; private int _activeSurface = (int)ShortcutGuideOverlaySurface.Hidden; + private int _disposed; + private int _shutdownStarted; private static readonly UIntPtr _ignoreKeyEventFlag = 0x5557; @@ -68,6 +73,20 @@ namespace ShortcutGuide { try { + var dispatcher = DispatcherQueue.GetForCurrentThread(); + _runnerExitRegistration = ThreadPool.RegisterWaitForSingleObject( + Program.RunnerExitEvent, + (_, _) => + { + if (!dispatcher.TryEnqueue(Shutdown)) + { + Logger.LogWarning("Failed to enqueue Shortcut Guide shutdown after the PowerToys runner exited."); + } + }, + null, + Timeout.Infinite, + true); + this.LoadData(); OverlayWindow = new OverlayWindow(); OverlayWindow.ClosingStarted += (_, _) => ResetActivationState(); @@ -79,16 +98,12 @@ namespace ShortcutGuide OverlayWindow.SessionDurationMs, OverlayWindow.CloseType)); - // WinUI3's dispatcher loop does not terminate when the last - // window closes; without Exit() the SG.exe process stays - // alive, holds the AppInstance single-instance lock, and - // blocks the next launch (the well-known "every other - // long-press works" bug). - Current.Exit(); + Shutdown(); }; _regularHotkeyEvent = TryOpenActivationEvent(Constants.ShortcutGuideTriggerEvent()); _winKeyHoldEvent = TryOpenActivationEvent(Constants.ShortcutGuideWinKeyHoldEvent()); + _exitEvent = TryOpenActivationEvent(Constants.ShortcutGuideExitEvent()); _listenForActivationEventsThread = new Thread(ListenForActivationEvents) { @@ -133,7 +148,8 @@ namespace ShortcutGuide // Any failure in launch is fatal for this short-lived overlay; log and exit // cleanly rather than letting WinUI surface a generic crash dialog. Logger.LogError("Failed to launch Shortcut Guide.", ex); - Environment.Exit(1); + Environment.ExitCode = 1; + Shutdown(); } } @@ -195,29 +211,42 @@ namespace ShortcutGuide activationEvents.Add((_winKeyHoldEvent, ShortcutGuideActivationSource.WindowsKeyHold)); } - if (activationEvents.Count == 0) + List handles = activationEvents.ConvertAll(item => item.Handle); + int exitEventIndex = -1; + if (_exitEvent != null) { - Logger.LogError("Failed to open any Shortcut Guide activation trigger events."); + exitEventIndex = handles.Count; + handles.Add(_exitEvent); + } + + if (handles.Count == 0) + { + Logger.LogError("Failed to open any Shortcut Guide events."); return; } - WaitHandle[] handles = activationEvents.ConvertAll(item => item.Handle).ToArray(); - try + int listenerShutdownEventIndex = handles.Count; + handles.Add(_listenerShutdownEvent); + WaitHandle[] waitHandles = handles.ToArray(); + Logger.LogInfo("Shortcut Guide activation-event listener started."); + while (true) { - Logger.LogInfo("Shortcut Guide activation-event listener started."); - while (true) + int eventIndex = WaitHandle.WaitAny(waitHandles); + if (eventIndex == listenerShutdownEventIndex) { - int eventIndex = WaitHandle.WaitAny(handles); - var activationSource = activationEvents[eventIndex].Source; - Logger.LogInfo($"Shortcut Guide trigger event signaled by {activationSource}."); - OverlayWindow.DispatcherQueue.TryEnqueue(() => _ = HandleActivationAsync(activationSource)); + return; } - } - catch (ObjectDisposedException) - { - } - catch (ThreadInterruptedException) - { + + if (eventIndex == exitEventIndex) + { + Logger.LogInfo("Shortcut Guide exit event signaled."); + OverlayWindow.DispatcherQueue.TryEnqueue(Shutdown); + return; + } + + var activationSource = activationEvents[eventIndex].Source; + Logger.LogInfo($"Shortcut Guide trigger event signaled by {activationSource}."); + OverlayWindow.DispatcherQueue.TryEnqueue(() => _ = HandleActivationAsync(activationSource)); } } @@ -431,32 +460,53 @@ namespace ShortcutGuide e.SetObserved(); } - public void Dispose() + private void Shutdown() { - _regularHotkeyEvent?.Dispose(); - _winKeyHoldEvent?.Dispose(); - - if (_listenForActivationEventsThread == null) + if (Interlocked.Exchange(ref _shutdownStarted, 1) != 0) { return; } - try - { - if (!_listenForActivationEventsThread.Join(TimeSpan.FromMilliseconds(250))) - { - _listenForActivationEventsThread.Interrupt(); - _listenForActivationEventsThread.Join(TimeSpan.FromMilliseconds(250)); - } - } - catch (ThreadInterruptedException) - { - } - catch (ThreadStateException) + Dispose(); + Current.Exit(); + } + + public void Dispose() + { + if (Interlocked.Exchange(ref _disposed, 1) != 0) { + return; } - _listenForActivationEventsThread = null; + _winKeyUpKeyboardHook?.Dispose(); + _runnerExitRegistration?.Unregister(null); + + this.UnhandledException -= App_UnhandledException; + AppDomain.CurrentDomain.UnhandledException -= CurrentDomain_UnhandledException; + TaskScheduler.UnobservedTaskException -= TaskScheduler_UnobservedTaskException; + + _listenerShutdownEvent.Set(); + if (_listenForActivationEventsThread != null) + { + try + { + if (!_listenForActivationEventsThread.Join(TimeSpan.FromSeconds(1))) + { + Logger.LogWarning("Shortcut Guide activation-event listener did not stop within the timeout."); + } + } + catch (ThreadStateException ex) + { + Logger.LogWarning($"Failed to join Shortcut Guide activation-event listener: {ex.Message}"); + } + + _listenForActivationEventsThread = null; + } + + _regularHotkeyEvent?.Dispose(); + _winKeyHoldEvent?.Dispose(); + _exitEvent?.Dispose(); + _listenerShutdownEvent.Dispose(); GC.SuppressFinalize(this); } } diff --git a/src/modules/ShortcutGuide/ShortcutGuideModuleInterface/dllmain.cpp b/src/modules/ShortcutGuide/ShortcutGuideModuleInterface/dllmain.cpp index 43a39d1bea..fd27c95212 100644 --- a/src/modules/ShortcutGuide/ShortcutGuideModuleInterface/dllmain.cpp +++ b/src/modules/ShortcutGuide/ShortcutGuideModuleInterface/dllmain.cpp @@ -111,10 +111,7 @@ public: if (_enabled) { _enabled = false; - if (IsProcessActive()) - { - TerminateProcess(m_hProcess, 0); - } + StopProcess(); } else { @@ -190,7 +187,7 @@ private: //contains the non localized key of the powertoy std::wstring app_key; bool _enabled = false; - HANDLE m_hProcess = nullptr; + winrt::handle m_process; // Hotkey to invoke the module HotkeyEx m_hotkey; @@ -227,18 +224,28 @@ private: bool StartProcess(std::wstring args = L"") { - if (exitEvent) + const bool trackProcess = args.empty(); + if (trackProcess && IsProcessActive()) { - ResetEvent(exitEvent); + return true; } - if (triggerEvent) + if (trackProcess) { - ResetEvent(triggerEvent); - } - if (winKeyHoldEvent) - { - ResetEvent(winKeyHoldEvent); + if (exitEvent) + { + ResetEvent(exitEvent); + } + + if (triggerEvent) + { + ResetEvent(triggerEvent); + } + + if (winKeyHoldEvent) + { + ResetEvent(winKeyHoldEvent); + } } unsigned long powertoys_pid = GetCurrentProcessId(); @@ -267,25 +274,80 @@ private: return false; } - Logger::trace(L"Started SG process with pid={}", GetProcessId(sei.hProcess)); - m_hProcess = sei.hProcess; + winrt::handle launchedProcess{ sei.hProcess }; + Logger::trace(L"Started SG process with pid={}", GetProcessId(launchedProcess.get())); + if (trackProcess) + { + m_process = std::move(launchedProcess); + } + return true; } bool IsProcessActive() { - if (!m_hProcess) + if (!m_process) { return false; } - auto result = WaitForSingleObject(m_hProcess, 0); + + auto result = WaitForSingleObject(m_process.get(), 0); if (result == WAIT_FAILED) { Logger::error("Failed to wait for SG process."); } + + if (result == WAIT_OBJECT_0) + { + m_process = {}; + } + return result == WAIT_TIMEOUT; } + void StopProcess() + { + if (exitEvent) + { + if (!SetEvent(exitEvent)) + { + Logger::error(L"Failed to signal {}. {}", CommonSharedConstants::SHORTCUT_GUIDE_EXIT_EVENT, get_last_error_or_default(GetLastError())); + } + } + + if (!m_process) + { + return; + } + + if (!IsProcessActive()) + { + return; + } + + constexpr DWORD gracefulShutdownTimeoutMs = 2000; + constexpr DWORD forcedShutdownTimeoutMs = 5000; + auto waitResult = WaitForSingleObject(m_process.get(), gracefulShutdownTimeoutMs); + if (waitResult == WAIT_TIMEOUT) + { + Logger::warn("Shortcut Guide did not exit gracefully; terminating it."); + if (!TerminateProcess(m_process.get(), 0)) + { + Logger::error(L"Failed to terminate Shortcut Guide. {}", get_last_error_or_default(GetLastError())); + } + else if (WaitForSingleObject(m_process.get(), forcedShutdownTimeoutMs) != WAIT_OBJECT_0) + { + Logger::error("Shortcut Guide did not terminate within the timeout."); + } + } + else if (waitResult == WAIT_FAILED) + { + Logger::error(L"Failed to wait for Shortcut Guide shutdown. {}", get_last_error_or_default(GetLastError())); + } + + m_process = {}; + } + void InitSettings() { try @@ -409,10 +471,7 @@ private: void WindowsKeyPressBehavior() { - if (IsProcessActive()) - { - TerminateProcess(m_hProcess, 0); - } + StopProcess(); } };