Browse Source

Merge pull request #12526 from adirh3/fixes/context-menu-leak

Fixed memory leaks in ContextMenu.cs
Max Katz 2 years ago
parent
commit
fa6bcc9a8c
2 changed files with 65 additions and 6 deletions
  1. 25 6
      src/Avalonia.Controls/ContextMenu.cs
  2. 40 0
      tests/Avalonia.LeakTests/ControlTests.cs

+ 25 - 6
src/Avalonia.Controls/ContextMenu.cs

@@ -114,7 +114,6 @@ namespace Avalonia.Controls
         /// </summary>
         static ContextMenu()
         {
-            ItemsPanelProperty.OverrideDefaultValue<ContextMenu>(DefaultPanel);
             PlacementProperty.OverrideDefaultValue<ContextMenu>(PlacementMode.Pointer);
             ContextMenuProperty.Changed.Subscribe(ContextMenuChanged);
             AutomationProperties.AccessibilityViewProperty.OverrideDefaultValue<ContextMenu>(AccessibilityView.Control);
@@ -216,18 +215,23 @@ 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<Control>();
-                newMenu._attachedControls.Add(control);
                 control.ContextRequested += ControlContextRequested;
+                control.AttachedToVisualTree += ControlOnAttachedToVisualTree;
                 control.DetachedFromVisualTree += ControlDetachedFromVisualTree;
             }
+            
+            if (control.IsAttachedToVisualTree)
+            {
+                AttachControlToContextMenu(control); 
+            }
         }
 
         protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs change)
@@ -428,11 +432,25 @@ namespace Avalonia.Controls
                 e.Handled = true;
             }
         }
+        
+        
+        private static void ControlOnAttachedToVisualTree(object? sender, VisualTreeAttachmentEventArgs e)
+        {
+            AttachControlToContextMenu(sender);
+        }
+
+        private static void AttachControlToContextMenu(object? sender)
+        {
+            if (sender is Control { ContextMenu: { } contextMenu } control)
+            {
+                contextMenu._attachedControls ??= new List<Control>();
+                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 +458,7 @@ namespace Avalonia.Controls
                 }
 
                 contextMenu.Close();
+                contextMenu._attachedControls?.Remove(control);
             }
         }
 

+ 40 - 0
tests/Avalonia.LeakTests/ControlTests.cs

@@ -583,6 +583,46 @@ namespace Avalonia.LeakTests
                     Assert.Equal(initialMenuItemCount, memory.GetObjects(where => where.Type.Is<MenuItem>()).ObjectsCount));
             }
         }
+        
+        [Fact]
+        public void Attached_Control_From_ContextMenu_Is_Freed()
+        {
+            using (Start())
+            {
+                var contextMenu = new ContextMenu();
+                Func<Window> 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.
+                    window.LayoutManager.ExecuteInitialLayoutPass();
+                    Assert.IsType<TextBlock>(window.Presenter.Child);
+
+                    // 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<TextBlock>()).ObjectsCount));
+            }
+        }
 
         [Fact]
         public void Standalone_ContextMenu_Is_Freed()