[PR #1018] [MERGED] Fix race condition in ObservableCollection replacement causing VirtualizingWrapPanel crash #941

New issue

Closed

opened 2026-02-26 12:00:34 +03:00 by kerem · 0 comments

kerem commented

2026-02-26 12:00:34 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/1Remote/1Remote/pull/1018
Author: @Copilot
Created: 11/6/2025
Status: ✅ Merged
Merged: 11/21/2025
Merged by: @VShawn

Base: main ← Head: copilot/fix-crash-issue-1014

📝 Commits (4)

a93a93f Initial plan
2a05d3f Fix race condition crash in VirtualizingWrapPanel during ListView rebuild
daf62cd Apply same fix to ServerTreeViewModel for consistency
1f45c00 Remove redundant PropertyChanged unsubscribe operations

📊 Changes

2 files changed (+46 additions, -21 deletions)

View changed files

📝 Ui/View/ServerView/List/ServerListPageViewModel.cs (+25 -13)
📝 Ui/View/ServerView/Tree/ServerTreeViewModel.cs (+21 -8)

📄 Description

Closes #1014

Description

The application crashes with ArgumentOutOfRangeException during window resize when ListView is rebuilt. The crash occurs in VirtualizingWrapPanel.Virtualize() when accessing collection indices.

Root cause: BuildView() creates a new ObservableCollection and assigns it to VmServerList, breaking WPF binding references. When window resize triggers layout measurement immediately after, the virtualization panel accesses stale indices from the replaced collection.

Changes

Modify collection in-place: Replace new ObservableCollection<T>() assignment with Clear() + Add() pattern in both ServerListPageViewModel and ServerTreeViewModel
Fix memory leak: Move CollectionChanged handler registration from BuildView() to constructor (was re-registering on every rebuild)
Remove redundant code: Eliminate duplicate PropertyChanged unsubscribes already performed earlier

// Before: Replaces entire collection reference
VmServerList = new ObservableCollection<ProtocolBaseViewModel>(list);

// After: Modifies existing collection
VmServerList.Clear();
foreach (var item in list)
{
    VmServerList.Add(item);
}

Tradeoffs

Slightly more verbose code (explicit loop vs constructor) in exchange for collection reference stability and WPF binding safety.

Original prompt

check issue #1014, and find the crash reason.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/1Remote/1Remote/pull/1018 **Author:** [@Copilot](https://github.com/apps/copilot-swe-agent) **Created:** 11/6/2025 **Status:** ✅ Merged **Merged:** 11/21/2025 **Merged by:** [@VShawn](https://github.com/VShawn) **Base:** `main` ← **Head:** `copilot/fix-crash-issue-1014` --- ### 📝 Commits (4) - [`a93a93f`](https://github.com/1Remote/1Remote/commit/a93a93f6a353b462d0d7c38d584f3d6be02ab79e) Initial plan - [`2a05d3f`](https://github.com/1Remote/1Remote/commit/2a05d3f7ee2e24fb1cb829c67c3077834d789520) Fix race condition crash in VirtualizingWrapPanel during ListView rebuild - [`daf62cd`](https://github.com/1Remote/1Remote/commit/daf62cd4065922e443e08b3bbaccd2fb73faf33e) Apply same fix to ServerTreeViewModel for consistency - [`1f45c00`](https://github.com/1Remote/1Remote/commit/1f45c0004d27bf10c50c528eba5ca62f2a29c99c) Remove redundant PropertyChanged unsubscribe operations ### 📊 Changes **2 files changed** (+46 additions, -21 deletions) <details> <summary>View changed files</summary> 📝 `Ui/View/ServerView/List/ServerListPageViewModel.cs` (+25 -13) 📝 `Ui/View/ServerView/Tree/ServerTreeViewModel.cs` (+21 -8) </details> ### 📄 Description Closes #1014 ## Description The application crashes with `ArgumentOutOfRangeException` during window resize when ListView is rebuilt. The crash occurs in `VirtualizingWrapPanel.Virtualize()` when accessing collection indices. **Root cause**: `BuildView()` creates a new `ObservableCollection` and assigns it to `VmServerList`, breaking WPF binding references. When window resize triggers layout measurement immediately after, the virtualization panel accesses stale indices from the replaced collection. ## Changes - **Modify collection in-place**: Replace `new ObservableCollection<T>()` assignment with `Clear()` + `Add()` pattern in both `ServerListPageViewModel` and `ServerTreeViewModel` - **Fix memory leak**: Move `CollectionChanged` handler registration from `BuildView()` to constructor (was re-registering on every rebuild) - **Remove redundant code**: Eliminate duplicate `PropertyChanged` unsubscribes already performed earlier ```csharp // Before: Replaces entire collection reference VmServerList = new ObservableCollection<ProtocolBaseViewModel>(list); // After: Modifies existing collection VmServerList.Clear(); foreach (var item in list) { VmServerList.Add(item); } ``` ## Tradeoffs Slightly more verbose code (explicit loop vs constructor) in exchange for collection reference stability and WPF binding safety.  <details> <summary>Original prompt</summary> > check issue #1014, and find the crash reason. </details>  --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>