From 11971a0431bdda95581c8f4914c164f87e8acab1 Mon Sep 17 00:00:00 2001 From: adirh Date: Sat, 12 Aug 2023 16:42:50 +0300 Subject: [PATCH 1/5] Fixed memory leaks in ContextMenu.cs --- src/Avalonia.Controls/ContextMenu.cs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/Avalonia.Controls/ContextMenu.cs b/src/Avalonia.Controls/ContextMenu.cs index e3a419d7b3..7fab36d931 100644 --- a/src/Avalonia.Controls/ContextMenu.cs +++ b/src/Avalonia.Controls/ContextMenu.cs @@ -114,7 +114,6 @@ namespace Avalonia.Controls /// static ContextMenu() { - ItemsPanelProperty.OverrideDefaultValue(DefaultPanel); PlacementProperty.OverrideDefaultValue(PlacementMode.Pointer); ContextMenuProperty.Changed.Subscribe(ContextMenuChanged); AutomationProperties.AccessibilityViewProperty.OverrideDefaultValue(AccessibilityView.Control); @@ -216,16 +215,16 @@ namespace Avalonia.Controls if (e.OldValue is ContextMenu oldMenu) { control.ContextRequested -= ControlContextRequested; + control.AttachedToVisualTree -= ControlOnAttachedToVisualTree; control.DetachedFromVisualTree -= ControlDetachedFromVisualTree; oldMenu._attachedControls?.Remove(control); ((ISetLogicalParent?)oldMenu._popup)?.SetParent(null); } - if (e.NewValue is ContextMenu newMenu) + if (e.NewValue is ContextMenu) { - newMenu._attachedControls ??= new List(); - newMenu._attachedControls.Add(control); control.ContextRequested += ControlContextRequested; + control.AttachedToVisualTree += ControlOnAttachedToVisualTree; control.DetachedFromVisualTree += ControlDetachedFromVisualTree; } } @@ -428,11 +427,20 @@ namespace Avalonia.Controls e.Handled = true; } } + + + private static void ControlOnAttachedToVisualTree(object? sender, VisualTreeAttachmentEventArgs e) + { + if (sender is Control { ContextMenu: {} contextMenu } control) + { + contextMenu._attachedControls ??= new List(); + contextMenu._attachedControls.Add(control); + } + } private static void ControlDetachedFromVisualTree(object? sender, VisualTreeAttachmentEventArgs e) { - if (sender is Control control - && control.ContextMenu is ContextMenu contextMenu) + if (sender is Control { ContextMenu: { } contextMenu } control) { if (contextMenu._popup?.Parent == control) { @@ -440,6 +448,7 @@ namespace Avalonia.Controls } contextMenu.Close(); + contextMenu._attachedControls?.Remove(control); } } From 5f7d823b59c96473d048f5c66ab098c36bd055bb Mon Sep 17 00:00:00 2001 From: adirh Date: Sat, 12 Aug 2023 17:26:02 +0300 Subject: [PATCH 2/5] Added unit test --- tests/Avalonia.LeakTests/ControlTests.cs | 42 ++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/Avalonia.LeakTests/ControlTests.cs b/tests/Avalonia.LeakTests/ControlTests.cs index ab8e4377b6..9a4bcb1808 100644 --- a/tests/Avalonia.LeakTests/ControlTests.cs +++ b/tests/Avalonia.LeakTests/ControlTests.cs @@ -583,6 +583,48 @@ namespace Avalonia.LeakTests Assert.Equal(initialMenuItemCount, memory.GetObjects(where => where.Type.Is()).ObjectsCount)); } } + + [Fact] + public void Attached_Control_From_ContextMenu_Is_Freed() + { + using (Start()) + { + var contextMenu = new ContextMenu(); + Func run = () => + { + var window = new Window + { + Content = new TextBlock + { + ContextMenu = contextMenu + } + }; + + window.Show(); + + // Do a layout and make sure that TextBlock gets added to visual tree with + // its render transform. + window.LayoutManager.ExecuteInitialLayoutPass(); + var textBlock = Assert.IsType(window.Presenter.Child); + Assert.IsType(textBlock.RenderTransform); + + // Clear the content and ensure the TextBlock is removed. + window.Content = null; + window.LayoutManager.ExecuteLayoutPass(); + Assert.Null(window.Presenter.Child); + + return window; + }; + + var result = run(); + + // Process all Loaded events to free control reference(s) + Dispatcher.UIThread.RunJobs(DispatcherPriority.Loaded); + + dotMemory.Check(memory => + Assert.Equal(0, memory.GetObjects(where => where.Type.Is()).ObjectsCount)); + } + } [Fact] public void Standalone_ContextMenu_Is_Freed() From 1d484685370d7ddb60259b74f1f24630a975f17b Mon Sep 17 00:00:00 2001 From: adirh Date: Sat, 12 Aug 2023 17:30:51 +0300 Subject: [PATCH 3/5] Added check for when control already attached to visual tree --- src/Avalonia.Controls/ContextMenu.cs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/ContextMenu.cs b/src/Avalonia.Controls/ContextMenu.cs index 7fab36d931..6300239e12 100644 --- a/src/Avalonia.Controls/ContextMenu.cs +++ b/src/Avalonia.Controls/ContextMenu.cs @@ -227,6 +227,11 @@ namespace Avalonia.Controls control.AttachedToVisualTree += ControlOnAttachedToVisualTree; control.DetachedFromVisualTree += ControlDetachedFromVisualTree; } + + if (control.IsAttachedToVisualTree) + { + AttachControlToContextMenu(control); + } } protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs change) @@ -431,7 +436,12 @@ namespace Avalonia.Controls private static void ControlOnAttachedToVisualTree(object? sender, VisualTreeAttachmentEventArgs e) { - if (sender is Control { ContextMenu: {} contextMenu } control) + AttachControlToContextMenu(sender); + } + + private static void AttachControlToContextMenu(object? sender) + { + if (sender is Control { ContextMenu: { } contextMenu } control) { contextMenu._attachedControls ??= new List(); contextMenu._attachedControls.Add(control); From 038c96375037bc9fe5e72382c5697bac73aa18f2 Mon Sep 17 00:00:00 2001 From: adirh Date: Sat, 12 Aug 2023 21:39:45 +0300 Subject: [PATCH 4/5] Fixed unit test --- tests/Avalonia.LeakTests/ControlTests.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/Avalonia.LeakTests/ControlTests.cs b/tests/Avalonia.LeakTests/ControlTests.cs index 9a4bcb1808..c6f9c5eae0 100644 --- a/tests/Avalonia.LeakTests/ControlTests.cs +++ b/tests/Avalonia.LeakTests/ControlTests.cs @@ -602,11 +602,9 @@ namespace Avalonia.LeakTests window.Show(); - // Do a layout and make sure that TextBlock gets added to visual tree with - // its render transform. + // Do a layout and make sure that TextBlock gets added to visual tree. window.LayoutManager.ExecuteInitialLayoutPass(); - var textBlock = Assert.IsType(window.Presenter.Child); - Assert.IsType(textBlock.RenderTransform); + Assert.IsType(window.Presenter.Child); // Clear the content and ensure the TextBlock is removed. window.Content = null; From 74a5e13d2e20bf3d31238e050c4d5775c7c5b137 Mon Sep 17 00:00:00 2001 From: adirh Date: Tue, 15 Aug 2023 21:23:42 +0300 Subject: [PATCH 5/5] Trigger devops build again --- src/Avalonia.Controls/ContextMenu.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/ContextMenu.cs b/src/Avalonia.Controls/ContextMenu.cs index 6300239e12..78f6c5f77d 100644 --- a/src/Avalonia.Controls/ContextMenu.cs +++ b/src/Avalonia.Controls/ContextMenu.cs @@ -230,7 +230,7 @@ namespace Avalonia.Controls if (control.IsAttachedToVisualTree) { - AttachControlToContextMenu(control); + AttachControlToContextMenu(control); } }