CmdPal: Harden ListViewModel fetch synchronization (#46429)

## Summary of the Pull Request

This PR improves fetching of list items in ListViewModel:
- Fixes _vmCache concurrency with copy-on-write cache publication.
- Preserves latest-fetch-wins behavior across overlapping RPC GetItems()
calls.
- Prevents stale or canceled fetches from publishing and makes them
abort promptly.
- Improves cancellation cleanup for abandoned item view models and
replaced token sources.
- Updates empty-state tracking to follow overlapping fetch activity
correctly.
- Reduces hot-path cache overhead by removing per-item cache locking and
full cache rebuilds.
- Adds guard against re-entry, to prevent situations like #46329:
- Defers ItemsChanged-triggered fetches raised during GetItems() until
the call unwinds;
- Uses a thread-local reentry guard so unrelated cross-thread fetches
are not delayed;
- Adds a regression test covering recursive GetItems() refresh behavior.
- Make sure we never invoke FetchItems on UI thread, and be loud in
debug when we are.

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

- [x] Closes: #46331
<!-- - [ ] 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
This commit is contained in:
Jiří Polášek
2026-03-31 04:27:08 +02:00
committed by GitHub
parent 7d171a4428
commit c34fb7f953
2 changed files with 416 additions and 114 deletions

View File

@@ -4,6 +4,7 @@
using System.Collections.ObjectModel; using System.Collections.ObjectModel;
using System.Runtime.CompilerServices; using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using CommunityToolkit.Mvvm.Input; using CommunityToolkit.Mvvm.Input;
using CommunityToolkit.Mvvm.Messaging; using CommunityToolkit.Mvvm.Messaging;
using Microsoft.CmdPal.Common; using Microsoft.CmdPal.Common;
@@ -12,6 +13,7 @@ using Microsoft.CmdPal.UI.ViewModels.Messages;
using Microsoft.CmdPal.UI.ViewModels.Models; using Microsoft.CmdPal.UI.ViewModels.Models;
using Microsoft.CommandPalette.Extensions; using Microsoft.CommandPalette.Extensions;
using Microsoft.CommandPalette.Extensions.Toolkit; using Microsoft.CommandPalette.Extensions.Toolkit;
using Microsoft.UI.Dispatching;
using Windows.Foundation; using Windows.Foundation;
namespace Microsoft.CmdPal.UI.ViewModels; namespace Microsoft.CmdPal.UI.ViewModels;
@@ -20,9 +22,11 @@ public partial class ListViewModel : PageViewModel, IDisposable
{ {
public const int IncrementalRefresh = -2; public const int IncrementalRefresh = -2;
private static readonly IEqualityComparer<IListItem> VmCacheComparer = new ProxyReferenceEqualityComparer();
private readonly TaskFactory filterTaskFactory = new(new ConcurrentExclusiveSchedulerPair().ExclusiveScheduler); private readonly TaskFactory filterTaskFactory = new(new ConcurrentExclusiveSchedulerPair().ExclusiveScheduler);
private readonly Dictionary<IListItem, ListItemViewModel> _vmCache = new(new ProxyReferenceEqualityComparer()); private Dictionary<IListItem, ListItemViewModel> _vmCache = new(VmCacheComparer);
// TODO: Do we want a base "ItemsPageViewModel" for anything that's going to have items? // TODO: Do we want a base "ItemsPageViewModel" for anything that's going to have items?
@@ -36,24 +40,33 @@ public partial class ListViewModel : PageViewModel, IDisposable
private readonly ExtensionObject<IListPage> _model; private readonly ExtensionObject<IListPage> _model;
private readonly Lock _fetchStateLock = new();
private readonly Lock _listLock = new(); private readonly Lock _listLock = new();
private readonly IContextMenuFactory _contextMenuFactory; private readonly IContextMenuFactory _contextMenuFactory;
[ThreadStatic]
private static Dictionary<ListViewModel, int>? _getItemsDepthByViewModel;
private InterlockedBoolean _isLoading; private InterlockedBoolean _isLoading;
private bool _isFetching; private int _activeFetchCount;
private int _latestFetchGeneration;
private bool _deferredFetchRequested;
private bool _deferredFetchKeepSelection = true;
public event TypedEventHandler<ListViewModel, ItemsUpdatedEventArgs>? ItemsUpdated; public event TypedEventHandler<ListViewModel, ItemsUpdatedEventArgs>? ItemsUpdated;
public bool ShowEmptyContent => public bool ShowEmptyContent =>
IsInitialized && IsInitialized &&
FilteredItems.Count == 0 && FilteredItems.Count == 0 &&
(!_isFetching) && !IsFetching &&
IsLoading == false; !IsLoading;
public bool IsGridView { get; private set; } public bool IsGridView { get; private set; }
public IGridPropertiesViewModel? GridProperties { get; private set; } public IGridPropertiesViewModel? GridProperties { get; private set; }
private bool IsFetching => Volatile.Read(ref _activeFetchCount) > 0;
// Remember - "observable" properties from the model (via PropChanged) // Remember - "observable" properties from the model (via PropChanged)
// cannot be marked [ObservableProperty] // cannot be marked [ObservableProperty]
public bool ShowDetails { get; private set; } public bool ShowDetails { get; private set; }
@@ -123,8 +136,10 @@ public partial class ListViewModel : PageViewModel, IDisposable
} }
} }
// TODO: Does this need to hop to a _different_ thread, so that we don't block the extension while we're fetching? private void Model_ItemsChanged(object sender, IItemsChangedEventArgs args)
private void Model_ItemsChanged(object sender, IItemsChangedEventArgs args) => FetchItems(args.TotalItems == IncrementalRefresh); {
RequestFetch(args.TotalItems == IncrementalRefresh);
}
protected override void OnSearchTextBoxUpdated(string searchTextBox) protected override void OnSearchTextBoxUpdated(string searchTextBox)
{ {
@@ -132,9 +147,9 @@ public partial class ListViewModel : PageViewModel, IDisposable
// something needs to change, by raising ItemsChanged. // something needs to change, by raising ItemsChanged.
if (_isDynamic) if (_isDynamic)
{ {
filterCancellationTokenSource?.Cancel(); CancelAndDisposeTokenSource(ref filterCancellationTokenSource);
filterCancellationTokenSource?.Dispose(); var filterCts = filterCancellationTokenSource = new CancellationTokenSource();
filterCancellationTokenSource = new CancellationTokenSource(); var filterToken = filterCts.Token;
// Hop off to an exclusive scheduler background thread to update the // Hop off to an exclusive scheduler background thread to update the
// extension. We do this to ensure that all filter update requests // extension. We do this to ensure that all filter update requests
@@ -144,7 +159,7 @@ public partial class ListViewModel : PageViewModel, IDisposable
_ = filterTaskFactory.StartNew( _ = filterTaskFactory.StartNew(
() => () =>
{ {
filterCancellationTokenSource.Token.ThrowIfCancellationRequested(); filterToken.ThrowIfCancellationRequested();
try try
{ {
@@ -161,7 +176,7 @@ public partial class ListViewModel : PageViewModel, IDisposable
ShowException(ex, _model?.Unsafe?.Name); ShowException(ex, _model?.Unsafe?.Name);
} }
}, },
filterCancellationTokenSource.Token, filterToken,
TaskCreationOptions.None, TaskCreationOptions.None,
filterTaskFactory.Scheduler!); filterTaskFactory.Scheduler!);
} }
@@ -199,9 +214,73 @@ public partial class ListViewModel : PageViewModel, IDisposable
}); });
} }
private void RequestFetch(bool keepSelection)
{
// Keep RPC GetItems work off the UI thread. If the provider raises
// ItemsChanged while we're already on a background thread, stay on that
// thread so same-thread reentrancy detection still works.
if (IsCurrentThreadUiThread())
{
QueueObservedBackgroundFetch(() => RequestFetch(keepSelection), "Failed to request background fetch");
return;
}
if (IsGetItemsActiveOnCurrentThread())
{
lock (_fetchStateLock)
{
_deferredFetchRequested = true;
_deferredFetchKeepSelection &= keepSelection;
}
return;
}
FetchItems(keepSelection);
}
private void QueueDeferredFetchIfNeeded()
{
bool deferredFetchRequested;
bool keepSelection;
lock (_fetchStateLock)
{
deferredFetchRequested = _deferredFetchRequested;
keepSelection = _deferredFetchKeepSelection;
_deferredFetchRequested = false;
_deferredFetchKeepSelection = true;
}
if (deferredFetchRequested)
{
QueueObservedBackgroundFetch(() => FetchItems(keepSelection), "Failed to execute deferred fetch");
}
}
private static void QueueObservedBackgroundFetch(Action action, string logMessage)
{
_ = Task.Run(
() =>
{
try
{
action();
}
catch (OperationCanceledException)
{
}
catch (Exception ex)
{
CoreLogger.LogError(logMessage, ex);
}
});
}
//// Run on background thread, from InitializeAsync or Model_ItemsChanged //// Run on background thread, from InitializeAsync or Model_ItemsChanged
private void FetchItems(bool keepSelection) private void FetchItems(bool keepSelection)
{ {
System.Diagnostics.Debug.Assert(!IsCurrentThreadUiThread(), "FetchItems should not run on the UI thread.");
// If this fetch should reset selection, remember that intent even if // If this fetch should reset selection, remember that intent even if
// a later incremental fetch cancels us. // a later incremental fetch cancels us.
if (!keepSelection) if (!keepSelection)
@@ -209,34 +288,50 @@ public partial class ListViewModel : PageViewModel, IDisposable
_forceFirstItemPending = true; _forceFirstItemPending = true;
} }
// Cancel any previous FetchItems operation CancellationToken cancellationToken;
_fetchItemsCancellationTokenSource?.Cancel(); int fetchGeneration;
_fetchItemsCancellationTokenSource?.Dispose(); lock (_fetchStateLock)
_fetchItemsCancellationTokenSource = new CancellationTokenSource(); {
// Cancel any previous FetchItems operation
CancelAndDisposeTokenSource(ref _fetchItemsCancellationTokenSource);
_fetchItemsCancellationTokenSource = new CancellationTokenSource();
var cancellationToken = _fetchItemsCancellationTokenSource.Token; cancellationToken = _fetchItemsCancellationTokenSource.Token;
fetchGeneration = Interlocked.Increment(ref _latestFetchGeneration);
}
_isFetching = true; // Declared outside try so catch blocks can reference them
List<ListItemViewModel> createdViewModels = [];
// Collect all the items into new viewmodels var itemsTransferredToList = false;
List<ListItemViewModel> newViewModels = []; var fetchCountIncremented = false;
try try
{ {
// Check for cancellation before starting expensive operations fetchCountIncremented = true;
if (cancellationToken.IsCancellationRequested) if (Interlocked.Increment(ref _activeFetchCount) == 1)
{ {
return; UpdateEmptyContent();
} }
var newItems = _model.Unsafe!.GetItems(); ThrowIfFetchCanceledOrStale(fetchGeneration, cancellationToken);
// Check for cancellation after getting items from extension IListItem[] newItems;
if (cancellationToken.IsCancellationRequested) try
{ {
return; EnterGetItemsScope();
newItems = _model.Unsafe!.GetItems();
}
finally
{
ExitGetItemsScope();
} }
ThrowIfFetchCanceledOrStale(fetchGeneration, cancellationToken);
// Collect all the items into new viewmodels
List<ListItemViewModel> newViewModels = new(newItems.Length);
var currentCache = ReadVmCache();
var nextCache = new Dictionary<IListItem, ListItemViewModel>(newItems.Length, VmCacheComparer);
var showsTitle = GridProperties?.ShowTitle ?? true; var showsTitle = GridProperties?.ShowTitle ?? true;
var showsSubtitle = GridProperties?.ShowSubtitle ?? true; var showsSubtitle = GridProperties?.ShowSubtitle ?? true;
var created = 0; var created = 0;
@@ -250,17 +345,14 @@ public partial class ListViewModel : PageViewModel, IDisposable
continue; continue;
} }
// Check for cancellation during item processing ThrowIfFetchCanceledOrStale(fetchGeneration, cancellationToken);
if (cancellationToken.IsCancellationRequested)
{
return;
}
if (_vmCache.TryGetValue(item, out var existing)) if (nextCache.TryGetValue(item, out var existing) || currentCache.TryGetValue(item, out existing))
{ {
existing.LayoutShowsTitle = showsTitle; existing.LayoutShowsTitle = showsTitle;
existing.LayoutShowsSubtitle = showsSubtitle; existing.LayoutShowsSubtitle = showsSubtitle;
newViewModels.Add(existing); newViewModels.Add(existing);
nextCache[item] = existing;
reused++; reused++;
continue; continue;
} }
@@ -273,68 +365,65 @@ public partial class ListViewModel : PageViewModel, IDisposable
viewModel.LayoutShowsTitle = showsTitle; viewModel.LayoutShowsTitle = showsTitle;
viewModel.LayoutShowsSubtitle = showsSubtitle; viewModel.LayoutShowsSubtitle = showsSubtitle;
_vmCache[item] = viewModel;
newViewModels.Add(viewModel); newViewModels.Add(viewModel);
createdViewModels.Add(viewModel);
nextCache[item] = viewModel;
created++; created++;
} }
} }
catch (OperationCanceledException)
{
// Our own stale/cancel checks throw OCE to stop the whole fetch
// promptly. Only swallow item-local cancellations.
ThrowIfFetchCanceledOrStale(fetchGeneration, cancellationToken);
CoreLogger.LogDebug("Item load cancelled during fetch");
}
catch (Exception ex) catch (Exception ex)
{ {
CoreLogger.LogError("Failed to load item:\n", ex + ToString()); CoreLogger.LogError("Failed to load item:\n", ex);
} }
} }
#if DEBUG #if DEBUG
CoreLogger.LogInfo($"[ListViewModel] FetchItems: {created} created, {reused} reused, {_vmCache.Count} cached"); CoreLogger.LogInfo($"[ListViewModel] FetchItems: {created} created, {reused} reused, {nextCache.Count} cached");
#endif #endif
// Check for cancellation before initializing first twenty items ThrowIfFetchCanceledOrStale(fetchGeneration, cancellationToken);
if (cancellationToken.IsCancellationRequested)
{
return;
}
var firstTwenty = newViewModels.Take(20); var firstTwenty = newViewModels.Take(20);
foreach (var item in firstTwenty) foreach (var item in firstTwenty)
{ {
if (cancellationToken.IsCancellationRequested) ThrowIfFetchCanceledOrStale(fetchGeneration, cancellationToken);
{
return;
}
item?.SafeInitializeProperties(); item?.SafeInitializeProperties();
} }
// Cancel any ongoing search // Cancel any ongoing property initialization for the previous list.
_cancellationTokenSource?.Cancel(); CancelAndDisposeTokenSource(ref _cancellationTokenSource);
// Check for cancellation before updating the list ThrowIfFetchCanceledOrStale(fetchGeneration, cancellationToken);
if (cancellationToken.IsCancellationRequested)
{
return;
}
List<ListItemViewModel> removedItems; List<ListItemViewModel> removedItems;
lock (_listLock) lock (_fetchStateLock)
{ {
// Now that we have new ViewModels for everything from the ThrowIfFetchCanceledOrStale(fetchGeneration, cancellationToken);
// extension, smartly update our list of VMs
ListHelpers.InPlaceUpdateList(Items, newViewModels, out removedItems);
_vmCache.Clear(); lock (_listLock)
foreach (var vm in newViewModels)
{ {
if (vm.Model.Unsafe is { } li) // Now that we have new ViewModels for everything from the
{ // extension, smartly update our list of VMs
_vmCache[li] = vm; ListHelpers.InPlaceUpdateList(Items, newViewModels, out removedItems);
}
}
// DO NOT ThrowIfCancellationRequested AFTER THIS! If you do, PublishVmCache(nextCache);
// you'll clean up list items that we've now transferred into
// .Items // DO NOT ThrowIfCancellationRequested AFTER THIS! If you do,
// you'll clean up list items that we've now transferred into
// .Items
}
} }
itemsTransferredToList = true;
// If we removed items, we need to clean them up, to remove our event handlers // If we removed items, we need to clean them up, to remove our event handlers
foreach (var removedItem in removedItems) foreach (var removedItem in removedItems)
{ {
@@ -348,9 +437,12 @@ public partial class ListViewModel : PageViewModel, IDisposable
// However, if we were cancelled, we didn't actually add these items to // However, if we were cancelled, we didn't actually add these items to
// our Items list. Before we release them to the GC, make sure we clean // our Items list. Before we release them to the GC, make sure we clean
// them up // them up
foreach (var vm in newViewModels) if (!itemsTransferredToList)
{ {
vm.SafeCleanup(); foreach (var vm in createdViewModels)
{
vm.SafeCleanup();
}
} }
return; return;
@@ -359,52 +451,83 @@ public partial class ListViewModel : PageViewModel, IDisposable
{ {
// TODO: Move this within the for loop, so we can catch issues with individual items // TODO: Move this within the for loop, so we can catch issues with individual items
// Create a special ListItemViewModel for errors and use an ItemTemplateSelector in the ListPage to display error items differently. // Create a special ListItemViewModel for errors and use an ItemTemplateSelector in the ListPage to display error items differently.
if (!itemsTransferredToList)
{
foreach (var vm in createdViewModels)
{
vm.SafeCleanup();
}
}
ShowException(ex, _model?.Unsafe?.Name); ShowException(ex, _model?.Unsafe?.Name);
throw; throw;
} }
finally finally
{ {
_isFetching = false; if (fetchCountIncremented && Interlocked.Decrement(ref _activeFetchCount) == 0)
{
UpdateEmptyContent();
}
} }
_cancellationTokenSource = new CancellationTokenSource(); var initializeItemsCts = new CancellationTokenSource();
_cancellationTokenSource = initializeItemsCts;
var initializeItemsToken = initializeItemsCts.Token;
_initializeItemsTask = new Task(() => _initializeItemsTask = new Task(() =>
{ {
InitializeItemsTask(_cancellationTokenSource.Token); InitializeItemsTask(initializeItemsToken);
}); });
_initializeItemsTask.Start(); _initializeItemsTask.Start();
DoOnUiThread( DoOnUiThread(
() => () =>
{ {
lock (_listLock) lock (_fetchStateLock)
{ {
// Now that our Items contains everything we want, it's time for us to if (!IsLatestFetchGeneration(fetchGeneration))
// re-evaluate our Filter on those items.
if (!_isDynamic)
{ {
// A static list? Great! Just run the filter. return;
ApplyFilterUnderLock();
}
else
{
// A dynamic list? Even better! Just stick everything into
// FilteredItems. The extension already did any filtering it cared about.
var snapshot = Items.Where(i => !i.IsInErrorState).ToList();
ListHelpers.InPlaceUpdateList(FilteredItems, snapshot);
} }
UpdateEmptyContent(); lock (_listLock)
{
if (!IsLatestFetchGeneration(fetchGeneration))
{
return;
}
// Now that our Items contains everything we want, it's time for us to
// re-evaluate our Filter on those items.
if (!_isDynamic)
{
// A static list? Great! Just run the filter.
ApplyFilterUnderLock();
}
else
{
// A dynamic list? Even better! Just stick everything into
// FilteredItems. The extension already did any filtering it cared about.
var snapshot = Items.Where(i => !i.IsInErrorState).ToList();
ListHelpers.InPlaceUpdateList(FilteredItems, snapshot);
}
UpdateEmptyContent();
}
if (!IsLatestFetchGeneration(fetchGeneration))
{
return;
}
// Consume the pending flag on the UI thread so a
// forceFirstItem=true intent survives cancellation.
var forceFirst = _forceFirstItemPending;
_forceFirstItemPending = false;
ItemsUpdated?.Invoke(this, new ItemsUpdatedEventArgs(forceFirstItem: IsRootPage && forceFirst));
_isLoading.Clear();
} }
// Consume the pending flag on the UI thread so a
// forceFirstItem=true intent survives cancellation.
var forceFirst = _forceFirstItemPending;
_forceFirstItemPending = false;
ItemsUpdated?.Invoke(this, new ItemsUpdatedEventArgs(forceFirstItem: IsRootPage && forceFirst));
_isLoading.Clear();
}); });
} }
@@ -449,6 +572,105 @@ public partial class ListViewModel : PageViewModel, IDisposable
/// </summary> /// </summary>
private void ApplyFilterUnderLock() => ListHelpers.InPlaceUpdateList(FilteredItems, FilterList(Items, SearchTextBox)); private void ApplyFilterUnderLock() => ListHelpers.InPlaceUpdateList(FilteredItems, FilterList(Items, SearchTextBox));
private Dictionary<IListItem, ListItemViewModel> ReadVmCache() => Volatile.Read(ref _vmCache);
private static bool IsCurrentThreadUiThread()
{
try
{
return DispatcherQueue.GetForCurrentThread()?.HasThreadAccess == true;
}
catch (COMException)
{
return false;
}
}
/// <summary>
/// Detects if we're currently within a GetItems call on this thread for this view model. This is used to detect
/// reentrant calls to GetItems, so we can defer subsequent calls until the first one finishes, to avoid
/// concurrent GetItems calls which most extensions won't be expecting.
/// </summary>
/// <returns>
/// <see langword="true"/> if we're currently within a GetItems call on this thread for this view model; otherwise, <see langword="false"/>.
/// </returns>
private bool IsGetItemsActiveOnCurrentThread()
{
var depths = _getItemsDepthByViewModel;
return depths is not null &&
depths.TryGetValue(this, out var depth) &&
depth > 0;
}
private void EnterGetItemsScope()
{
var depths = _getItemsDepthByViewModel ??= [];
depths.TryGetValue(this, out var depth);
depths[this] = depth + 1;
}
private void ExitGetItemsScope()
{
var depths = _getItemsDepthByViewModel;
if (depths is null || !depths.TryGetValue(this, out var depth))
{
return;
}
if (depth == 1)
{
depths.Remove(this);
if (depths.Count == 0)
{
_getItemsDepthByViewModel = null;
}
try
{
QueueDeferredFetchIfNeeded();
}
catch (Exception ex)
{
CoreLogger.LogError("Failed to queue deferred fetch", ex);
}
}
else
{
depths[this] = depth - 1;
}
}
private static void CancelAndDisposeTokenSource(ref CancellationTokenSource? tokenSource)
{
var tokenSourceToDispose = Interlocked.Exchange(ref tokenSource, null);
if (tokenSourceToDispose is null)
{
return;
}
tokenSourceToDispose.Cancel();
tokenSourceToDispose.Dispose();
}
private void ThrowIfFetchCanceledOrStale(int fetchGeneration, CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();
if (Volatile.Read(ref _latestFetchGeneration) != fetchGeneration)
{
throw new OperationCanceledException();
}
}
private bool IsLatestFetchGeneration(int fetchGeneration)
{
return Volatile.Read(ref _latestFetchGeneration) == fetchGeneration;
}
private void PublishVmCache(Dictionary<IListItem, ListItemViewModel> newCache)
{
Volatile.Write(ref _vmCache, newCache);
}
/// <summary> /// <summary>
/// Helper to generate a weighting for a given list item, based on title, /// Helper to generate a weighting for a given list item, based on title,
/// subtitle, etc. Largely a copy of the version in ListHelpers, but /// subtitle, etc. Largely a copy of the version in ListHelpers, but
@@ -548,7 +770,7 @@ public partial class ListViewModel : PageViewModel, IDisposable
// Cancel any in-flight slow init from a previous selection and defer // Cancel any in-flight slow init from a previous selection and defer
// the expensive work (extension IPC for MoreCommands, details) so // the expensive work (extension IPC for MoreCommands, details) so
// rapid arrow-key navigation skips intermediate items entirely. // rapid arrow-key navigation skips intermediate items entirely.
_selectedItemCts?.Cancel(); CancelAndDisposeTokenSource(ref _selectedItemCts);
var cts = _selectedItemCts = new CancellationTokenSource(); var cts = _selectedItemCts = new CancellationTokenSource();
var ct = cts.Token; var ct = cts.Token;
@@ -634,7 +856,7 @@ public partial class ListViewModel : PageViewModel, IDisposable
private void ClearSelectedItem() private void ClearSelectedItem()
{ {
_selectedItemCts?.Cancel(); CancelAndDisposeTokenSource(ref _selectedItemCts);
WeakReferenceMessenger.Default.Send<UpdateCommandBarMessage>(new(null)); WeakReferenceMessenger.Default.Send<UpdateCommandBarMessage>(new(null));
WeakReferenceMessenger.Default.Send<HideDetailsMessage>(); WeakReferenceMessenger.Default.Send<HideDetailsMessage>();
@@ -820,21 +1042,10 @@ public partial class ListViewModel : PageViewModel, IDisposable
public void Dispose() public void Dispose()
{ {
GC.SuppressFinalize(this); GC.SuppressFinalize(this);
_cancellationTokenSource?.Cancel(); CancelAndDisposeTokenSource(ref _cancellationTokenSource);
_cancellationTokenSource?.Dispose(); CancelAndDisposeTokenSource(ref filterCancellationTokenSource);
_cancellationTokenSource = null; CancelAndDisposeTokenSource(ref _fetchItemsCancellationTokenSource);
CancelAndDisposeTokenSource(ref _selectedItemCts);
filterCancellationTokenSource?.Cancel();
filterCancellationTokenSource?.Dispose();
filterCancellationTokenSource = null;
_fetchItemsCancellationTokenSource?.Cancel();
_fetchItemsCancellationTokenSource?.Dispose();
_fetchItemsCancellationTokenSource = null;
_selectedItemCts?.Cancel();
_selectedItemCts?.Dispose();
_selectedItemCts = null;
} }
protected override void UnsafeCleanup() protected override void UnsafeCleanup()
@@ -844,10 +1055,10 @@ public partial class ListViewModel : PageViewModel, IDisposable
EmptyContent?.SafeCleanup(); EmptyContent?.SafeCleanup();
EmptyContent = new(new(null), PageContext, contextMenuFactory: null); // necessary? EmptyContent = new(new(null), PageContext, contextMenuFactory: null); // necessary?
_cancellationTokenSource?.Cancel(); CancelAndDisposeTokenSource(ref _cancellationTokenSource);
filterCancellationTokenSource?.Cancel(); CancelAndDisposeTokenSource(ref filterCancellationTokenSource);
_fetchItemsCancellationTokenSource?.Cancel(); CancelAndDisposeTokenSource(ref _fetchItemsCancellationTokenSource);
_selectedItemCts?.Cancel(); CancelAndDisposeTokenSource(ref _selectedItemCts);
lock (_listLock) lock (_listLock)
{ {
@@ -865,6 +1076,8 @@ public partial class ListViewModel : PageViewModel, IDisposable
FilteredItems.Clear(); FilteredItems.Clear();
} }
PublishVmCache(new(VmCacheComparer));
Filters?.PropertyChanged -= FiltersPropertyChanged; Filters?.PropertyChanged -= FiltersPropertyChanged;
Filters?.SafeCleanup(); Filters?.SafeCleanup();

View File

@@ -0,0 +1,89 @@
// 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.Threading;
using System.Threading.Tasks;
using Microsoft.CommandPalette.Extensions;
using Microsoft.CommandPalette.Extensions.Toolkit;
using Microsoft.VisualStudio.TestTools.UnitTesting;
namespace Microsoft.CmdPal.UI.ViewModels.UnitTests;
[TestClass]
public partial class ListViewModelTests
{
private sealed partial class TestAppExtensionHost : AppExtensionHost
{
public override string? GetExtensionDisplayName() => "Test Host";
}
private sealed partial class RecursiveItemsChangedPage : ListPage
{
private int _getItemsCallCount;
private int _recursiveItemsChangedRaised;
private TaskCompletionSource<bool> _deferredFetchObserved = NewDeferredFetchObserved();
public int GetItemsCallCount => Volatile.Read(ref _getItemsCallCount);
public Task DeferredFetchObserved => _deferredFetchObserved.Task;
public bool RaiseItemsChangedDuringGetItems { get; set; }
public override IListItem[] GetItems()
{
var callCount = Interlocked.Increment(ref _getItemsCallCount);
if (callCount >= 2)
{
_deferredFetchObserved.TrySetResult(true);
}
if (RaiseItemsChangedDuringGetItems && Interlocked.Exchange(ref _recursiveItemsChangedRaised, 1) == 0)
{
RaiseItemsChanged(0);
}
return [new ListItem(new NoOpCommand() { Name = $"Item {callCount}" })];
}
public void PrepareRecursiveFetch()
{
Volatile.Write(ref _getItemsCallCount, 0);
Volatile.Write(ref _recursiveItemsChangedRaised, 0);
RaiseItemsChangedDuringGetItems = true;
_deferredFetchObserved = NewDeferredFetchObserved();
}
public void TriggerItemsChanged(int totalItems = 0) => RaiseItemsChanged(totalItems);
private static TaskCompletionSource<bool> NewDeferredFetchObserved() => new(TaskCreationOptions.RunContinuationsAsynchronously);
}
private static ListViewModel CreateViewModel(RecursiveItemsChangedPage page) =>
new(page, TaskScheduler.Default, new TestAppExtensionHost(), CommandProviderContext.Empty, DefaultContextMenuFactory.Instance);
[TestMethod]
public async Task RecursiveItemsChangedDuringGetItems_IsDeferredUntilGetItemsReturns()
{
var page = new RecursiveItemsChangedPage
{
Id = "list.page",
Name = "List Page",
Title = "List Page",
};
var viewModel = CreateViewModel(page);
viewModel.InitializeProperties();
page.PrepareRecursiveFetch();
page.TriggerItemsChanged();
var completed = await Task.WhenAny(page.DeferredFetchObserved, Task.Delay(TimeSpan.FromSeconds(2)));
Assert.AreSame(page.DeferredFetchObserved, completed);
Assert.AreEqual(2, page.GetItemsCallCount);
viewModel.SafeCleanup();
viewModel.Dispose();
}
}