Merge pull request AvaloniaUI#8189 from AvaloniaUI/fixes/8178-elementname-binding-leak

Make AvaloniaPropertyAccessorPlugin use weak events.

Author: MarchingCube

src/Avalonia.Base/Collections/Pooled/PooledList.cs

@@ -587,7 +587,7 @@ public void Clear()
             if (size > 0 && _clearOnFree)
             {
                 // Clear the elements so that the gc can reclaim the references.
-                Array.Clear(_items, 0, _size);
+                Array.Clear(_items, 0, size);
             }
         }
 

src/Avalonia.Base/Data/Core/Plugins/AvaloniaPropertyAccessorPlugin.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Runtime.ExceptionServices;
+using Avalonia.Utilities;
 
 namespace Avalonia.Data.Core.Plugins
 {
@@ -60,11 +61,10 @@ public bool Match(object obj, string propertyName)
             return AvaloniaPropertyRegistry.Instance.FindRegistered(o, propertyName);
         }
 
-        private class Accessor : PropertyAccessorBase, IObserver<object?>
+        private class Accessor : PropertyAccessorBase, IWeakEventSubscriber<AvaloniaPropertyChangedEventArgs>
         {
             private readonly WeakReference<AvaloniaObject> _reference;
             private readonly AvaloniaProperty _property;
-            private IDisposable? _subscription;
 
             public Accessor(WeakReference<AvaloniaObject> reference, AvaloniaProperty property)
             {
@@ -95,29 +95,45 @@ public override bool SetValue(object? value, BindingPriority priority)
                 return false;
             }
 
-            protected override void SubscribeCore()
+            void IWeakEventSubscriber<AvaloniaPropertyChangedEventArgs>.
+                OnEvent(object? notifyPropertyChanged, WeakEvent ev, AvaloniaPropertyChangedEventArgs e)
             {
-                _subscription = Instance?.GetObservable(_property).Subscribe(this);
+                if (e.Property == _property)
+                {
+                    SendCurrentValue();
+                }
             }
 
-            protected override void UnsubscribeCore()
+            protected override void SubscribeCore()
             {
-                _subscription?.Dispose();
-                _subscription = null;
+                SubscribeToChanges();
+                SendCurrentValue();
             }
 
-            void IObserver<object?>.OnCompleted()
+            protected override void UnsubscribeCore()
             {
+                var instance = Instance;
+
+                if (instance != null)
+                    WeakEvents.AvaloniaPropertyChanged.Unsubscribe(instance, this);
             }
 
-            void IObserver<object?>.OnError(Exception error)
+            private void SendCurrentValue()
             {
-                ExceptionDispatchInfo.Capture(error).Throw();
+                try
+                {
+                    var value = Value;
+                    PublishValue(value);
+                }
+                catch { }
             }
 
-            void IObserver<object?>.OnNext(object? value)
+            private void SubscribeToChanges()
             {
-                PublishValue(value);
+                var instance = Instance;
+
+                if (instance != null)
+                    WeakEvents.AvaloniaPropertyChanged.Subscribe(instance, this);
             }
         }
     }

src/Avalonia.Base/Utilities/WeakEvents.cs

@@ -31,10 +31,23 @@ public static readonly WeakEvent<INotifyPropertyChanged, PropertyChangedEventArg
                 return () => s.PropertyChanged -= handler;
             });
 
+
+    /// <summary>
+    /// Represents PropertyChanged event from <see cref="AvaloniaObject"/>
+    /// </summary>
+    public static readonly WeakEvent<AvaloniaObject, AvaloniaPropertyChangedEventArgs>
+        AvaloniaPropertyChanged = WeakEvent.Register<AvaloniaObject, AvaloniaPropertyChangedEventArgs>(
+            (s, h) =>
+            {
+                EventHandler<AvaloniaPropertyChangedEventArgs> handler = (_, e) => h(s, e);
+                s.PropertyChanged += handler;
+                return () => s.PropertyChanged -= handler;
+            });
+
     /// <summary>
     /// Represents CanExecuteChanged event from <see cref="ICommand"/>
     /// </summary>
     public static readonly WeakEvent<ICommand, EventArgs> CommandCanExecuteChanged =
         WeakEvent.Register<ICommand>((s, h) => s.CanExecuteChanged += h,
             (s, h) => s.CanExecuteChanged -= h);
-}
+}

tests/Avalonia.LeakTests/ControlTests.cs

@@ -1,17 +1,18 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using System.Runtime.Remoting.Contexts;
+using System.Reactive.Disposables;
 using Avalonia.Controls;
 using Avalonia.Controls.Shapes;
 using Avalonia.Controls.Templates;
+using Avalonia.Data;
 using Avalonia.Diagnostics;
 using Avalonia.Input;
-using Avalonia.Layout;
 using Avalonia.Media;
 using Avalonia.Platform;
 using Avalonia.Rendering;
 using Avalonia.Styling;
+using Avalonia.Threading;
 using Avalonia.UnitTests;
 using Avalonia.VisualTree;
 using JetBrains.dotMemoryUnit;
@@ -619,14 +620,96 @@ public void ItemsRepeater_Is_Freed()
             }
         }
 
+        [Fact]
+        public void ElementName_Binding_In_DataTemplate_Is_Freed()
+        {
+            using (Start())
+            {
+                var items = new ObservableCollection<int>(Enumerable.Range(0, 10));
+                NameScope ns;
+                TextBox tb;
+                ListBox lb;
+                var window = new Window
+                {
+                    [NameScope.NameScopeProperty] = ns = new NameScope(),
+                    Width = 100,
+                    Height = 100,
+                    Content = new StackPanel
+                    {
+                        Children =
+                        {
+                            (tb = new TextBox
+                            {
+                                Name = "tb",
+                                Text = "foo",
+                            }),
+                            (lb = new ListBox
+                            {
+                                Items = items,
+                                ItemTemplate = new FuncDataTemplate<int>((_, _) =>
+                                    new Canvas
+                                    {
+                                        Width = 10,
+                                        Height = 10,
+                                        [!Control.TagProperty] = new Binding
+                                        {
+                                            ElementName = "tb",
+                                            Path = "Text",
+                                            NameScope = new WeakReference<INameScope>(ns),
+                                        }
+                                    })
+                            }),
+                        }
+                    }
+                };
+
+                tb.RegisterInNameScope(ns);
+
+                window.Show();
+                window.LayoutManager.ExecuteInitialLayoutPass();
+
+                void AssertInitialItemState()
+                {
+                    var item0 = (ListBoxItem)lb.ItemContainerGenerator.Containers.First().ContainerControl;
+                    var canvas0 = (Canvas)item0.Presenter.Child;
+                    Assert.Equal("foo", canvas0.Tag);
+                }
+               
+                Assert.Equal(10, lb.ItemContainerGenerator.Containers.Count());
+                AssertInitialItemState();
+
+                items.Clear();
+                window.LayoutManager.ExecuteLayoutPass();
+
+                Assert.Empty(lb.ItemContainerGenerator.Containers);
+
+                dotMemory.Check(memory =>
+                    Assert.Equal(0, memory.GetObjects(where => where.Type.Is<Canvas>()).ObjectsCount));
+            }
+        }
+
         private IDisposable Start()
         {
-            return UnitTestApplication.Start(TestServices.StyledWindow.With(
-                focusManager: new FocusManager(),
-                keyboardDevice: () => new KeyboardDevice(),
-                inputManager: new InputManager()));
+            void Cleanup()
+            {
+                // KeyboardDevice holds a reference to the focused item.
+                KeyboardDevice.Instance.SetFocusedElement(null, NavigationMethod.Unspecified, KeyModifiers.None);
+                
+                // Empty the dispatcher queue.
+                Dispatcher.UIThread.RunJobs();
+            }
+
+            return new CompositeDisposable
+            {
+                Disposable.Create(Cleanup),
+                UnitTestApplication.Start(TestServices.StyledWindow.With(
+                    focusManager: new FocusManager(),
+                    keyboardDevice: () => new KeyboardDevice(),
+                    inputManager: new InputManager()))
+            };
         }
 
+
         private class Node
         {
             public string Name { get; set; }