CmdPal: Fix Dock refresh resource leak (#49742)

## Summary of the Pull Request

This PR improves Dock band refresh and partially eliminates our favorite
leak:

- Reuses Dock item view models while their source items remain stable.
- Coalesces bursty ItemsChanged notifications into a single follow-up
refresh.
- Cleans replaced and discarded view models after applying UI updates.
- Prevents queued refreshes from repopulating bands after cleanup.
- Handles unavailable UI schedulers without abandoning created view
models.
- Adds unit tests for reuse and cleanup.


## Pictures? Pictures!

Before

<img width="1671" height="400" alt="image"
src="https://github.com/user-attachments/assets/3a0874e6-eded-44f0-8bc2-bfa223e2888d"
/>


After

<img width="1671" height="716" alt="image"
src="https://github.com/user-attachments/assets/1560bfdb-5701-40bd-9f20-d4e885159ab8"
/>


<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist

- [x] Closes: #49428
<!-- - [ ] Closes: #yyy (add separate lines for additional resolved
issues) -->
- [ ] **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

<!-- Provide a more detailed description of the PR, other things fixed,
or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments

<!-- Describe how you validated the behavior. Add automated tests
wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Dock/DockBandViewModel.cs

@@ -5,6 +5,7 @@
 using System.Collections.Immutable;
 using System.Collections.ObjectModel;
 using System.Threading;
+using Microsoft.CmdPal.Common;
 using Microsoft.CmdPal.Common.Helpers;
 using Microsoft.CmdPal.UI.ViewModels.Models;
 using Microsoft.CmdPal.UI.ViewModels.Services;
@@ -24,6 +25,9 @@ public sealed partial class DockBandViewModel : ExtensionObjectViewModel
     private readonly Lock _subscriptionLock = new();
 
     private DockBandSettings _bandSettings;
+    private Dictionary<IListItem, DockItemViewModel> _viewModelCache = new(ReferenceEqualityComparer.Instance);
+    private InterlockedBoolean _refreshInFlight;
+    private InterlockedBoolean _refreshRequested;
     private InterlockedBoolean _cleanupStarted;
     private IListPage? _subscribedList;
 
@@ -235,24 +239,165 @@ public sealed partial class DockBandViewModel : ExtensionObjectViewModel
 
     private void InitializeFromList(IListPage list)
     {
-        var items = list.GetItems();
-        var newViewModels = new List<DockItemViewModel>();
-        foreach (var item in items)
+        if (_cleanupStarted.Value)
         {
-            var newItemVm = new DockItemViewModel(new(item), this.PageContext, _showTitles, _showSubtitles, _contextMenuFactory);
-            newItemVm.SlowInitializeProperties();
-            newViewModels.Add(newItemVm);
+            return;
         }
 
-        List<DockItemViewModel> removed = new();
-        DoOnUiThread(() =>
+        _refreshRequested.Set();
+        if (!_refreshInFlight.Set())
         {
-            ListHelpers.InPlaceUpdateList(Items, newViewModels, out removed);
-        });
+            return;
+        }
 
-        foreach (var removedItem in removed)
+        BuildRefresh(list);
+    }
+
+    private void BuildRefresh(IListPage list)
+    {
+        List<DockItemViewModel> createdViewModels = [];
+        try
         {
-            removedItem.SafeCleanup();
+            _refreshRequested.Clear();
+            if (_cleanupStarted.Value)
+            {
+                CompleteRefresh(list);
+                return;
+            }
+
+            var items = list.GetItems();
+            var currentCache = Volatile.Read(ref _viewModelCache);
+            var nextCache = new Dictionary<IListItem, DockItemViewModel>(items.Length, ReferenceEqualityComparer.Instance);
+            var newViewModels = new List<DockItemViewModel>(items.Length);
+
+            foreach (var item in items)
+            {
+                if (item is null)
+                {
+                    continue;
+                }
+
+                if (!nextCache.TryGetValue(item, out var itemViewModel) &&
+                    !currentCache.TryGetValue(item, out itemViewModel))
+                {
+                    itemViewModel = new DockItemViewModel(new(item), PageContext, _showTitles, _showSubtitles, _contextMenuFactory);
+                    createdViewModels.Add(itemViewModel);
+                    itemViewModel.SlowInitializeProperties();
+                }
+
+                nextCache[item] = itemViewModel;
+                newViewModels.Add(itemViewModel);
+            }
+
+            if (!TryDoOnUiThread(() => ApplyRefresh(list, nextCache, newViewModels, createdViewModels)))
+            {
+                QueueCleanup(createdViewModels);
+                CompleteRefresh(list);
+            }
+        }
+        catch
+        {
+            // Clear the gate directly rather than through CompleteRefresh: any pending _refreshRequested is dropped
+            // on purpose, because re-arming against a GetItems() that keeps throwing would spin a tight retry loop.
+            // The next ItemsChanged starts a fresh refresh.
+            QueueCleanup(createdViewModels);
+            _refreshInFlight.Clear();
+            throw;
+        }
+    }
+
+    private void ApplyRefresh(
+        IListPage list,
+        Dictionary<IListItem, DockItemViewModel> nextCache,
+        IReadOnlyList<DockItemViewModel> newViewModels,
+        IReadOnlyList<DockItemViewModel> createdViewModels)
+    {
+        List<DockItemViewModel> removedItems = [];
+        try
+        {
+            if (_cleanupStarted.Value)
+            {
+                return;
+            }
+
+            foreach (var viewModel in newViewModels)
+            {
+                viewModel.ShowTitle = _showTitles;
+                viewModel.ShowSubtitle = _showSubtitles;
+            }
+
+            ListHelpers.InPlaceUpdateList(Items, newViewModels, out removedItems);
+            Volatile.Write(ref _viewModelCache, nextCache);
+        }
+        finally
+        {
+            CleanupDiscardedViewModels(removedItems, createdViewModels);
+            CompleteRefresh(list);
+        }
+    }
+
+    private void CompleteRefresh(IListPage list)
+    {
+        _refreshInFlight.Clear();
+        if (_cleanupStarted.Value ||
+            !_refreshRequested.Value)
+        {
+            return;
+        }
+
+        _ = Task.Run(
+            () =>
+            {
+                try
+                {
+                    InitializeFromList(list);
+                }
+                catch (Exception ex)
+                {
+                    CoreLogger.LogError("Failed to refresh a Dock band.", ex);
+                }
+            });
+    }
+
+    private void CleanupDiscardedViewModels(
+        IEnumerable<DockItemViewModel> removedItems,
+        IEnumerable<DockItemViewModel> createdViewModels)
+    {
+        var retained = new HashSet<DockItemViewModel>(Items, ReferenceEqualityComparer.Instance);
+        var discarded = new HashSet<DockItemViewModel>(ReferenceEqualityComparer.Instance);
+
+        foreach (var item in removedItems)
+        {
+            if (!retained.Contains(item))
+            {
+                discarded.Add(item);
+            }
+        }
+
+        foreach (var item in createdViewModels)
+        {
+            if (!retained.Contains(item))
+            {
+                discarded.Add(item);
+            }
+        }
+
+        QueueCleanup(discarded);
+    }
+
+    private static void QueueCleanup(IEnumerable<DockItemViewModel> viewModels)
+    {
+        var pendingCleanup = viewModels.ToArray();
+        if (pendingCleanup.Length != 0)
+        {
+            _ = Task.Run(
+                () =>
+                {
+                    foreach (var viewModel in pendingCleanup)
+                    {
+                        viewModel.SafeCleanup();
+                    }
+                });
         }
     }
 
@@ -267,7 +412,6 @@ public sealed partial class DockBandViewModel : ExtensionObjectViewModel
         var list = command.Model.Unsafe as IListPage;
         if (list is not null)
         {
-            InitializeFromList(list);
             lock (_subscriptionLock)
             {
                 if (_cleanupStarted.Value || _subscribedList is not null)
@@ -278,15 +422,32 @@ public sealed partial class DockBandViewModel : ExtensionObjectViewModel
                 list.ItemsChanged += HandleItemsChanged;
                 _subscribedList = list;
             }
+
+            if (_cleanupStarted.Value)
+            {
+                return;
+            }
+
+            InitializeFromList(list);
         }
         else
         {
             var dockItem = new DockItemViewModel(_rootItem, _showTitles, _showSubtitles, _contextMenuFactory);
             dockItem.SlowInitializeProperties();
-            DoOnUiThread(() =>
+            if (!TryDoOnUiThread(() =>
             {
-                Items.Add(dockItem);
-            });
+                if (!_cleanupStarted.Value)
+                {
+                    Items.Add(dockItem);
+                }
+                else
+                {
+                    QueueCleanup([dockItem]);
+                }
+            }))
+            {
+                QueueCleanup([dockItem]);
+            }
         }
     }
 
@@ -324,11 +485,22 @@ public sealed partial class DockBandViewModel : ExtensionObjectViewModel
             subscribedList.ItemsChanged -= HandleItemsChanged;
         }
 
-        foreach (var item in Items)
+        Volatile.Write(
+            ref _viewModelCache,
+            new Dictionary<IListItem, DockItemViewModel>(ReferenceEqualityComparer.Instance));
+
+        if (!TryDoOnUiThread(DrainItems))
         {
-            item.SafeCleanup();
+            QueueCleanup(Items.ToArray());
         }
     }
+
+    private void DrainItems()
+    {
+        var items = Items.ToArray();
+        Items.Clear();
+        QueueCleanup(items);
+    }
 }
 
 public partial class DockItemViewModel : CommandItemViewModel
@@ -418,6 +590,10 @@ public partial class DockItemViewModel : CommandItemViewModel
             }
         };
     }
+
+    public override bool Equals(object? obj) => obj is DockItemViewModel viewModel && viewModel.Model.Equals(Model);
+
+    public override int GetHashCode() => Model.GetHashCode();
 }
 
 

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

@@ -255,6 +255,9 @@ public abstract partial class ExtensionObjectViewModel : ObservableObject, IBatc
     private static PropertyChangedEventArgs Args(string name) => new(name);
 
     protected void DoOnUiThread(Action action)
+        => _ = TryDoOnUiThread(action);
+
+    protected bool TryDoOnUiThread(Action action)
     {
         if (PageContext.TryGetTarget(out var pageContext))
         {
@@ -263,7 +266,10 @@ public abstract partial class ExtensionObjectViewModel : ObservableObject, IBatc
                 CancellationToken.None,
                 TaskCreationOptions.None,
                 pageContext.Scheduler);
+            return true;
         }
+
+        return false;
     }
 
     protected virtual void UnsafeCleanup()

src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/DockItemViewModelTests.cs

@@ -0,0 +1,314 @@
+// Copyright (c) Microsoft Corporation
+// The Microsoft Corporation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Collections.Concurrent;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Reflection;
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.CmdPal.UI.ViewModels.Dock;
+using Microsoft.CmdPal.UI.ViewModels.Models;
+using Microsoft.CmdPal.UI.ViewModels.Services;
+using Microsoft.CmdPal.UI.ViewModels.Settings;
+using Microsoft.CommandPalette.Extensions;
+using Microsoft.CommandPalette.Extensions.Toolkit;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Moq;
+
+namespace Microsoft.CmdPal.UI.ViewModels.UnitTests;
+
+[TestClass]
+public partial class DockItemViewModelTests
+{
+    private sealed class TestPageContext(TaskScheduler scheduler) : IPageContext
+    {
+        public TaskScheduler Scheduler { get; } = scheduler;
+
+        public ICommandProviderContext ProviderContext => CommandProviderContext.Empty;
+
+        public void ShowException(Exception ex, string? extensionHint = null)
+        {
+            throw new AssertFailedException($"Unexpected exception from view model: {ex}");
+        }
+    }
+
+    private sealed class QueuedTaskScheduler : TaskScheduler
+    {
+        private readonly ConcurrentQueue<Task> _tasks = [];
+
+        protected override IEnumerable<Task> GetScheduledTasks() => _tasks.ToArray();
+
+        protected override void QueueTask(Task task) => _tasks.Enqueue(task);
+
+        protected override bool TryExecuteTaskInline(Task task, bool taskWasPreviouslyQueued) => false;
+
+        public void ExecuteAllAvailable()
+        {
+            while (_tasks.TryDequeue(out var task))
+            {
+                TryExecuteTask(task);
+            }
+        }
+
+        public void ExecuteUntil(Func<bool> condition)
+        {
+            var timeout = Stopwatch.StartNew();
+            while (!condition())
+            {
+                ExecuteAllAvailable();
+                if (timeout.Elapsed > TimeSpan.FromSeconds(2))
+                {
+                    Assert.Fail("Timed out waiting for scheduled Dock work.");
+                }
+
+                Thread.Sleep(5);
+            }
+        }
+    }
+
+    private sealed partial class TestDockPage : ListPage
+    {
+        private IListItem[] _items;
+        private int _getItemsCallCount;
+        private Action? _getItemsCallback;
+
+        public TestDockPage(IListItem item)
+        {
+            _items = [item];
+        }
+
+        public int GetItemsCallCount => Volatile.Read(ref _getItemsCallCount);
+
+        public override IListItem[] GetItems()
+        {
+            Interlocked.Increment(ref _getItemsCallCount);
+            Interlocked.Exchange(ref _getItemsCallback, null)?.Invoke();
+            return Volatile.Read(ref _items);
+        }
+
+        public int ItemsChangedSubscriberCount =>
+            ((Delegate?)typeof(ListPage)
+                .GetField(nameof(ItemsChanged), BindingFlags.Instance | BindingFlags.NonPublic)!
+                .GetValue(this))?
+                .GetInvocationList()
+                .Length ?? 0;
+
+        public void SetItem(IListItem item) => SetItems(item);
+
+        public void SetItems(params IListItem[] items) => Volatile.Write(ref _items, items);
+
+        public void OnNextGetItems(Action callback) => Volatile.Write(ref _getItemsCallback, callback);
+
+        public void TriggerItemsChanged() => RaiseItemsChanged(_items.Length);
+    }
+
+    private sealed record BandFixture(
+        DockBandViewModel Band,
+        CommandItemViewModel Root,
+        TestDockPage Page,
+        TestPageContext Context,
+        QueuedTaskScheduler Scheduler);
+
+    [TestMethod]
+    public void ItemsChanged_CleansReplacedViewModelAfterUiUpdate()
+    {
+        var fixture = CreateBandFixture();
+        try
+        {
+            var original = fixture.Band.Items[0];
+            fixture.Page.SetItem(CreateItem("Replacement"));
+
+            fixture.Page.TriggerItemsChanged();
+            fixture.Scheduler.ExecuteUntil(() => !ReferenceEquals(original, fixture.Band.Items[0]));
+
+            Assert.IsTrue(
+                SpinWait.SpinUntil(
+                    () => original.Initialized.HasFlag(InitializedState.CleanedUp),
+                    TimeSpan.FromSeconds(2)),
+                "The replaced Dock item was not cleaned.");
+        }
+        finally
+        {
+            CleanupFixture(fixture);
+        }
+    }
+
+    [TestMethod]
+    public void RepeatedItemsChanged_ReusesStableViewModel()
+    {
+        var fixture = CreateBandFixture();
+        try
+        {
+            var original = fixture.Band.Items[0];
+
+            for (var i = 0; i < 100; i++)
+            {
+                fixture.Page.TriggerItemsChanged();
+                fixture.Scheduler.ExecuteAllAvailable();
+            }
+
+            Assert.AreSame(original, fixture.Band.Items[0]);
+            Assert.IsFalse(original.Initialized.HasFlag(InitializedState.CleanedUp));
+        }
+        finally
+        {
+            CleanupFixture(fixture);
+        }
+    }
+
+    [TestMethod]
+    public void DuplicateItems_ReuseSingleViewModel()
+    {
+        var fixture = CreateBandFixture();
+        try
+        {
+            var duplicate = CreateItem("Duplicate");
+            fixture.Page.SetItems(duplicate, duplicate);
+
+            fixture.Page.TriggerItemsChanged();
+            fixture.Scheduler.ExecuteUntil(() => fixture.Band.Items.Count == 2);
+
+            Assert.AreSame(fixture.Band.Items[0], fixture.Band.Items[1]);
+
+            var sharedViewModel = fixture.Band.Items[0];
+            fixture.Page.SetItem(duplicate);
+            fixture.Page.TriggerItemsChanged();
+            fixture.Scheduler.ExecuteUntil(() => fixture.Band.Items.Count == 1);
+
+            Assert.AreSame(sharedViewModel, fixture.Band.Items[0]);
+            Assert.IsFalse(sharedViewModel.Initialized.HasFlag(InitializedState.CleanedUp));
+        }
+        finally
+        {
+            CleanupFixture(fixture);
+        }
+    }
+
+    [TestMethod]
+    public void BurstyItemsChanged_CoalescesRefreshWork()
+    {
+        var fixture = CreateBandFixture();
+        try
+        {
+            var initialCalls = fixture.Page.GetItemsCallCount;
+
+            for (var i = 0; i < 1000; i++)
+            {
+                fixture.Page.TriggerItemsChanged();
+            }
+
+            Assert.AreEqual(initialCalls + 1, fixture.Page.GetItemsCallCount);
+
+            fixture.Scheduler.ExecuteAllAvailable();
+            Assert.IsTrue(
+                SpinWait.SpinUntil(
+                    () => fixture.Page.GetItemsCallCount == initialCalls + 2,
+                    TimeSpan.FromSeconds(2)),
+                "The coalesced follow-up refresh did not run.");
+            fixture.Scheduler.ExecuteAllAvailable();
+
+            Assert.AreEqual(initialCalls + 2, fixture.Page.GetItemsCallCount);
+            Assert.AreEqual(1, fixture.Band.Items.Count);
+        }
+        finally
+        {
+            CleanupFixture(fixture);
+        }
+    }
+
+    [TestMethod]
+    public void CleanupWithQueuedRefresh_DoesNotRepopulateItems()
+    {
+        var fixture = CreateBandFixture();
+        try
+        {
+            var original = fixture.Band.Items[0];
+            fixture.Page.SetItem(CreateItem("Replacement"));
+            fixture.Page.TriggerItemsChanged();
+
+            fixture.Band.SafeCleanup();
+            fixture.Scheduler.ExecuteAllAvailable();
+
+            Assert.AreEqual(0, fixture.Band.Items.Count);
+            Assert.IsTrue(
+                SpinWait.SpinUntil(
+                    () => original.Initialized.HasFlag(InitializedState.CleanedUp),
+                    TimeSpan.FromSeconds(2)),
+                "The disposed Dock item was not cleaned.");
+        }
+        finally
+        {
+            CleanupFixture(fixture);
+        }
+    }
+
+    [TestMethod]
+    public void CleanupDuringInitialization_DoesNotLeaveItemsChangedSubscription()
+    {
+        var fixture = CreateBandFixture(cleanupDuringInitialization: true);
+        try
+        {
+            Assert.AreEqual(0, fixture.Page.ItemsChangedSubscriberCount);
+        }
+        finally
+        {
+            CleanupFixture(fixture);
+        }
+    }
+
+    private static BandFixture CreateBandFixture(bool cleanupDuringInitialization = false)
+    {
+        var scheduler = new QueuedTaskScheduler();
+        var context = new TestPageContext(scheduler);
+        var page = new TestDockPage(CreateItem("Initial"))
+        {
+            Id = "test.dock.page",
+            Name = "Test Dock Page",
+            Title = "Test Dock Page",
+        };
+        var root = new CommandItemViewModel(
+            new(new CommandItem(page) { Title = page.Title }),
+            new(context),
+            DefaultContextMenuFactory.Instance);
+        root.SlowInitializeProperties();
+
+        var settingsService = new Mock<ISettingsService>();
+        settingsService.SetupGet(service => service.Settings).Returns(new SettingsModel());
+
+        var band = new DockBandViewModel(
+            root,
+            new(context),
+            new DockBandSettings { ProviderId = "test", CommandId = page.Id },
+            settingsService.Object,
+            DefaultContextMenuFactory.Instance);
+        if (cleanupDuringInitialization)
+        {
+            page.OnNextGetItems(band.SafeCleanup);
+        }
+
+        band.InitializeProperties();
+        if (cleanupDuringInitialization)
+        {
+            scheduler.ExecuteAllAvailable();
+        }
+        else
+        {
+            scheduler.ExecuteUntil(() => band.Items.Count == 1);
+        }
+
+        return new(band, root, page, context, scheduler);
+    }
+
+    private static ListItem CreateItem(string title) =>
+        new(new NoOpCommand { Name = title }) { Title = title };
+
+    private static void CleanupFixture(BandFixture fixture)
+    {
+        fixture.Band.SafeCleanup();
+        fixture.Root.SafeCleanup();
+        fixture.Scheduler.ExecuteAllAvailable();
+    }
+}