From 8079ebe1dc3d96e4f2a23897071b16e26a9e505a Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 8 Sep 2020 10:49:48 +0200 Subject: [PATCH 1/4] Added failing test for #4599. --- tests/Avalonia.LeakTests/ControlTests.cs | 36 ++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/Avalonia.LeakTests/ControlTests.cs b/tests/Avalonia.LeakTests/ControlTests.cs index 0b81276240..d61813b50b 100644 --- a/tests/Avalonia.LeakTests/ControlTests.cs +++ b/tests/Avalonia.LeakTests/ControlTests.cs @@ -552,6 +552,42 @@ namespace Avalonia.LeakTests } } + [Fact] + public void ItemsRepeater_Is_Freed() + { + using (Start()) + { + var geometry = new EllipseGeometry { Rect = new Rect(0, 0, 10, 10) }; + + Func run = () => + { + var window = new Window + { + Content = new ItemsRepeater(), + }; + + window.Show(); + + window.LayoutManager.ExecuteInitialLayoutPass(); + Assert.IsType(window.Presenter.Child); + + window.Content = null; + window.LayoutManager.ExecuteLayoutPass(); + Assert.Null(window.Presenter.Child); + + return window; + }; + + var result = run(); + + dotMemory.Check(memory => + Assert.Equal(0, memory.GetObjects(where => where.Type.Is()).ObjectsCount)); + + // We are keeping geometry alive to simulate a resource that outlives the control. + GC.KeepAlive(geometry); + } + } + private IDisposable Start() { return UnitTestApplication.Start(TestServices.StyledWindow.With( From 60c0a44a8737da790d68c73f9d8036b72396aad3 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 8 Sep 2020 10:51:52 +0200 Subject: [PATCH 2/4] Clean up EffectiveViewportChanged earlier. Clean up the `EffectiveViewportChanged` subscriptions before calling `OnDetachedFromVisualTree` so that (un)subscribing to the `EffectiveViewportChanged` event in `OnDetachedFromVisualTree` doesn't cause a leak. --- src/Avalonia.Layout/Layoutable.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Layout/Layoutable.cs b/src/Avalonia.Layout/Layoutable.cs index e62e22f8ec..aca2965ea6 100644 --- a/src/Avalonia.Layout/Layoutable.cs +++ b/src/Avalonia.Layout/Layoutable.cs @@ -758,8 +758,6 @@ namespace Avalonia.Layout protected override void OnDetachedFromVisualTreeCore(VisualTreeAttachmentEventArgs e) { - base.OnDetachedFromVisualTreeCore(e); - if (e.Root is ILayoutRoot r) { if (_layoutUpdated is object) @@ -772,6 +770,8 @@ namespace Avalonia.Layout r.LayoutManager.UnregisterEffectiveViewportListener(this); } } + + base.OnDetachedFromVisualTreeCore(e); } /// From 68504d212d068cf3786eb97199359be1222d4259 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 8 Sep 2020 13:20:22 +0200 Subject: [PATCH 3/4] Use weak event handlers. Seems that C++/WinRT uses weak event handlers by default so we need to do the same here. --- .../Repeater/ItemsRepeater.cs | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Controls/Repeater/ItemsRepeater.cs b/src/Avalonia.Controls/Repeater/ItemsRepeater.cs index 8bc356bdec..772a17e4ea 100644 --- a/src/Avalonia.Controls/Repeater/ItemsRepeater.cs +++ b/src/Avalonia.Controls/Repeater/ItemsRepeater.cs @@ -7,10 +7,10 @@ using System; using System.Collections; using System.Collections.Specialized; using Avalonia.Controls.Templates; -using Avalonia.Data; using Avalonia.Input; using Avalonia.Layout; using Avalonia.Logging; +using Avalonia.Utilities; using Avalonia.VisualTree; namespace Avalonia.Controls @@ -681,8 +681,15 @@ namespace Avalonia.Controls if (oldValue != null) { oldValue.UninitializeForContext(LayoutContext); - oldValue.MeasureInvalidated -= InvalidateMeasureForLayout; - oldValue.ArrangeInvalidated -= InvalidateArrangeForLayout; + + WeakEventHandlerManager.Unsubscribe( + oldValue, + nameof(newValue.MeasureInvalidated), + InvalidateMeasureForLayout); + WeakEventHandlerManager.Unsubscribe( + oldValue, + nameof(newValue.ArrangeInvalidated), + InvalidateArrangeForLayout); // Walk through all the elements and make sure they are cleared foreach (var element in Children) @@ -699,8 +706,15 @@ namespace Avalonia.Controls if (newValue != null) { newValue.InitializeForContext(LayoutContext); - newValue.MeasureInvalidated += InvalidateMeasureForLayout; - newValue.ArrangeInvalidated += InvalidateArrangeForLayout; + + WeakEventHandlerManager.Subscribe( + newValue, + nameof(newValue.MeasureInvalidated), + InvalidateMeasureForLayout); + WeakEventHandlerManager.Subscribe( + newValue, + nameof(newValue.ArrangeInvalidated), + InvalidateArrangeForLayout); } bool isVirtualizingLayout = newValue != null && newValue is VirtualizingLayout; From e579e61f3820cd37e33258b3688cfc974477aa9c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 8 Sep 2020 13:46:25 +0200 Subject: [PATCH 4/4] Teaks and remove copy-pasta. --- src/Avalonia.Controls/Repeater/ItemsRepeater.cs | 8 ++++---- tests/Avalonia.LeakTests/ControlTests.cs | 5 ----- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/Avalonia.Controls/Repeater/ItemsRepeater.cs b/src/Avalonia.Controls/Repeater/ItemsRepeater.cs index 772a17e4ea..40f1b8dbb9 100644 --- a/src/Avalonia.Controls/Repeater/ItemsRepeater.cs +++ b/src/Avalonia.Controls/Repeater/ItemsRepeater.cs @@ -684,11 +684,11 @@ namespace Avalonia.Controls WeakEventHandlerManager.Unsubscribe( oldValue, - nameof(newValue.MeasureInvalidated), + nameof(AttachedLayout.MeasureInvalidated), InvalidateMeasureForLayout); WeakEventHandlerManager.Unsubscribe( oldValue, - nameof(newValue.ArrangeInvalidated), + nameof(AttachedLayout.ArrangeInvalidated), InvalidateArrangeForLayout); // Walk through all the elements and make sure they are cleared @@ -709,11 +709,11 @@ namespace Avalonia.Controls WeakEventHandlerManager.Subscribe( newValue, - nameof(newValue.MeasureInvalidated), + nameof(AttachedLayout.MeasureInvalidated), InvalidateMeasureForLayout); WeakEventHandlerManager.Subscribe( newValue, - nameof(newValue.ArrangeInvalidated), + nameof(AttachedLayout.ArrangeInvalidated), InvalidateArrangeForLayout); } diff --git a/tests/Avalonia.LeakTests/ControlTests.cs b/tests/Avalonia.LeakTests/ControlTests.cs index d61813b50b..0c7b966f29 100644 --- a/tests/Avalonia.LeakTests/ControlTests.cs +++ b/tests/Avalonia.LeakTests/ControlTests.cs @@ -557,8 +557,6 @@ namespace Avalonia.LeakTests { using (Start()) { - var geometry = new EllipseGeometry { Rect = new Rect(0, 0, 10, 10) }; - Func run = () => { var window = new Window @@ -582,9 +580,6 @@ namespace Avalonia.LeakTests dotMemory.Check(memory => Assert.Equal(0, memory.GetObjects(where => where.Type.Is()).ObjectsCount)); - - // We are keeping geometry alive to simulate a resource that outlives the control. - GC.KeepAlive(geometry); } }