refine: add clarifying comments per code review feedback

Agent-Logs-Url: https://github.com/microsoft/PowerToys/sessions/0bf9ec35-b19d-4ea4-971d-803a1d4f305e

Co-authored-by: MuyuanMS <116717757+MuyuanMS@users.noreply.github.com>

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandItemViewModel.cs

@@ -160,6 +160,17 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa
             return;
         }
 
+        // Guard against a race with UnsafeCleanup on the UI thread.
+        // _initializeItemsTask runs on a background thread and may call
+        // InitializeProperties() after SafeCleanup() has already removed the
+        // event subscriptions.  If we allowed the subscription to happen here
+        // there would be no cleanup path for it, leaking the view model via
+        // the cross-process COM event registration.
+        if (IsCleanedUp)
+        {
+            return;
+        }
+
         if (!IsFastInitialized)
         {
             FastInitializeProperties();
@@ -184,6 +195,15 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa
         model.PropChanged += Model_PropChanged;
         Command.PropertyChanged += Command_PropertyChanged;
 
+        // Second guard: handle the narrow race window where SafeCleanup ran
+        // between the check above and the subscriptions just made.
+        if (IsCleanedUp)
+        {
+            model.PropChanged -= Model_PropChanged;
+            Command.PropertyChanged -= Command_PropertyChanged;
+            return;
+        }
+
         UpdateProperty(nameof(Name));
         UpdateProperty(nameof(Title));
         UpdateProperty(nameof(Subtitle));

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ExtensionObjectViewModel.cs

@@ -271,8 +271,22 @@ public abstract partial class ExtensionObjectViewModel : ObservableObject, IBatc
         // base doesn't do anything, but sub-classes should override this.
     }
 
+    private volatile bool _isCleanedUp;
+
+    /// <summary>
+    /// Gets a value indicating whether <see cref="SafeCleanup"/> has been called on this object.
+    /// Safe to read from any thread.  Once set, it is never cleared.
+    /// </summary>
+    public bool IsCleanedUp => _isCleanedUp;
+
     public virtual void SafeCleanup()
     {
+        // Set the flag *before* UnsafeCleanup so that any concurrent
+        // InitializeProperties calls on background threads can see it and
+        // bail out before subscribing to COM events that would never be
+        // unsubscribed.
+        _isCleanedUp = true;
+
         try
         {
             UnsafeCleanup();

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ListViewModel.cs

@@ -1113,6 +1113,15 @@ public partial class ListViewModel : PageViewModel, IDisposable
         CancelAndDisposeTokenSource(ref _fetchItemsCancellationTokenSource);
         CancelAndDisposeTokenSource(ref _selectedItemCts);
 
+        // Unsubscribe the selected-item PropertyChanged handler to prevent
+        // _lastSelectedItem from keeping this ListViewModel alive via the
+        // SelectedItemPropertyChanged delegate.
+        if (_lastSelectedItem is not null)
+        {
+            _lastSelectedItem.PropertyChanged -= SelectedItemPropertyChanged;
+            _lastSelectedItem = null;
+        }
+
         lock (_listLock)
         {
             foreach (var item in Items)

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/PageViewModel.cs

@@ -158,6 +158,14 @@ public partial class PageViewModel : ExtensionObjectViewModel, IPageContext
 
     public override void InitializeProperties()
     {
+        // Guard against a race with UnsafeCleanup on the UI thread.
+        // If this ViewModel has already been cleaned up, do not subscribe to
+        // any COM events as there would be no path to unsubscribe them.
+        if (IsCleanedUp)
+        {
+            return;
+        }
+
         var page = _pageModel.Unsafe;
         if (page is null)
         {
@@ -182,6 +190,13 @@ public partial class PageViewModel : ExtensionObjectViewModel, IPageContext
         UpdateProperty(nameof(HasSearchBox));
 
         page.PropChanged += Model_PropChanged;
+
+        // Second guard: handle the narrow race window where SafeCleanup ran
+        // between the check above and the subscription just made.
+        if (IsCleanedUp)
+        {
+            page.PropChanged -= Model_PropChanged;
+        }
     }
 
     private void Model_PropChanged(object sender, IPropChangedEventArgs args)

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ShellViewModel.cs

@@ -177,6 +177,13 @@ public partial class ShellViewModel : ObservableObject,
                             {
                                 if (cancellationToken.IsCancellationRequested)
                                 {
+                                    // SafeCleanup removes COM event subscriptions that were added during
+                                    // initialization (items PropChanged, page PropChanged, etc.).
+                                    // Without this, items remain alive via cross-process COM event
+                                    // registrations that are never unregistered when navigation is cancelled
+                                    // by a rapid second hotkey press.
+                                    viewModel.SafeCleanup();
+
                                     if (viewModel is IDisposable disposable)
                                     {
                                         try
@@ -194,7 +201,12 @@ public partial class ShellViewModel : ObservableObject,
 
                                 CurrentPage = viewModel;
                             },
-                            cancellationToken,
+                            // Use CancellationToken.None (not cancellationToken) so that this
+                            // UI-thread task is always scheduled and always runs its body.
+                            // If we passed the already-cancelled token the task would be placed
+                            // in the Cancelled state immediately and the cleanup lambda above
+                            // would never execute, leaking the initialized ViewModel.
+                            CancellationToken.None,
                             TaskCreationOptions.None,
                             _scheduler);
                         await t;
@@ -207,6 +219,12 @@ public partial class ShellViewModel : ObservableObject,
         {
             if (cancellationToken.IsCancellationRequested)
             {
+                // SafeCleanup removes COM event subscriptions that were added during
+                // initialization (items PropChanged, page PropChanged, etc.).
+                // Without this, items remain alive via cross-process COM event
+                // registrations that are never unregistered when navigation is cancelled.
+                viewModel.SafeCleanup();
+
                 if (viewModel is IDisposable disposable)
                 {
                     try