From 8dbff245d6634630d03d0cbb01df1ade4adaa689 Mon Sep 17 00:00:00 2001
From: Mike Griese <migrie@microsoft.com>
Date: Tue, 19 Aug 2025 06:20:06 -0500
Subject: [PATCH] CmdPal: cancel fetching new items when we get another one
 (#41166)

This just adds a simple `CancellationToken` around
`ListViewModel.FetchItems()`. Now, when we start a second `FetchItems`
(in responce to a `RaiseItemsChanged`), we'll cancel the old one first.
That'll prevent a particularly long first `GetItems` call from returning
after a second one has already set the list.

Closes #41149

No longer repros the evil sample from #41158
---
 .../ListViewModel.cs                          | 34 +++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/ListViewModel.cs b/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/ListViewModel.cs
index 5a5a14fbdd..c461943f8a 100644
--- a/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/ListViewModel.cs
+++ b/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/ListViewModel.cs
@@ -63,6 +63,7 @@ public partial class ListViewModel : PageViewModel, IDisposable
 
     private Task? _initializeItemsTask;
     private CancellationTokenSource? _cancellationTokenSource;
+    private CancellationTokenSource? _fetchItemsCancellationTokenSource;
 
     private ListItemViewModel? _lastSelectedItem;
 
@@ -129,14 +130,27 @@ public partial class ListViewModel : PageViewModel, IDisposable
     //// Run on background thread, from InitializeAsync or Model_ItemsChanged
     private void FetchItems()
     {
+        // Cancel any previous FetchItems operation
+        _fetchItemsCancellationTokenSource?.Cancel();
+        _fetchItemsCancellationTokenSource?.Dispose();
+        _fetchItemsCancellationTokenSource = new CancellationTokenSource();
+
+        var cancellationToken = _fetchItemsCancellationTokenSource.Token;
+
         // TEMPORARY: just plop all the items into a single group
         // see 9806fe5d8 for the last commit that had this with sections
         _isFetching = true;
 
         try
         {
+            // Check for cancellation before starting expensive operations
+            cancellationToken.ThrowIfCancellationRequested();
+
             var newItems = _model.Unsafe!.GetItems();
 
+            // Check for cancellation after getting items from extension
+            cancellationToken.ThrowIfCancellationRequested();
+
             // Collect all the items into new viewmodels
             Collection<ListItemViewModel> newViewModels = [];
 
@@ -145,6 +159,9 @@ public partial class ListViewModel : PageViewModel, IDisposable
             // building new viewmodels for the ones we haven't already built.
             foreach (var item in newItems)
             {
+                // Check for cancellation during item processing
+                cancellationToken.ThrowIfCancellationRequested();
+
                 ListItemViewModel viewModel = new(item, new(this));
 
                 // If an item fails to load, silently ignore it.
@@ -154,15 +171,22 @@ public partial class ListViewModel : PageViewModel, IDisposable
                 }
             }
 
+            // Check for cancellation before initializing first twenty items
+            cancellationToken.ThrowIfCancellationRequested();
+
             var firstTwenty = newViewModels.Take(20);
             foreach (var item in firstTwenty)
             {
+                cancellationToken.ThrowIfCancellationRequested();
                 item?.SafeInitializeProperties();
             }
 
             // Cancel any ongoing search
             _cancellationTokenSource?.Cancel();
 
+            // Check for cancellation before updating the list
+            cancellationToken.ThrowIfCancellationRequested();
+
             lock (_listLock)
             {
                 // Now that we have new ViewModels for everything from the
@@ -173,6 +197,11 @@ public partial class ListViewModel : PageViewModel, IDisposable
             // TODO: Iterate over everything in Items, and prune items from the
             // cache if we don't need them anymore
         }
+        catch (OperationCanceledException)
+        {
+            // Cancellation is expected, don't treat as error
+            return;
+        }
         catch (Exception ex)
         {
             // TODO: Move this within the for loop, so we can catch issues with individual items
@@ -560,6 +589,10 @@ public partial class ListViewModel : PageViewModel, IDisposable
         _cancellationTokenSource?.Cancel();
         _cancellationTokenSource?.Dispose();
         _cancellationTokenSource = null;
+
+        _fetchItemsCancellationTokenSource?.Cancel();
+        _fetchItemsCancellationTokenSource?.Dispose();
+        _fetchItemsCancellationTokenSource = null;
     }
 
     protected override void UnsafeCleanup()
@@ -570,6 +603,7 @@ public partial class ListViewModel : PageViewModel, IDisposable
         EmptyContent = new(new(null), PageContext); // necessary?
 
         _cancellationTokenSource?.Cancel();
+        _fetchItemsCancellationTokenSource?.Cancel();
 
         lock (_listLock)
         {