From 22c262bfe4ec32f7c1709f0c4459ad3369fe01ae Mon Sep 17 00:00:00 2001 From: Giuseppe Lippolis Date: Wed, 24 Apr 2024 12:58:17 +0200 Subject: [PATCH] fix: RoutedCommand CanExecuteChanged Memory Leak --- .../ViewModels/RouteCommandViewModel.cs | 4 + .../RoutedCommand.cs | 19 +++-- .../Utilities/WeakCollection.cs | 79 +++++++++++++++++++ 3 files changed, 96 insertions(+), 6 deletions(-) create mode 100644 src/Avalonia.Labs.CommandManager/Utilities/WeakCollection.cs diff --git a/samples/Avalonia.Labs.Catalog/ViewModels/RouteCommandViewModel.cs b/samples/Avalonia.Labs.Catalog/ViewModels/RouteCommandViewModel.cs index 7d1c112..9c4a32b 100644 --- a/samples/Avalonia.Labs.Catalog/ViewModels/RouteCommandViewModel.cs +++ b/samples/Avalonia.Labs.Catalog/ViewModels/RouteCommandViewModel.cs @@ -4,6 +4,7 @@ using System.Threading.Tasks; using Avalonia.Controls; using Avalonia.Labs.Catalog.Views; +using Avalonia.Labs.Input; using Avalonia.Platform.Storage; namespace Avalonia.Labs.Catalog.ViewModels; @@ -42,6 +43,7 @@ public async void Open(object? parameter) item.Text = file.Name; } } + CommandManager.InvalidateRequerySuggested(); } } @@ -53,6 +55,7 @@ public async void Save(object? parameter) if (parameter is RouteCommandItemViewModel { HasChanges: true } item) { item.Accept(); + CommandManager.InvalidateRequerySuggested(); } await Task.CompletedTask; } @@ -66,6 +69,7 @@ public void Delete(object? parameter) { Dettails.Remove(item); item.Accept(); + CommandManager.InvalidateRequerySuggested(); } } diff --git a/src/Avalonia.Labs.CommandManager/RoutedCommand.cs b/src/Avalonia.Labs.CommandManager/RoutedCommand.cs index b4d7511..e9e3aec 100644 --- a/src/Avalonia.Labs.CommandManager/RoutedCommand.cs +++ b/src/Avalonia.Labs.CommandManager/RoutedCommand.cs @@ -11,7 +11,11 @@ namespace Avalonia.Labs.Input; /// public class RoutedCommand : ICommand { - private EventHandler? _canExecuteChanged; + /// + /// The weak collection of delegates for . + /// + private readonly WeakCollection _canExecuteChanged = new(); + private RoutedCommandRequeryHandler? _handler; private IList? _gestures; @@ -63,8 +67,7 @@ event EventHandler? ICommand.CanExecuteChanged { add { - _canExecuteChanged += value; - + _canExecuteChanged.Add(value!); if (_handler is null) { _handler ??= new RoutedCommandRequeryHandler(this); @@ -73,8 +76,7 @@ event EventHandler? ICommand.CanExecuteChanged } remove { - _canExecuteChanged -= value; - + _canExecuteChanged.Remove(value!); if (_handler is not null && _canExecuteChanged is null) { CommandManager.PrivateRequerySuggestedEvent.Unsubscribe(CommandManager.Current, _handler); @@ -155,6 +157,11 @@ internal bool ExecuteCore(object? parameter, IInputElement? target) private class RoutedCommandRequeryHandler(RoutedCommand command) : IWeakEventSubscriber { - public void OnEvent(object? sender, WeakEvent ev, EventArgs e) => command._canExecuteChanged?.Invoke(sender, e); + public void OnEvent(object? sender, WeakEvent ev, EventArgs e) + { + var list = command._canExecuteChanged.GetLiveItems(); + foreach (var canExecuteChanged in list) + canExecuteChanged(sender, EventArgs.Empty); + } } } diff --git a/src/Avalonia.Labs.CommandManager/Utilities/WeakCollection.cs b/src/Avalonia.Labs.CommandManager/Utilities/WeakCollection.cs new file mode 100644 index 0000000..912f68b --- /dev/null +++ b/src/Avalonia.Labs.CommandManager/Utilities/WeakCollection.cs @@ -0,0 +1,79 @@ +// Based on: https://github.com/StephenCleary/Mvvm.Core/blob/cbac65eda98760cc0dabd0b82a6468be0e1dd44f/src/Nito.Mvvm.Core/WeakCollection.cs +using System; +using System.Collections.Generic; + +namespace Avalonia.Utilities; + +/// +/// A collection of weak references to objects. +/// +/// The type of object to hold weak references to. +internal sealed class WeakCollection where T : class +{ + /// + /// The actual collection of strongly-typed weak references. + /// + private readonly List> _list = []; + + /// + /// Gets a list of live objects from this collection, causing a purge. + /// + /// + public IReadOnlyList GetLiveItems() + { + var ret = new List(_list.Count); + + // This implementation uses logic similar to List.RemoveAll, which always has O(n) time. + // Some other implementations seen in the wild have O(n*m) time, where m is the number of dead entries. + // As m approaches n (e.g., mass object extinctions), their running time approaches O(n^2). + int writeIndex = 0; + for (int readIndex = 0; readIndex != _list.Count; ++readIndex) + { + WeakReference weakReference = _list[readIndex]; + T? item; + if (weakReference.TryGetTarget(out item)) + { + ret.Add(item); + + if (readIndex != writeIndex) + _list[writeIndex] = _list[readIndex]; + + ++writeIndex; + } + } + + _list.RemoveRange(writeIndex, _list.Count - writeIndex); + + return ret; + } + + /// + /// Adds a weak reference to an object to the collection. Does not cause a purge. + /// + /// The object to add a weak reference to. + public void Add(T item) + { + _list.Add(new WeakReference(item)); + } + + /// + /// Removes a weak reference to an object from the collection. Does not cause a purge. + /// + /// The object to remove a weak reference to. + /// True if the object was found and removed; false if the object was not found. + public bool Remove(T item) + { + for (int i = 0; i != _list.Count; ++i) + { + var weakReference = _list[i]; + T? entry; + if (weakReference.TryGetTarget(out entry) && entry == item) + { + _list.RemoveAt(i); + return true; + } + } + + return false; + } +}