Merge pull request #4012 from icsharpcode/fix/ilspy-tests-window-leak

Stop ILSpy.Tests from retaining every test's app graph (15 GB -> 0.7 GB)

ILSpy.Tests/MainWindow/MainMenuTests.cs

@@ -17,6 +17,7 @@
 // DEALINGS IN THE SOFTWARE.
 
 using System;
+using System.Collections.Generic;
 using System.Linq;
 
 using Avalonia;
@@ -95,6 +96,83 @@ 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");
+
+		MainMenu.PromoteHelpToMacAppMenu(
+			WindowMenuWithHelpItems("About (first window)", out var firstByTag), firstByTag);
+		var afterFirst = appMenu!.Items.Count;
+		var promoted = MainMenu.PromoteHelpToMacAppMenu(
+			WindowMenuWithHelpItems("About (second window)", out var secondByTag), secondByTag);
+		try
+		{
+			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)");
+		}
+		finally
+		{
+			RestoreAppMenu(appMenu, promoted);
+		}
+	}
+
+	// The Help items a window promotes are withdrawn when it closes, but only that window's own:
+	// a window closing after a second one has promoted its items must leave those in the app menu,
+	// or macOS shows an app menu with no About / Check for Updates while the second window is still
+	// on screen and nothing ever puts them back.
+	[AvaloniaTest]
+	public void Closing_An_Earlier_Window_Leaves_A_Later_Window_Help_Items_In_Place()
+	{
+		var appMenu = NativeMenu.GetMenu(Application.Current!);
+		appMenu.Should().NotBeNull("App.axaml declares the NativeMenu the Help items move into");
+
+		var first = MainMenu.PromoteHelpToMacAppMenu(
+			WindowMenuWithHelpItems("About (first window)", out var firstByTag), firstByTag);
+		var second = MainMenu.PromoteHelpToMacAppMenu(
+			WindowMenuWithHelpItems("About (second window)", out var secondByTag), secondByTag);
+		try
+		{
+			// What the first window's Closed handler does, now that the second window has promoted.
+			MainMenu.WithdrawHelpItems(first);
+
+			appMenu!.Items.OfType<NativeMenuItem>().Select(i => i.Header)
+				.Should().Contain("About (second window)",
+					"the still-open window's Help items must survive an earlier window closing");
+		}
+		finally
+		{
+			RestoreAppMenu(appMenu!, second);
+		}
+	}
+
+	// The app menu is declared on Application and outlives every test, so a test that promotes
+	// placeholder items into it has to take them back out; otherwise a later test reading it
+	// (see MainMenu_top_level_items_are_File_View_Window_in_order) sees this test's leftovers.
+	static void RestoreAppMenu(NativeMenu appMenu, List<NativeMenuItemBase> promoted)
+	{
+		foreach (var item in promoted)
+			appMenu.Items.Remove(item);
+	}
+
+	static NativeMenu WindowMenuWithHelpItems(string header, out Dictionary<string, NativeMenuItem> byTag)
+	{
+		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);
+		byTag = new Dictionary<string, NativeMenuItem>(StringComparer.Ordinal) { ["_Help"] = help };
+		return root;
+	}
+
 	// 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

ILSpy.Tests/ResetAppStateAttribute.cs

@@ -17,14 +17,16 @@
 // DEALINGS IN THE SOFTWARE.
 
 using System;
+using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 
 using Avalonia;
-using Avalonia.Controls.ApplicationLifetimes;
+using Avalonia.Controls;
 using Avalonia.Threading;
+using Avalonia.VisualTree;
 
 using ICSharpCode.ILSpyX.Settings;
 
@@ -92,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
@@ -100,15 +109,50 @@ public sealed class ResetAppStateAttribute : Attribute, ITestAction
 		DrainPendingWork();
 
 		// Close any windows the test showed so their view-models (alive and weakly subscribed to
-		// MessageBus) can't react to events raised by later tests, then drain once more.
-		if (Application.Current.ApplicationLifetime is IClassicDesktopStyleApplicationLifetime desktop)
+		// MessageBus) can't react to events raised by later tests, then drain once more. A window
+		// left open also outlives its container: the compositor keeps every open top level
+		// reachable, and with it the view-models, the assembly tree and the loaded assemblies -
+		// about 13 MB per test, which over the suite is what pushed the CI runner into paging.
+		foreach (var window in openWindows.ToArray())
 		{
-			foreach (var window in desktop.Windows.ToArray())
-				window.Close();
+			DetachFlyouts(window);
+			window.Close();
 		}
 		Dispatcher.UIThread.RunJobs();
 	}
 
+	// Avalonia's Button subscribes to its flyout's Opened/Closed events when its template is
+	// applied and unsubscribes only when the Flyout property changes, not when the button leaves
+	// the tree. Dock's ToolChromeControl theme gives every tool pane's chrome button the same
+	// MenuFlyout resource, so that one shared flyout would keep the visual tree of every window
+	// this suite ever showed alive. Clearing the property before the window closes is what
+	// makes the button let go.
+	static void DetachFlyouts(Window window)
+	{
+		foreach (var button in window.GetVisualDescendants().OfType<Button>())
+		{
+			if (button.Flyout != null)
+				button.Flyout = null;
+		}
+	}
+
+	// The headless host runs the app without an application lifetime, so nothing tracks the
+	// windows the tests show. These are the same class handlers ClassicDesktopStyleApplicationLifetime
+	// installs to maintain its Windows list.
+	static readonly List<Window> openWindows = new();
+
+	static ResetAppStateAttribute()
+	{
+		Window.WindowOpenedEvent.AddClassHandler(typeof(Window), (sender, _) => {
+			if (sender is Window window && !openWindows.Contains(window))
+				openWindows.Add(window);
+		});
+		Window.WindowClosedEvent.AddClassHandler(typeof(Window), (sender, _) => {
+			if (sender is Window window)
+				openWindows.Remove(window);
+		});
+	}
+
 	static void DrainPendingWork()
 	{
 		Task quiesce;

ILSpy.Tests/Search/SearchProgressTests.cs

@@ -24,6 +24,7 @@ using System.Threading.Tasks;
 using Avalonia.Controls;
 using Avalonia.Headless.NUnit;
 using Avalonia.Media;
+using Avalonia.Threading;
 
 using AwesomeAssertions;
 
@@ -115,7 +116,19 @@ public class SearchProgressTests
 		var progress = pane.FindControl<ProgressBar>("SearchProgress");
 		((object?)progress).Should().NotBeNull(
 			"the pane must host a progress indicator the user can see while a search runs");
-		progress!.IsIndeterminate.Should().BeTrue(
-			"the indicator runs in indeterminate mode — we don't know the total work up front");
+
+		// Indeterminate mode is tied to the search, not switched on permanently: the indicator
+		// is an infinite animation, and one that ran while idle would keep the render clock
+		// busy for as long as the pane exists.
+		var search = AppComposition.Current.GetExport<SearchPaneModel>();
+		pane.DataContext.Should().BeSameAs(search, "the indicator binds to the pane's own model; anything else makes the assertions below meaningless");
+		progress!.IsIndeterminate.Should().BeFalse("nothing is running yet");
+		search.IsSearching = true;
+		Dispatcher.UIThread.RunJobs();
+		progress.IsIndeterminate.Should().BeTrue(
+			"the indicator runs in indeterminate mode while a search is in flight - we don't know the total work up front");
+		search.IsSearching = false;
+		Dispatcher.UIThread.RunJobs();
+		progress.IsIndeterminate.Should().BeFalse("the animation stops with the search");
 	}
 }

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

ILSpy/Analyzers/AnalyzerRegistry.cs

@@ -29,7 +29,8 @@ namespace ICSharpCode.ILSpy.Analyzers
 	/// only resolves <c>[ImportMany]</c> with metadata through constructor injection, so this
 	/// registry is the single place that pulls the factories out of the composition host. Each
 	/// <see cref="AnalyzerEntityTreeNode"/> reads <see cref="Analyzers"/> through the static
-	/// accessor on <see cref="AnalyzerTreeNode"/>, which in turn resolves this registry once.
+	/// accessor on <see cref="AnalyzerTreeNode"/>, which resolves this shared registry from the
+	/// current composition host on each access.
 	/// </summary>
 	[Export]
 	[Shared]

ILSpy/Analyzers/AnalyzerTreeNode.cs

@@ -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
 		/// <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 };
 

ILSpy/Controls/TreeView/RichNodeText.cs

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

ILSpy/Search/SearchPane.axaml

@@ -49,9 +49,12 @@
 		</Grid>
 		<!-- Indeterminate progress strip: lights up while RunningSearch is in flight. Mirrors
 		     WPF's searchProgressBar; the height is just enough to be noticed without stealing
-		     space from the results list. -->
+		     space from the results list. IsIndeterminate follows the search too, not just
+		     IsVisible: the indeterminate indicator is an infinite animation that keeps running
+		     (and keeps the pane's visual tree alive through the render clock) for as long as
+		     the pseudo-class is set, hidden or not. -->
 		<ProgressBar Grid.Row="1" Name="SearchProgress"
-		             IsIndeterminate="True"
+		             IsIndeterminate="{Binding IsSearching}"
 		             IsVisible="{Binding IsSearching}"
 		             Height="2" Margin="0,0,0,1"
 		             BorderThickness="0"

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.

ILSpy/TextView/DecompilerTabPageModel.cs

@@ -103,6 +103,7 @@ namespace ICSharpCode.ILSpy.TextView
 		/// in the header while this is set.
 		/// </summary>
 		[ObservableProperty]
+		[NotifyPropertyChangedFor(nameof(ProgressBarIsIndeterminate))]
 		private bool isDecompiling;
 
 		/// <summary>
@@ -120,8 +121,17 @@ namespace ICSharpCode.ILSpy.TextView
 		/// off so the bar becomes determinate; an in-place decompile leaves it on.
 		/// </summary>
 		[ObservableProperty]
+		[NotifyPropertyChangedFor(nameof(ProgressBarIsIndeterminate))]
 		private bool progressIsIndeterminate = true;
 
+		/// <summary>
+		/// What the progress bar binds its IsIndeterminate to: indeterminate mode, but only while a
+		/// decompilation is running. The indeterminate indicator is an infinite animation, and it
+		/// keeps running - and keeps the view alive through the render clock - for as long as the
+		/// pseudo-class is set, whether the bar is visible or not.
+		/// </summary>
+		public bool ProgressBarIsIndeterminate => IsDecompiling && ProgressIsIndeterminate;
+
 		/// <summary>Total units to process (the project's file count) for the determinate bar.</summary>
 		[ObservableProperty]
 		private double progressMaximum;

ILSpy/TextView/DecompilerTextView.axaml

@@ -40,7 +40,7 @@
 					           Text="{Binding ProgressTitle}" />
 					<ProgressBar Name="ProgressBar"
 					             x:CompileBindings="False"
-					             IsIndeterminate="{Binding ProgressIsIndeterminate}"
+					             IsIndeterminate="{Binding ProgressBarIsIndeterminate}"
 					             Minimum="0"
 					             Maximum="{Binding ProgressMaximum}"
 					             Value="{Binding ProgressValue}"

ILSpy/ViewModels/DebugStepsPaneModel.cs

@@ -18,6 +18,7 @@
 
 #if DEBUG
 
+using System;
 using System.Collections.Generic;
 using System.ComponentModel;
 using System.Composition;
@@ -52,7 +53,7 @@ namespace ICSharpCode.ILSpy.ViewModels
 	[Export]
 	[ExportToolPane(ContentId = PaneContentId, Alignment = ToolPaneAlignment.Bottom, Order = 1, IsVisibleByDefault = false)]
 	[Shared]
-	public sealed partial class DebugStepsPaneModel : ToolPaneModel
+	public sealed partial class DebugStepsPaneModel : ToolPaneModel, IDisposable
 	{
 		public const string PaneContentId = "DebugSteps";
 
@@ -186,6 +187,21 @@ namespace ICSharpCode.ILSpy.ViewModels
 			TryAttachToCurrentLanguage();
 		}
 
+		/// <summary>
+		/// Called when the composition container that owns this pane is disposed. The writing
+		/// options are process-wide static state, so the subscription taken in the constructor
+		/// would otherwise keep this pane - and through <see cref="languageService"/> the whole
+		/// composition it belongs to - alive for as long as the process runs.
+		/// </summary>
+		public void Dispose()
+		{
+			WritingOptions.PropertyChanged -= OnWritingOptionsChanged;
+			MessageBus<AssemblyTreeSelectionChangedEventArgs>.Subscribers -= OnSelectionChanged;
+			if (languageService != null)
+				languageService.PropertyChanged -= OnLanguageServiceChanged;
+			DetachFromLanguage();
+		}
+
 		void OnLanguageServiceChanged(object? sender, PropertyChangedEventArgs e)
 		{
 			if (e.PropertyName == nameof(LanguageService.CurrentLanguage))

ILSpy/Views/MainMenu.axaml.cs

@@ -73,13 +73,17 @@ public static class MainMenu
 		if (OperatingSystem.IsMacOS())
 		{
 			TranslateGesturesForMacOS(menu);
-			PromoteHelpToMacAppMenu(menu, topLevelByTag);
+			var promoted = PromoteHelpToMacAppMenu(menu, topLevelByTag);
+			window.Closed += (_, _) => WithdrawHelpItems(promoted);
 		}
 
 		RegisterGestureKeyBindings(window, menu);
 		NativeMenu.SetMenu(window, menu);
 	}
 
+	// The Help items the most recent PromoteHelpToMacAppMenu put into the app-level NativeMenu.
+	static 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,23 +125,46 @@ 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 each window takes its own back out when it closes; leaving them in would keep
+	// that window's command graph, and everything those commands reach, alive for the life of
+	// the process. The returned list is what that window has to withdraw.
+	internal static List<NativeMenuItemBase> PromoteHelpToMacAppMenu(NativeMenu rootMenu, Dictionary<string, NativeMenuItem> topLevelByTag)
 	{
-		if (!topLevelByTag.TryGetValue("_Help", out var helpItem) || helpItem.Menu is null)
-			return;
+		WithdrawHelpItems(promotedHelpItems);
+		var promoted = new List<NativeMenuItemBase>();
 		if (Application.Current is null)
-			return;
+			return promoted;
 		var appMenu = NativeMenu.GetMenu(Application.Current);
 		if (appMenu is null)
-			return;
+			return promoted;
+		if (!topLevelByTag.TryGetValue("_Help", out var helpItem) || helpItem.Menu is null)
+			return promoted;
 		var index = 0;
 		foreach (var item in helpItem.Menu.Items.ToArray())
 		{
 			helpItem.Menu.Items.Remove(item);
 			appMenu.Items.Insert(index++, item);
+			promoted.Add(item);
 		}
 		rootMenu.Items.Remove(helpItem);
 		topLevelByTag.Remove("_Help");
+		promotedHelpItems = promoted;
+		return promoted;
+	}
+
+	// Takes exactly the listed items back out of the app-level NativeMenu -- not "whatever is
+	// promoted right now". A window that closes after a later window promoted its own Help items
+	// must leave those in place, or the app menu ends up with no About / Check for Updates while
+	// that later window is still on screen. Removing an item that is no longer there is a no-op,
+	// so withdrawing a superseded window's list is harmless.
+	internal static void WithdrawHelpItems(List<NativeMenuItemBase> items)
+	{
+		var appMenu = Application.Current is null ? null : NativeMenu.GetMenu(Application.Current);
+		foreach (var item in items)
+			appMenu?.Items.Remove(item);
+		items.Clear();
 	}
 
 	static bool TryGetExports(