Browse Source

Withdraw a window's Help items from the macOS app menu, and add a retention canary

The app-level NativeMenu declared in App.axaml lives as long as the
process, while every MainWindow builds its own Help items over its own
command instances (AboutCommand reaches the DockWorkspace and, through
it, the whole app graph). PromoteHelpToMacAppMenu inserted each window's
items without taking the previous window's out and nothing removed them
on close, so on macOS the headless suite kept every test's app graph
alive - the same 13 MB per test as the anchors fixed earlier on this
branch, and the reason the memory win did not reproduce on macOS
(retained gen2 still climbing to ~4 GB there while a Windows run peaks
at 0.7 GB). Forcing the macOS path on Windows reproduces the growth
(14.3 GB peak private bytes over the suite); withdrawn, it is 0.7 GB.

Three smaller anchors of the same kind, found while making the canary
below hold in the full suite: RichNodeText and AnalyzerTreeNode cached
the first container's exports in statics, which subscribed later
windows to a stale settings object, handed later analyzers the first
test's assembly list, and kept the first app graph reachable for the
run; and a search still in flight when its container went away kept its
drain timer and IsSearching - hence the pane's indeterminate progress
animation on the render clock - alive, retaining every window a search
test closed mid-run (about 30 of them, ~400 MB).

The canary test closes a MainWindow the way the per-test teardown does
and waits for it to become collectable. It fails on any single anchor
being restored (checked by leaving DetachFlyouts out), which is the
regression guard the individual anchor fixes lacked; the teardown body
is exposed as TearDownTestState so the test performs exactly what
AfterTest does.

Assisted-by: Claude:claude-fable-5:Claude Code
pull/4012/head
Christoph Wille 1 month ago committed by Siegfried Pammer
parent
commit
84a7839a29
  1. 32
      ILSpy.Tests/MainWindow/MainMenuTests.cs
  2. 7
      ILSpy.Tests/ResetAppStateAttribute.cs
  3. 91
      ILSpy.Tests/TeardownRetentionTests.cs
  4. 13
      ILSpy/Analyzers/AnalyzerTreeNode.cs
  5. 6
      ILSpy/Controls/TreeView/RichNodeText.cs
  6. 12
      ILSpy/Search/SearchPaneModel.cs
  7. 25
      ILSpy/Views/MainMenu.axaml.cs

32
ILSpy.Tests/MainWindow/MainMenuTests.cs

@ -95,6 +95,38 @@ public class MainMenuTests @@ -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<string, NativeMenuItem> 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<string, NativeMenuItem>(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<NativeMenuItem>().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

7
ILSpy.Tests/ResetAppStateAttribute.cs

@ -94,6 +94,13 @@ public sealed class ResetAppStateAttribute : Attribute, ITestAction @@ -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

91
ILSpy.Tests/TeardownRetentionTests.cs

@ -0,0 +1,91 @@ @@ -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<AssemblyTreeModel>().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<MainWindow>();
window.Show();
Dispatcher.UIThread.RunJobs();
return new WeakReference(window);
}
}

13
ILSpy/Analyzers/AnalyzerTreeNode.cs

@ -37,23 +37,24 @@ namespace ICSharpCode.ILSpy.Analyzers @@ -37,23 +37,24 @@ namespace ICSharpCode.ILSpy.Analyzers
/// </summary>
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.
/// <summary>
/// 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.
/// </summary>
protected static Languages.Language Language
=> (cachedLanguageService ??= AppComposition.Current.GetExport<LanguageService>()).CurrentLanguage;
=> AppComposition.Current.GetExport<LanguageService>().CurrentLanguage;
/// <summary>
/// The active <see cref="AssemblyList"/> backing the assembly tree. Search nodes pass
/// it into <c>AnalyzerContext</c> so each analyser can iterate the loaded modules.
/// </summary>
protected static AssemblyList? CurrentAssemblyList
=> (cachedAssemblyTreeModel ??= AppComposition.Current.GetExport<AssemblyTreeModel>()).AssemblyList;
=> AppComposition.Current.GetExport<AssemblyTreeModel>().AssemblyList;
/// <summary>
/// All MEF-registered <see cref="IAnalyzer"/> exports, ordered by their declared
@ -63,7 +64,7 @@ namespace ICSharpCode.ILSpy.Analyzers @@ -63,7 +64,7 @@ namespace ICSharpCode.ILSpy.Analyzers
/// <see cref="IAnalyzer.Show"/> returns true for the wrapped entity.
/// </summary>
public static IReadOnlyList<ExportFactory<IAnalyzer, AnalyzerMetadata>> Analyzers
=> (cachedRegistry ??= AppComposition.Current.GetExport<AnalyzerRegistry>()).Analyzers;
=> AppComposition.Current.GetExport<AnalyzerRegistry>().Analyzers;
public override bool CanDelete() => Parent is { IsRoot: true };

6
ILSpy/Controls/TreeView/RichNodeText.cs

@ -58,9 +58,11 @@ namespace ICSharpCode.ILSpy.Controls.TreeView @@ -58,9 +58,11 @@ namespace ICSharpCode.ILSpy.Controls.TreeView
static readonly AttachedProperty<bool> CleanupHookedProperty =
AvaloniaProperty.RegisterAttached<TextBlock, bool>("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<SettingsService>()?.SessionSettings.LanguageSettings;
=> AppComposition.TryGetExport<SettingsService>()?.SessionSettings.LanguageSettings;
static RichNodeText()
{

12
ILSpy/Search/SearchPaneModel.cs

@ -53,7 +53,7 @@ namespace ICSharpCode.ILSpy.Search @@ -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 @@ -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.

25
ILSpy/Views/MainMenu.axaml.cs

@ -74,12 +74,16 @@ public static class MainMenu @@ -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<NativeMenuItemBase> 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 @@ -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<string, NativeMenuItem> 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<string, NativeMenuItem> 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,

Loading…
Cancel
Save