From db7a4cfdee6c462a44c27ea35272cd2bfaed420b Mon Sep 17 00:00:00 2001 From: Chek Wei Tan Date: Tue, 1 Jul 2025 07:17:38 +0800 Subject: [PATCH] Fix: Prevent memory leak in view model lifecycle (#40216) ## Summary of the Pull Request A memory leak was identified where activating a Command Palette extension via a hotkey would lead to a linear increase in resource consumption and a corresponding decrease in search performance. Each activation created a new `PageViewModel` instance for the extension's UI. However, the `ShellViewModel` did not dispose of the previous `PageViewModel` instance when a new one was created. The fix is implemented in `Microsoft.CmdPal.UI.ViewModels/ShellViewModel.cs` by ensuring that `PageViewModel` instances are correctly disposed of when they are no longer active. ## PR Checklist - [x] **Closes:** #40199 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx ## Validation Steps Performed 1. Installed [Workspace Launcher for Visual Studio / Code](https://github.com/tanchekwei/WorkspaceLauncherForVSCode) 2. Configured a global hotkey for the extension (e.g., Alt + Q). 3. Ran the following AHK script. Pressing F1 simulates pressing the hotkey 10 times with a 200ms interval, followed by typing p to trigger a search and observe how many times `GetItems` is invoked. ```ahk #NoEnv SetBatchLines, -1 ; Press F1 to run the sequence F1:: Loop, 10 { Send, !q Sleep, 200 } Send, p return ``` ### Before the Fix (PowerToys v0.91.1) - Pressing the hotkey 10 times then search will triggered `GetItems` 12 times. - See video and log output below: https://github.com/user-attachments/assets/3c7d59d6-2dda-4ab4-b230-2f2472c45776 ```log [2025-06-25 00:04:58.251] [VisualStudioCodePage.UpdateSearchText] Started [2025-06-25 00:04:58.251] [VisualStudioCodePage.UpdateSearchText] SearchText: p [2025-06-25 00:04:58.251] [WorkspaceFilter.Filter] Started [2025-06-25 00:04:58.263] [WorkspaceFilter.Filter] Finished in 11ms [2025-06-25 00:04:58.269] [VisualStudioCodePage.GetItems] Started [2025-06-25 00:04:58.269] [VisualStudioCodePage.GetItems] Finished in 0ms [2025-06-25 00:04:58.373] [VisualStudioCodePage.GetItems] Started [2025-06-25 00:04:58.373] [VisualStudioCodePage.GetItems] Finished in 0ms [2025-06-25 00:04:58.408] [VisualStudioCodePage.GetItems] Started [2025-06-25 00:04:58.408] [VisualStudioCodePage.GetItems] Finished in 0ms [2025-06-25 00:04:58.445] [VisualStudioCodePage.GetItems] Started [2025-06-25 00:04:58.446] [VisualStudioCodePage.GetItems] Finished in 0ms [2025-06-25 00:04:58.485] [VisualStudioCodePage.GetItems] Started [2025-06-25 00:04:58.486] [VisualStudioCodePage.GetItems] Finished in 0ms [2025-06-25 00:04:58.524] [VisualStudioCodePage.GetItems] Started [2025-06-25 00:04:58.525] [VisualStudioCodePage.GetItems] Finished in 0ms [2025-06-25 00:04:58.563] [VisualStudioCodePage.GetItems] Started [2025-06-25 00:04:58.564] [VisualStudioCodePage.GetItems] Finished in 0ms [2025-06-25 00:04:58.604] [VisualStudioCodePage.GetItems] Started [2025-06-25 00:04:58.604] [VisualStudioCodePage.GetItems] Finished in 0ms [2025-06-25 00:04:58.645] [VisualStudioCodePage.GetItems] Started [2025-06-25 00:04:58.645] [VisualStudioCodePage.GetItems] Finished in 0ms [2025-06-25 00:04:58.685] [VisualStudioCodePage.GetItems] Started [2025-06-25 00:04:58.686] [VisualStudioCodePage.GetItems] Finished in 0ms [2025-06-25 00:04:58.755] [VisualStudioCodePage.GetItems] Started [2025-06-25 00:04:58.756] [VisualStudioCodePage.GetItems] Finished in 0ms [2025-06-25 00:04:58.798] [VisualStudioCodePage.GetItems] Started [2025-06-25 00:04:58.798] [VisualStudioCodePage.GetItems] Finished in 0ms [2025-06-25 00:04:58.843] [VisualStudioCodePage.UpdateSearchText] Finished in 591ms ``` ### After the Fix - Pressing the hotkey 10 times then search will now triggers `GetItems` only once. - See updated video and log output: https://github.com/user-attachments/assets/e38f3eb5-5353-44da-a9d9-ff83647eb6e9 ```log [2025-06-25 00:03:44.862] [VisualStudioCodePage.UpdateSearchText] Started [2025-06-25 00:03:44.863] [VisualStudioCodePage.UpdateSearchText] SearchText: p [2025-06-25 00:03:44.864] [WorkspaceFilter.Filter] Started [2025-06-25 00:03:44.866] [WorkspaceFilter.Filter] Finished in 2ms [2025-06-25 00:03:44.870] [VisualStudioCodePage.GetItems] Started [2025-06-25 00:03:44.870] [VisualStudioCodePage.GetItems] Finished in 0ms [2025-06-25 00:03:44.909] [VisualStudioCodePage.UpdateSearchText] Finished in 46ms ``` --- .../ShellViewModel.cs | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ShellViewModel.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ShellViewModel.cs index d9212733eb..b8d140e548 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ShellViewModel.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ShellViewModel.cs @@ -29,8 +29,30 @@ public partial class ShellViewModel(IServiceProvider _serviceProvider, TaskSched [ObservableProperty] public partial bool IsDetailsVisible { get; set; } - [ObservableProperty] - public partial PageViewModel CurrentPage { get; set; } = new LoadingPageViewModel(null, _scheduler); + private PageViewModel _currentPage = new LoadingPageViewModel(null, _scheduler); + + public PageViewModel CurrentPage + { + get => _currentPage; + set + { + var oldValue = _currentPage; + if (SetProperty(ref _currentPage, value)) + { + if (oldValue is IDisposable disposable) + { + try + { + disposable.Dispose(); + } + catch (Exception ex) + { + Logger.LogError(ex.ToString()); + } + } + } + } + } private MainListPage? _mainListPage;