From 50d6dbc07cd319042024734ae612e3cba70c1747 Mon Sep 17 00:00:00 2001 From: Julien Lebosquain Date: Thu, 26 Feb 2026 10:59:43 +0000 Subject: [PATCH] Fix menu memory leak (#20753) * Add test for menu memory leak * Fix menu memory leak --- src/Avalonia.Controls/Menu.cs | 18 +++++--- .../ButtonTests.cs | 3 +- .../TabControlTests.cs | 3 +- tests/Avalonia.LeakTests/ControlTests.cs | 41 ++++++++++++++++++- tests/Avalonia.UnitTests/TestServices.cs | 6 +-- .../Avalonia.UnitTests/UnitTestApplication.cs | 3 +- 6 files changed, 59 insertions(+), 15 deletions(-) diff --git a/src/Avalonia.Controls/Menu.cs b/src/Avalonia.Controls/Menu.cs index c1c869e41a..9cb572722a 100644 --- a/src/Avalonia.Controls/Menu.cs +++ b/src/Avalonia.Controls/Menu.cs @@ -13,6 +13,8 @@ namespace Avalonia.Controls /// public class Menu : MenuBase, IMainMenu { + private IAccessKeyHandler? _accessKeyHandler; + private static readonly FuncTemplate DefaultPanel = new (() => new StackPanel { Orientation = Orientation.Horizontal }); @@ -88,12 +90,18 @@ namespace Avalonia.Controls { base.OnAttachedToVisualTree(e); - var inputRoot = TopLevel.GetTopLevel(this); + _accessKeyHandler = TopLevel.GetTopLevel(this)?.AccessKeyHandler; + _accessKeyHandler?.MainMenu = this; + } - if (inputRoot?.AccessKeyHandler != null) - { - inputRoot.AccessKeyHandler.MainMenu = this; - } + protected override void OnDetachedFromVisualTree(VisualTreeAttachmentEventArgs e) + { + if (_accessKeyHandler?.MainMenu == this) + _accessKeyHandler.MainMenu = null; + + _accessKeyHandler = null; + + base.OnDetachedFromVisualTree(e); } protected internal override void PrepareContainerForItemOverride(Control element, object? item, int index) diff --git a/tests/Avalonia.Controls.UnitTests/ButtonTests.cs b/tests/Avalonia.Controls.UnitTests/ButtonTests.cs index 20065bac60..0ca18d5585 100644 --- a/tests/Avalonia.Controls.UnitTests/ButtonTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ButtonTests.cs @@ -305,11 +305,10 @@ namespace Avalonia.Controls.UnitTests public void Raises_Click_When_AccessKey_Raised() { var raised = 0; - var ah = new AccessKeyHandler(); var kd = new KeyboardDevice(); using var app = UnitTestApplication.Start(TestServices.StyledWindow .With( - accessKeyHandler: ah, + accessKeyHandler: () => new AccessKeyHandler(), keyboardDevice: () => kd) ); diff --git a/tests/Avalonia.Controls.UnitTests/TabControlTests.cs b/tests/Avalonia.Controls.UnitTests/TabControlTests.cs index 0e171474fe..fd255f2e7f 100644 --- a/tests/Avalonia.Controls.UnitTests/TabControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/TabControlTests.cs @@ -723,11 +723,10 @@ namespace Avalonia.Controls.UnitTests [InlineData(Key.D, "d", 0)] public void Should_TabControl_Recognizes_AccessKey(Key accessKey, string accessKeySymbol, int selectedTabIndex) { - var ah = new AccessKeyHandler(); var kd = new KeyboardDevice(); using (UnitTestApplication.Start(TestServices.StyledWindow .With( - accessKeyHandler: ah, + accessKeyHandler: () => new AccessKeyHandler(), keyboardDevice: () => kd) )) { diff --git a/tests/Avalonia.LeakTests/ControlTests.cs b/tests/Avalonia.LeakTests/ControlTests.cs index 11197888a5..33edc0d00d 100644 --- a/tests/Avalonia.LeakTests/ControlTests.cs +++ b/tests/Avalonia.LeakTests/ControlTests.cs @@ -1047,6 +1047,44 @@ namespace Avalonia.LeakTests } } + [ReleaseFact] + public void Menu_Is_Freed() + { + using (Start()) + { + var window = new Window(); + + WeakReference Run() + { + var menu = new Menu(); + window.Content = menu; + + window.Show(); + + // Do a layout and make sure that Menu gets added to visual tree + window.LayoutManager.ExecuteInitialLayoutPass(); + Assert.IsType(window.Presenter!.Child); + Assert.NotEmpty(window.Presenter.Child.GetVisualChildren()); + + // Clear the content and ensure the Menu is removed. + window.Content = null; + window.LayoutManager.ExecuteLayoutPass(); + Assert.Null(window.Presenter.Child); + + return new WeakReference(menu); + } + + var weakMenu = Run(); + Assert.True(weakMenu.IsAlive); + + CollectGarbage(); + + Assert.False(weakMenu.IsAlive); + + GC.KeepAlive(window); + } + } + private static FuncControlTemplate CreateWindowTemplate() { return new FuncControlTemplate((parent, scope) => @@ -1075,7 +1113,8 @@ namespace Avalonia.LeakTests Disposable.Create(Cleanup), UnitTestApplication.Start(TestServices.StyledWindow.With( keyboardDevice: () => new KeyboardDevice(), - inputManager: new InputManager())) + inputManager: new InputManager(), + accessKeyHandler: () => new AccessKeyHandler())) }; } diff --git a/tests/Avalonia.UnitTests/TestServices.cs b/tests/Avalonia.UnitTests/TestServices.cs index b7e4eef1b9..baf86a538f 100644 --- a/tests/Avalonia.UnitTests/TestServices.cs +++ b/tests/Avalonia.UnitTests/TestServices.cs @@ -87,7 +87,7 @@ namespace Avalonia.UnitTests ITextShaperImpl? textShaperImpl = null, IWindowImpl? windowImpl = null, IWindowingPlatform? windowingPlatform = null, - IAccessKeyHandler? accessKeyHandler = null) + Func? accessKeyHandler = null) { AssetLoader = assetLoader; InputManager = inputManager; @@ -110,7 +110,7 @@ namespace Avalonia.UnitTests public IAssetLoader? AssetLoader { get; } public IInputManager? InputManager { get; } internal IGlobalClock? GlobalClock { get; set; } - internal IAccessKeyHandler? AccessKeyHandler { get; } + internal Func? AccessKeyHandler { get; } public Func? KeyboardDevice { get; } internal Func? KeyboardNavigation { get; } public Func? MouseDevice { get; } @@ -128,7 +128,7 @@ namespace Avalonia.UnitTests IAssetLoader? assetLoader = null, IInputManager? inputManager = null, IGlobalClock? globalClock = null, - IAccessKeyHandler? accessKeyHandler = null, + Func? accessKeyHandler = null, Func? keyboardDevice = null, Func? keyboardNavigation = null, Func? mouseDevice = null, diff --git a/tests/Avalonia.UnitTests/UnitTestApplication.cs b/tests/Avalonia.UnitTests/UnitTestApplication.cs index 5c13b8d593..4d266aa809 100644 --- a/tests/Avalonia.UnitTests/UnitTestApplication.cs +++ b/tests/Avalonia.UnitTests/UnitTestApplication.cs @@ -89,8 +89,7 @@ namespace Avalonia.UnitTests .Bind().ToConstant(Services.WindowingPlatform) .Bind().ToSingleton() .Bind().ToSingleton() - .Bind().ToConstant(Services.AccessKeyHandler) - ; + .Bind().ToFunc(Services.AccessKeyHandler ?? (() => null)); // This is a hack to make tests work, we need to refactor the way font manager is registered // See https://github.com/AvaloniaUI/Avalonia/issues/10081