From b2eabc51f8368d278ffb4c37beb9ce66a31d291b Mon Sep 17 00:00:00 2001 From: Hendrik Mennen Date: Wed, 26 Aug 2026 18:16:22 +0200 Subject: [PATCH] Fix ArgumentOutOfRangeException crash in Problems list Avalonia's DataGridCollectionView.ProcessRemoveEvent does not check PassesFilter (unlike ProcessAddEvent). When an item that is filtered out of the view is removed from the source collection, removeIndex is -1, so AdjustCurrencyForRemove(-1) takes the `index < CurrentPosition` branch and sets CurrentPosition to -1 while CurrentItem is still in view. The following IsCurrentInSync check then calls GetItemAt(-1) and throws. In practice this crashed the app whenever a problem row at position 0 was current and a filtered-out error was removed, e.g. from IErrorService.RefreshErrors during AI project diagnostics refresh. Route all removal paths of the error list through a new BatchObservableCollection that suppresses notifications and raises a single Reset, which DataGridCollectionView handles via a safe full refresh that restores currency. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../BatchObservableCollection.cs | 67 +++++++++++++++++++ .../ViewModels/ErrorListViewModel.cs | 36 +++++++--- 2 files changed, 94 insertions(+), 9 deletions(-) create mode 100644 src/OneWare.ErrorList/BatchObservableCollection.cs diff --git a/src/OneWare.ErrorList/BatchObservableCollection.cs b/src/OneWare.ErrorList/BatchObservableCollection.cs new file mode 100644 index 000000000..b8ee57c1a --- /dev/null +++ b/src/OneWare.ErrorList/BatchObservableCollection.cs @@ -0,0 +1,67 @@ +using System.Collections.ObjectModel; +using System.Collections.Specialized; +using System.ComponentModel; + +namespace OneWare.ErrorList; + +/// +/// ObservableCollection that can suppress change notifications while a batch of mutations is applied and +/// raises a single Reset afterwards. +/// This is required for , which crashes +/// (ArgumentOutOfRangeException in AdjustCurrencyForRemove) when an item that is filtered out of the view is +/// removed while the current position is 0. Handling a Reset makes the view rebuild itself safely. +/// +internal class BatchObservableCollection : ObservableCollection +{ + private bool _isDirty; + private int _suspendLevel; + + public IDisposable BeginBatch() + { + _suspendLevel++; + return new BatchScope(this); + } + + protected override void OnCollectionChanged(NotifyCollectionChangedEventArgs e) + { + if (_suspendLevel > 0) + { + _isDirty = true; + return; + } + + base.OnCollectionChanged(e); + } + + protected override void OnPropertyChanged(PropertyChangedEventArgs e) + { + if (_suspendLevel > 0) return; + + base.OnPropertyChanged(e); + } + + private void EndBatch() + { + if (_suspendLevel == 0) return; + + _suspendLevel--; + if (_suspendLevel > 0 || !_isDirty) return; + + _isDirty = false; + OnPropertyChanged(new PropertyChangedEventArgs(nameof(Count))); + OnPropertyChanged(new PropertyChangedEventArgs("Item[]")); + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); + } + + private sealed class BatchScope(BatchObservableCollection owner) : IDisposable + { + private bool _disposed; + + public void Dispose() + { + if (_disposed) return; + _disposed = true; + owner.EndBatch(); + } + } +} diff --git a/src/OneWare.ErrorList/ViewModels/ErrorListViewModel.cs b/src/OneWare.ErrorList/ViewModels/ErrorListViewModel.cs index 97bacff8d..7f25b75eb 100644 --- a/src/OneWare.ErrorList/ViewModels/ErrorListViewModel.cs +++ b/src/OneWare.ErrorList/ViewModels/ErrorListViewModel.cs @@ -22,7 +22,7 @@ public class ErrorListViewModel : ExtendedTool, IErrorService { public const string IconKey = "MaterialDesign.ErrorOutline"; - private readonly ObservableCollection _items = new(); + private readonly BatchObservableCollection _items = new(); private readonly IMainDockService _mainDockService; private readonly IProjectExplorerService _projectExplorerExplorerViewModel; @@ -194,7 +194,11 @@ public void RegisterErrorSource(string source) public void ClearFile(string filePath) { - ListEx.RemoveMany(_items, _items.Where(x => x.FilePath.EqualPaths(filePath))); + using (_items.BeginBatch()) + { + ListEx.RemoveMany(_items, _items.Where(x => x.FilePath.EqualPaths(filePath))); + } + ErrorRefresh?.Invoke(this, filePath); RefreshCountToggle(); } @@ -204,7 +208,10 @@ public void Clear(string source) var errors = _items.Where(x => x.Source == source).ToList(); var files = errors.Select(x => x.FilePath).Distinct(); - ListEx.RemoveMany(_items, errors); + using (_items.BeginBatch()) + { + ListEx.RemoveMany(_items, errors); + } foreach (var file in files) ErrorRefresh?.Invoke(this, file); @@ -231,10 +238,13 @@ public IEnumerable GetErrorsForFile(string filePath) /// public void RefreshErrors(IList errors, string source, string filePath) { - ListEx.RemoveMany(_items, - _items.Where(x => x.FilePath.EqualPaths(filePath) && x.Source == source && !errors.Contains(x))); + using (_items.BeginBatch()) + { + ListEx.RemoveMany(_items, + _items.Where(x => x.FilePath.EqualPaths(filePath) && x.Source == source && !errors.Contains(x))); - foreach (var e in errors) Add(e); + foreach (var e in errors) Add(e); + } ErrorRefresh?.Invoke(this, filePath); RefreshCountToggle(); @@ -328,15 +338,23 @@ private void RefreshCountToggle() public void Clear(IProjectRoot project) { - ListEx.RemoveMany(_items, _items.Where(x => x.Root == project)); + using (_items.BeginBatch()) + { + ListEx.RemoveMany(_items, _items.Where(x => x.Root == project)); + } + ErrorRefresh?.Invoke(this, project); RefreshCountToggle(); } public void Clear(IProjectRoot project, string source) { - ListEx.RemoveMany(_items, - _items.Where(x => x.Root == project && x.Source == source)); + using (_items.BeginBatch()) + { + ListEx.RemoveMany(_items, + _items.Where(x => x.Root == project && x.Source == source)); + } + ErrorRefresh?.Invoke(this, project); RefreshCountToggle(); }