diff --git a/ILSpy.Tests/MainWindow/MainMenuTests.cs b/ILSpy.Tests/MainWindow/MainMenuTests.cs index 009516885..50ce4f43c 100644 --- a/ILSpy.Tests/MainWindow/MainMenuTests.cs +++ b/ILSpy.Tests/MainWindow/MainMenuTests.cs @@ -95,6 +95,38 @@ public class MainMenuTests openItem.Gesture!.Should().Be(expected); } + // The app-level NativeMenu (App.axaml) is process-wide, while every MainWindow builds + // its own Help items over its own command instances. On macOS each new window promotes + // them into that app menu; the ones an earlier window promoted must be replaced, not + // kept - otherwise the app menu pins every earlier window's command graph (and, in the + // headless suite, every test's app graph) for the life of the process. + [AvaloniaTest] + public void Promoting_Help_Again_Replaces_The_Items_An_Earlier_Window_Promoted() + { + var appMenu = NativeMenu.GetMenu(Application.Current!); + appMenu.Should().NotBeNull("App.axaml declares the NativeMenu the Help items move into"); + + static (NativeMenu Root, System.Collections.Generic.Dictionary ByTag) WindowMenuWithHelp(string header) + { + var help = new NativeMenuItem { Header = "_Help", Menu = new NativeMenu() }; + help.Menu.Items.Add(new NativeMenuItem { Header = header }); + var root = new NativeMenu(); + root.Items.Add(help); + return (root, new System.Collections.Generic.Dictionary(StringComparer.Ordinal) { ["_Help"] = help }); + } + + var first = WindowMenuWithHelp("About (first window)"); + MainMenu.PromoteHelpToMacAppMenu(first.Root, first.ByTag); + var afterFirst = appMenu!.Items.Count; + var second = WindowMenuWithHelp("About (second window)"); + MainMenu.PromoteHelpToMacAppMenu(second.Root, second.ByTag); + + appMenu.Items.Count.Should().Be(afterFirst, "the second window's Help items replace the first window's"); + appMenu.Items.OfType().Select(i => i.Header) + .Should().Contain("About (second window)") + .And.NotContain("About (first window)"); + } + // NativeMenuItem.Gesture is display-only when NativeMenuBar renders the menu inline // (the managed fallback binds it to MenuItem.InputGesture, which never handles input), // so every menu gesture must also be registered as a window-level KeyBinding or the diff --git a/ILSpy.Tests/ResetAppStateAttribute.cs b/ILSpy.Tests/ResetAppStateAttribute.cs index c4d3ebc73..de957195a 100644 --- a/ILSpy.Tests/ResetAppStateAttribute.cs +++ b/ILSpy.Tests/ResetAppStateAttribute.cs @@ -94,6 +94,13 @@ public sealed class ResetAppStateAttribute : Attribute, ITestAction if (Application.Current == null || !Dispatcher.UIThread.CheckAccess()) return; + TearDownTestState(); + } + + // Everything the per-test teardown does on the dispatcher thread; exposed so a test can + // perform the teardown itself and check what it leaves behind (see TeardownRetentionTests). + internal static void TearDownTestState() + { // Drive background work to quiescence BEFORE the next test rebuilds the composition. A test // that triggers a decompile spawns a Task.Run plus dispatcher continuations and rarely awaits // them to completion; left running, that continuation lands during the next test and reads diff --git a/ILSpy.Tests/TeardownRetentionTests.cs b/ILSpy.Tests/TeardownRetentionTests.cs new file mode 100644 index 000000000..d273afb5c --- /dev/null +++ b/ILSpy.Tests/TeardownRetentionTests.cs @@ -0,0 +1,91 @@ +// Copyright (c) 2026 Christoph Wille +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Threading.Tasks; + +using Avalonia.Headless; +using Avalonia.Headless.NUnit; +using Avalonia.Threading; + +using ICSharpCode.ILSpy.AppEnv; +using ICSharpCode.ILSpy.AssemblyTree; +using ICSharpCode.ILSpy.Views; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests; + +// Most tests in this suite show a MainWindow, and the per-test teardown closes it and rebuilds +// the composition container. Anything that still reaches a closed window - a static event, a +// shared XAML resource with a subscriber, an animation on the render clock, the app-level menu - +// keeps that test's whole app graph (view-models, tree, loaded assemblies; about 13 MB) alive +// for the rest of the run, and over the suite that is enough to push a 16 GB CI runner into +// paging. Rather than asserting the absence of each known anchor, this test performs the +// teardown itself and checks that the window is actually collectable afterwards. +[TestFixture] +public class TeardownRetentionTests +{ + [AvaloniaTest] + public async Task A_Main_Window_Closed_By_The_Teardown_Is_Collectable() + { + var window = ShowMainWindow(); + // Let the assembly loads the window started run to completion first: each one posts its + // completion to the dispatcher, and one posted after the teardown would hold the tree (and + // with it the window) until the next test pumps it - a false positive, not retention. + await Waiters.WaitForAsync(static () => AllAssembliesLoaded()); + + ResetAppStateAttribute.TearDownTestState(); + // What the next test's BeforeTest does: the fresh container drops the [Shared] MainWindow. + AppComposition.CreateContainer(); + + // The closed window's final composition batch (its target's disposal) references it until + // the compositor has committed and rendered it, and commits are throttled behind the + // previous batch's completion, which comes back through the thread pool - so keep pumping + // the dispatcher (and the headless render loop, which only ticks on request) while polling. + await Waiters.WaitForAsync(() => IsCollected(window), TimeSpan.FromSeconds(10), + "the closed MainWindow to become unreachable once its container is gone"); + } + + static bool IsCollected(WeakReference window) + { + AvaloniaHeadlessPlatform.ForceRenderTimerTick(); + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + return !window.IsAlive; + } + + static bool AllAssembliesLoaded() + { + var assemblies = AppComposition.Current.GetExport().AssemblyList?.GetAssemblies(); + return assemblies is { Length: > 0 } && assemblies.All(a => a.IsLoaded); + } + + // The window must not be referenced from this test's own frame while the GC runs. + [MethodImpl(MethodImplOptions.NoInlining)] + static WeakReference ShowMainWindow() + { + var window = AppComposition.Current.GetExport(); + window.Show(); + Dispatcher.UIThread.RunJobs(); + return new WeakReference(window); + } +} diff --git a/ILSpy/Analyzers/AnalyzerTreeNode.cs b/ILSpy/Analyzers/AnalyzerTreeNode.cs index 7c30fb6a2..c6093196d 100644 --- a/ILSpy/Analyzers/AnalyzerTreeNode.cs +++ b/ILSpy/Analyzers/AnalyzerTreeNode.cs @@ -37,23 +37,24 @@ namespace ICSharpCode.ILSpy.Analyzers /// public abstract class AnalyzerTreeNode : SharpTreeNode { - static LanguageService? cachedLanguageService; - static AnalyzerRegistry? cachedRegistry; - static AssemblyTreeModel? cachedAssemblyTreeModel; + // The exports below are resolved on every access rather than cached in statics: the + // composition root is rebuilt per test in the headless suite, and a static cache would + // hand later tests the first test's language service and assembly list and keep that + // first app graph reachable for the run. A warm GetExport is a dictionary lookup. /// /// The active language used to format entity text. Resolved lazily through the /// composition host so design-time previews (no MEF) don't NRE during XAML reload. /// protected static Languages.Language Language - => (cachedLanguageService ??= AppComposition.Current.GetExport()).CurrentLanguage; + => AppComposition.Current.GetExport().CurrentLanguage; /// /// The active backing the assembly tree. Search nodes pass /// it into AnalyzerContext so each analyser can iterate the loaded modules. /// protected static AssemblyList? CurrentAssemblyList - => (cachedAssemblyTreeModel ??= AppComposition.Current.GetExport()).AssemblyList; + => AppComposition.Current.GetExport().AssemblyList; /// /// All MEF-registered exports, ordered by their declared @@ -63,7 +64,7 @@ namespace ICSharpCode.ILSpy.Analyzers /// returns true for the wrapped entity. /// public static IReadOnlyList> Analyzers - => (cachedRegistry ??= AppComposition.Current.GetExport()).Analyzers; + => AppComposition.Current.GetExport().Analyzers; public override bool CanDelete() => Parent is { IsRoot: true }; diff --git a/ILSpy/Controls/TreeView/RichNodeText.cs b/ILSpy/Controls/TreeView/RichNodeText.cs index 83bfd75d7..26cab85c9 100644 --- a/ILSpy/Controls/TreeView/RichNodeText.cs +++ b/ILSpy/Controls/TreeView/RichNodeText.cs @@ -58,9 +58,11 @@ namespace ICSharpCode.ILSpy.Controls.TreeView static readonly AttachedProperty CleanupHookedProperty = AvaloniaProperty.RegisterAttached("CleanupHooked", typeof(RichNodeText)); - static LanguageSettings? languageSettings; + // Resolved on every call rather than cached: the composition root is rebuilt per test in the + // headless suite, and a static cache would both subscribe later windows to a stale settings + // object and keep the first window's tree reachable through it. A warm export lookup is cheap. static LanguageSettings? GetLanguageSettings() - => languageSettings ??= AppComposition.TryGetExport()?.SessionSettings.LanguageSettings; + => AppComposition.TryGetExport()?.SessionSettings.LanguageSettings; static RichNodeText() { diff --git a/ILSpy/Search/SearchPaneModel.cs b/ILSpy/Search/SearchPaneModel.cs index 56aca2131..5e96f5d30 100644 --- a/ILSpy/Search/SearchPaneModel.cs +++ b/ILSpy/Search/SearchPaneModel.cs @@ -53,7 +53,7 @@ namespace ICSharpCode.ILSpy.Search [Export] [ExportToolPane(ContentId = PaneContentId, Alignment = ToolPaneAlignment.Top, Order = 0, IsVisibleByDefault = false)] [Shared] - public partial class SearchPaneModel : ToolPaneModel + public sealed partial class SearchPaneModel : ToolPaneModel, IDisposable { public const string PaneContentId = "Search"; @@ -265,6 +265,16 @@ namespace ICSharpCode.ILSpy.Search run.Start(); } + // The composition container is the only owner and disposes this model with itself. A search + // still in flight at that point owns a dispatcher timer and keeps IsSearching (and with it the + // pane's indeterminate progress animation) on; both would otherwise outlive the container. + public void Dispose() + { + currentSearch?.Cancel(); + currentSearch = null; + IsSearching = false; + } + void OnRunCompleted(RunningSearch sender) { // Ignore late completions from cancelled runs — those are noise. diff --git a/ILSpy/Views/MainMenu.axaml.cs b/ILSpy/Views/MainMenu.axaml.cs index 1c6786c0b..68aeca559 100644 --- a/ILSpy/Views/MainMenu.axaml.cs +++ b/ILSpy/Views/MainMenu.axaml.cs @@ -74,12 +74,16 @@ public static class MainMenu { TranslateGesturesForMacOS(menu); PromoteHelpToMacAppMenu(menu, topLevelByTag); + window.Closed += (_, _) => WithdrawPromotedHelpItems(); } RegisterGestureKeyBindings(window, menu); NativeMenu.SetMenu(window, menu); } + // The Help items currently promoted into the app-level NativeMenu (see PromoteHelpToMacAppMenu). + static readonly List promotedHelpItems = new(); + // NativeMenuItem.Gesture is display-only when NativeMenuBar renders the menu inline: // the managed fallback binds it to MenuItem.InputGesture, which never handles input. // So each gesture is registered as a window-level KeyBinding too - that is what makes @@ -121,25 +125,40 @@ public static class MainMenu // the exporter subscribes to that instance's Items, so inserting fires a re-export. // Items go at the top, above the Services / Hide / Quit block the exporter appended. // We then remove _Help from the window menu so the items don't appear in both places. - static void PromoteHelpToMacAppMenu(NativeMenu rootMenu, Dictionary topLevelByTag) + // The app menu outlives any one window, and each window's Help items are built over + // that window's own command instances, so the items an earlier window promoted are + // taken out first (and again when the window closes); leaving them in would keep + // that window's command graph, and everything those commands reach, alive for the + // life of the process. + internal static void PromoteHelpToMacAppMenu(NativeMenu rootMenu, Dictionary topLevelByTag) { - if (!topLevelByTag.TryGetValue("_Help", out var helpItem) || helpItem.Menu is null) - return; + WithdrawPromotedHelpItems(); if (Application.Current is null) return; var appMenu = NativeMenu.GetMenu(Application.Current); if (appMenu is null) return; + if (!topLevelByTag.TryGetValue("_Help", out var helpItem) || helpItem.Menu is null) + return; var index = 0; foreach (var item in helpItem.Menu.Items.ToArray()) { helpItem.Menu.Items.Remove(item); appMenu.Items.Insert(index++, item); + promotedHelpItems.Add(item); } rootMenu.Items.Remove(helpItem); topLevelByTag.Remove("_Help"); } + static void WithdrawPromotedHelpItems() + { + var appMenu = Application.Current is null ? null : NativeMenu.GetMenu(Application.Current); + foreach (var item in promotedHelpItems) + appMenu?.Items.Remove(item); + promotedHelpItems.Clear(); + } + static bool TryGetExports( out SettingsService settings, out MainMenuCommandRegistry registry,