CmdPal: Harden ListViewModel fetch synchronization#46429
Open
jiripolasek wants to merge 3 commits intomainfrom
Open
CmdPal: Harden ListViewModel fetch synchronization#46429jiripolasek wants to merge 3 commits intomainfrom
jiripolasek wants to merge 3 commits intomainfrom
Conversation
- 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.
- 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.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens CmdPal ListViewModel item fetching to better handle overlapping refreshes, cancellation, and re-entrant ItemsChanged scenarios (notably the recursive GetItems() stack overflow pattern in #46329/#46429).
Changes:
- Reworks fetch synchronization: generation-based “latest fetch wins”, copy-on-write VM cache publication, and more robust cancellation/cleanup.
- Adds a thread-local re-entrancy guard to defer
ItemsChanged-triggered fetches raised duringGetItems()until unwinding. - Adds a regression unit test covering recursive refresh behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ListViewModel.cs |
Implements new fetch synchronization model (locks, generation tracking, deferred fetch), VM cache copy-on-write, and CTS cleanup helper. |
src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ListViewModelTests.cs |
Adds a regression test ensuring ItemsChanged during GetItems() triggers a deferred (non-recursive) subsequent fetch. |
src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ListViewModel.cs
Outdated
Show resolved
Hide resolved
src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ListViewModel.cs
Outdated
Show resolved
Hide resolved
src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ListViewModelTests.cs
Outdated
Show resolved
Hide resolved
Contributor
Collaborator
Author
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary of the Pull Request
This PR improves fetching of list items in ListViewModel:
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed