Fix menu memory leak (#20753)

* Add test for menu memory leak

* Fix menu memory leak

src/Avalonia.Controls/Menu.cs

@@ -13,6 +13,8 @@ namespace Avalonia.Controls
     /// </summary>
     public class Menu : MenuBase, IMainMenu
     {
+        private IAccessKeyHandler? _accessKeyHandler;
+
         private static readonly FuncTemplate<Panel?> 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)

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)
             );
 

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)
                    ))
             {

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<Menu>(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<Window>((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()))
             };
         }
 

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<IAccessKeyHandler?>? 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<IAccessKeyHandler?>? AccessKeyHandler { get; }
         public Func<IKeyboardDevice?>? KeyboardDevice { get; }
         internal Func<IKeyboardNavigationHandler?>? KeyboardNavigation { get; }
         public Func<IMouseDevice?>? MouseDevice { get; }
@@ -128,7 +128,7 @@ namespace Avalonia.UnitTests
             IAssetLoader? assetLoader = null,
             IInputManager? inputManager = null,
             IGlobalClock? globalClock = null,
-            IAccessKeyHandler? accessKeyHandler = null,
+            Func<IAccessKeyHandler?>? accessKeyHandler = null,
             Func<IKeyboardDevice?>? keyboardDevice = null,
             Func<IKeyboardNavigationHandler?>? keyboardNavigation = null,
             Func<IMouseDevice?>? mouseDevice = null,

tests/Avalonia.UnitTests/UnitTestApplication.cs

@@ -89,8 +89,7 @@ namespace Avalonia.UnitTests
                 .Bind<IWindowingPlatform?>().ToConstant(Services.WindowingPlatform)
                 .Bind<PlatformHotkeyConfiguration>().ToSingleton<PlatformHotkeyConfiguration>()
                 .Bind<IPlatformSettings>().ToSingleton<DefaultPlatformSettings>()
-                .Bind<IAccessKeyHandler?>().ToConstant(Services.AccessKeyHandler)
-                ;
+                .Bind<IAccessKeyHandler?>().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