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
This commit is contained in:
MaoShengelia
2025-07-10 22:10:38 +04:00
committed by GitHub
parent 74b6650a19
commit dbad946b6d

View File

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