From dbad946b6d9fb776a2101b5259af40a0fdf2323c Mon Sep 17 00:00:00 2001
From: MaoShengelia <82882980+MaoShengelia@users.noreply.github.com>
Date: Thu, 10 Jul 2025 22:10:38 +0400
Subject: [PATCH] Adding Lock to RecentCommandsManager (#40507)

According to Issue #40447 Without the Lock in RecentCommandsManager we
get Exception:
Collection was modified; enumeration operation may not execute.

It indicated that while GetCommandHistoryWeight was enumerating History,
another method (likely AddHistoryItem) modified it at the same time.
Since List is not thread-safe, simultaneous read+write can break it.

## Summary of the Pull Request

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

- [ ] **Closes:** #xxx
- [ ] **Communication:** I've discussed this with core contributors
already. If the work hasn't been agreed, this work might be rejected
- [ ] **Tests:** Unit Tests all pass, Manually Tested as Well
- [ ] **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

I've checked that in this project we use the new .NET 9 locking instead
of object locking and implemented it according to rest of the locks
## Detailed Description of the Pull Request / Additional comments

I've Manually tested it and also made sure that all the unit test pass
## Validation Steps Performed
---
 .../RecentCommandsManager.cs                  | 80 ++++++++++---------
 1 file changed, 44 insertions(+), 36 deletions(-)

diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/RecentCommandsManager.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/RecentCommandsManager.cs
index c740341c7a..8551b5f964 100644
--- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/RecentCommandsManager.cs
+++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/RecentCommandsManager.cs
@@ -12,63 +12,71 @@ public partial class RecentCommandsManager : ObservableObject
     [JsonInclude]
     internal List<HistoryItem> History { get; set; } = [];
 
+    private readonly Lock _lock = new();
+
     public RecentCommandsManager()
     {
     }
 
     public int GetCommandHistoryWeight(string commandId)
     {
-        var entry = History
+        lock (_lock)
+        {
+            var entry = History
             .Index()
             .Where(item => item.Item.CommandId == commandId)
             .FirstOrDefault();
 
-        // These numbers are vaguely scaled so that "VS" will make "Visual Studio" the
-        // match after one use.
-        // Usually it has a weight of 84, compared to 109 for the VS cmd prompt
-        if (entry.Item != null)
-        {
-            var index = entry.Index;
-
-            // First, add some weight based on how early in the list this appears
-            var bucket = index switch
+            // These numbers are vaguely scaled so that "VS" will make "Visual Studio" the
+            // match after one use.
+            // Usually it has a weight of 84, compared to 109 for the VS cmd prompt
+            if (entry.Item != null)
             {
-                var i when index <= 2 => 35,
-                var i when index <= 10 => 25,
-                var i when index <= 15 => 15,
-                var i when index <= 35 => 10,
-                _ => 5,
-            };
+                var index = entry.Index;
 
-            // Then, add weight for how often this is used, but cap the weight from usage.
-            var uses = Math.Min(entry.Item.Uses * 5, 35);
+                // First, add some weight based on how early in the list this appears
+                var bucket = index switch
+                {
+                    var i when index <= 2 => 35,
+                    var i when index <= 10 => 25,
+                    var i when index <= 15 => 15,
+                    var i when index <= 35 => 10,
+                    _ => 5,
+                };
 
-            return bucket + uses;
+                // Then, add weight for how often this is used, but cap the weight from usage.
+                var uses = Math.Min(entry.Item.Uses * 5, 35);
+
+                return bucket + uses;
+            }
+
+            return 0;
         }
-
-        return 0;
     }
 
     public void AddHistoryItem(string commandId)
     {
-        var entry = History
+        lock (_lock)
+        {
+            var entry = History
             .Where(item => item.CommandId == commandId)
             .FirstOrDefault();
-        if (entry == null)
-        {
-            var newitem = new HistoryItem() { CommandId = commandId, Uses = 1 };
-            History.Insert(0, newitem);
-        }
-        else
-        {
-            History.Remove(entry);
-            entry.Uses++;
-            History.Insert(0, entry);
-        }
+            if (entry == null)
+            {
+                var newitem = new HistoryItem() { CommandId = commandId, Uses = 1 };
+                History.Insert(0, newitem);
+            }
+            else
+            {
+                History.Remove(entry);
+                entry.Uses++;
+                History.Insert(0, entry);
+            }
 
-        if (History.Count > 50)
-        {
-            History.RemoveRange(50, History.Count - 50);
+            if (History.Count > 50)
+            {
+                History.RemoveRange(50, History.Count - 50);
+            }
         }
     }
 }