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

[christophwille]

Problem

ILSpy.Tests (the headless Avalonia suite) retains the whole app object graph of every test it runs: private bytes grow linearly by ~13 MB per test to ~15 GB by the end of the 1180-test run (measured locally and, via a temporary sampler, on the windows-2025 runner, whose 16 GB box then pages out every idle process). That is what has been failing the Windows CI jobs intermittently: the process-explorer tests in ILSpy.Tests and ILSpy.Tests.Windows scan every process's module list, and on a paged-out runner that walk hard-faults through ~150 processes and blows past its 60 s budget (100–400 s measured; 13–16 min when the decompiler suite added server GC on top).

Anchors found (ClrMD gcroot on dumps of the running suite)

1. Windows were never closed. The headless host runs without an ApplicationLifetime, so ResetAppState's window-closing loop had no list to iterate. Fixed by tracking Window.WindowOpenedEvent/WindowClosedEvent (the same class handlers the desktop lifetime uses) and closing after each test.
2. Shared MenuFlyout resource in Dock's ToolChromeControl theme. Avalonia's Button subscribes Opened/Closed on its flyout and only unsubscribes when the Flyout property changes, never on detach; every tool pane's chrome button shares one MenuFlyout, which therefore pinned every closed window's visual tree. The harness detaches the flyouts before closing.
3. DebugStepsPaneModel subscribing to the static WritingOptions.PropertyChanged without ever unsubscribing (Debug builds only) - retains each container's pane and, via its LanguageService, the rest of the container. Now IDisposable; System.Composition disposes it with the container.
4. Infinite indeterminate progress-bar animations: the search pane's bar was permanently IsIndeterminate="True" (only hidden when idle) and the decompiler view's bar defaults to indeterminate whether or not a decompile runs. The indicator is an infinite keyframe animation that keeps running - and keeps the visual tree alive through the render clock - as long as the pseudo-class is set, visible or not. Both now go indeterminate only while work is in progress (3 and 4 are also small runtime wins for the app itself).

Result

Full local run of ILSpy.Tests: peak private bytes 705 MB (was 15.8 GB), 6m36 (was 10m30), all tests green (one test asserting the old always-indeterminate binding updated).

🤖 Generated with Claude Code

[christophwille]

How the retention chains were found (heap analysis on the .NET 11 preview runtime)

Notes for the next person who has to do this, because the stock tooling does not work yet.

What does not work: dotnet-dump analyze (9.0.661903, the newest on NuGet) refuses dumps of the .NET 11 preview runtime with "The CLR debugging layer reported a version of 10 which this build of ClrMD does not support", so dumpheap/gcroot are unavailable.

What works:

1. Capture still works: dotnet-dump collect -p <pid> -o suite.dmp while the test host is running (mid-run, after enough tests to make the leak obvious). dotnet-gcdump collect -p <pid> works as well.

2. Analyse the full dump with ClrMD directly. Microsoft.Diagnostics.Runtime 4.0.73x (the current package, newer than the copy bundled in dotnet-dump) reads the dump fine. A scratch console app is enough:

   using var dt = DataTarget.LoadDump(path);
   using var runtime = dt.ClrVersions[0].CreateRuntime();
   var heap = runtime.Heap;
   var targets = heap.EnumerateObjects().Where(o => o.Type?.Name == "ICSharpCode.ILSpy.Views.MainWindow").ToList();
   var gcroot = new GCRoot(heap, targets.Take(1).Select(o => o.Address).ToList());
   foreach (var (root, path) in gcroot.EnumerateRootPaths())
       for (var link = path; link != null; link = link.Next)
           Console.WriteLine(heap.GetObject(link.Object).Type?.Name);

   EnumerateObjects() by type name gives you counts (345 MainWindows alive after ~350 tests was the smoking gun); GCRoot gives real root paths, including through dependent handles / weak-event tables. Reading fields (ReadObjectField, AsArray().GetStructValue(i)) answers the follow-up questions - e.g. dumping the Dictionary<object,object> entries of a ResourceDictionary on the path is how the shared MenuFlyout turned out to be Dock's ToolChromeControlContextMenu.

3. The .gcdump route is usable but weaker: dotnet-gcdump report only lists large objects, but the file contains the reference graph, and the reader lives inside the tool itself. Reference dotnet-gcdump.dll (plus Microsoft.Diagnostics.FastSerialization.dll / Microsoft.Diagnostics.Tracing.TraceEvent.dll from ~/.dotnet/tools/.store/dotnet-gcdump/<ver>/.../tools/net8.0/any/) from a scratch app and use new GCHeapDump(path).MemoryGraph + new SpanningTree(graph, Console.Out); tree.Parent(nodeIndex) walks to the root. Caveat: anything reached through a ConditionalWeakTable shows up under a [Dependent Handles] pseudo-root without naming the primary, so paths ending there are inconclusive - that is what pushed this investigation to ClrMD.

4. Attribute growth to tests without any tooling: sample Get-Process ILSpy.Tests | % PrivateMemorySize64 every 10 s and correlate with the trx startTimes; a per-fixture delta table shows immediately whether it is a few heavy tests or (as here) a fixed cost per test.

5. Distinguish managed retention from lazy GC cheaply: rerun the suite with DOTNET_GCHeapHardLimit=0x80000000 (2 GB). If private bytes still climb while the GC heap stays under the limit (here 0.8 GB heap vs 4.6 GB private after two minutes), the objects are rooted and the bulk is native - in this case the PEReader prefetched images behind every LoadedAssembly.

Typical anchors to check first once you have a root path: statics with events (WritingOptions.PropertyChanged), shared XAML resources with event subscribers (the Dock flyout), infinite animations subscribed to the render clock (ProgressBar indeterminate), and windows that are closed but never actually torn down.

[christophwille]

Review: correctness + verification of the memory claim

I reviewed the diff, reproduced the suite locally, and measured the memory claim independently. Summary: the four anchors are real and the direction is right, but the headline number does not reproduce off Windows, and there is at least one more retention root of exactly the same shape that the harness still leaves behind (an open ContextMenu) - which is also my best explanation for the Desktop (Windows) (Debug) failure.

1. Independent memory measurement (macOS 15.6, arm64, Debug, ILSpy.Tests, same 1181 tests)

Sampled dotnet-counters (System.Runtime) across full runs of both branches:

	master (0879e35)	this PR (c0a74fc)
gen2 heap after last GC, start -> end	210 MB -> 6406 MB	174 MB -> 5076 MB
gen2 fragmentation at end	1 MB	835 MB
live gen2 at end (heap - frag)	~4.5 GB	~4.0 GB
peak working set	10.7 GB	8.9 GB
gen2 collections	10	10
wall clock	4m34s	3m52s

Both runs green (1181 passed / 3 skipped). The wall-clock win reproduces (~15%). The memory win does not: retained gen2 still grows monotonically and near-linearly across the run on this branch, ending at ~4 GB live. That is roughly a 10-20% improvement, not 15.8 GB -> 0.7 GB.

I can't tell from here whether the remaining growth is Windows-vs-macOS or whether the Windows measurement was optimistic, but as it stands the PR description's "peak private bytes 705 MB" should not be treated as verified, and the hunt shouldn't be declared finished. Raw data and the sampling scripts are reproducible with dotnet-counters collect -p <pid> --counters System.Runtime around a plain ./ILSpy.Tests run.

2. The anchor the harness still misses: an open ContextMenu roots the whole window

This is the same bug class as anchor 2 (shared MenuFlyout), and it is arguably a bigger root. From Avalonia.Controls.Platform.DefaultMenuInteractionHandler.AttachCore (decompiled with this repo's ilspycmd):

_root = Menu.TopLevel;
_root?.AddHandler(InputElement.PointerPressedEvent, RootPointerPressed, RoutingStrategies.Tunnel);
if (_root is WindowBase windowBase) windowBase.Deactivated += WindowDeactivated;
_inputManagerSubscription = InputManager?.Process.Subscribe(RawInput);

InputManager.Instance is process-global. While a menu is open, the chain InputManager.Instance -> subscription -> DefaultMenuInteractionHandler -> Menu -> Menu.TopLevel pins the entire window, its visual tree, its view-models and its assemblies - exactly the ~13 MB/test shape you measured. DetachCore (which disposes that subscription) only runs on MenuBase.Close(). Destroying the window out from under an open menu does not call it.

Several tests open a tree context menu and never dismiss it (DecompileInNewViewTests.Right_Clicking_An_Unselected_Row_Does_Not_Change_The_Selection is one), so AfterTest closes those windows with the menu still open. Suggested extension of DetachFlyouts - one pass, both roots:

foreach (var control in window.GetVisualDescendants().OfType<Control>())
{
    control.ContextMenu?.Close();
    if (control is Button { Flyout: not null } button)
        button.Flyout = null;
}

3. Windows (Debug) CI failure

DecompileInNewViewTests.Right_Clicking_A_Second_Row_Moves_The_Context_Highlight_To_It failed with Expected Row(nodeC).Classes {empty} to contain "contextTarget". Only Desktop (Windows) (Debug) failed; Release, Linux and macOS passed, and I could not reproduce it in two full local runs of this branch.

The class is only ever missing if AssemblyListPane.OnTreeContextRequested did not run for that click, or OnContextMenuOpening cleared it because BuildContextMenuForCurrentState returned null. The test has two unguarded legs that produce the first case:

• Dismiss() pumps a fixed 4 x 20ms after Escape and never checks that the menu actually closed. If it is still open, the light-dismiss layer eats the next right-press and the tree never sees ContextRequested - precisely the observed symptom.
• RightClick() computes the point from row.Bounds with no hit-test check. This is the exact fragility that a134c8b ("Aim the tree-gesture pointer tests at a hit-testable row") had to fix in HeadlessMmbPointerTests a few hours earlier; DecompileInNewViewTests still has the old computation.

Proposal, independent of whether this PR caused it: hoist HeadlessMmbPointerTests.TryGetRowClickPoint into a shared helper and use it here, and make Dismiss()/RightClick() wait on Tree.ContextMenu.IsOpen instead of on a fixed delay. Also fold the menu state into the assertion message so the next CI failure explains itself instead of printing {empty}. Fixing (2) above removes the stale-menu leg as well, which is why I think the two are related.

4. Smaller items

Inline comments on the diff. Nothing blocking beyond the above.

Verified while reviewing (no action needed): System.Composition does dispose [Shared] IDisposable parts on CompositionHost.Dispose() (DisposalFeature.RewriteActivator -> LifetimeContext.AddBoundInstance -> LifetimeContext.Dispose), so anchor 3 works; and Window.HandleClosed does raise WindowClosedEvent, so openWindows does drain on a normal close.

[christophwille]

Follow-up on the review: the macOS number was real, and it had a cause of its own

Thanks for measuring - the discrepancy was not a measurement error on either side. There is a fifth anchor that only exists on macOS, which is why the win reproduced on Windows (15.8 GB -> 0.7 GB) and not on your machine.

The macOS-only anchor

MainMenu.PromoteHelpToMacAppMenu (guarded by OperatingSystem.IsMacOS()) moves every MainWindow's Help items into the process-wide NativeMenu declared on the Application in App.axaml. It never removed the previous window's items, and nothing removed them on close. Each item's AboutCommand holds that window's DockWorkspace, and through it the whole app graph - the same ~13 MB per test as the four Windows-side anchors, which matches the ~4 GB of retained gen2 you saw. Verified by forcing the macOS branch on Windows: 14.3 GB peak private bytes over the suite on the previous head; with the items withdrawn on the next promotion and on Window.Closed: 0.7 GB. Fixed in 7f0afb1, with a test that promotes twice and asserts the first window's items are gone. (One caveat on the metric: dotnet.gc.last_collection.heap.size{gen2} includes uncollected garbage between gen2 GCs, so it overstates live gen2 somewhat - but the near-linear growth you described was real.)

The open ContextMenu

Not confirmed as a root. I closed a window with the tree's context menu still open and rooted it from a dump: no path through DefaultMenuInteractionHandler / InputManager. TopLevel.HandleClosed detaches the logical tree, the menu's Popup closes on TargetDetached, and MenuBase.OnDetachedFromVisualTree runs InteractionHandler.Detach. What did keep that window alive was AvaloniaEdit's static RoutedCommand._inputElement (last focused element - bounded to the most recently focused window) and, in Debug, the async test frame's hoisted locals. So the harness is unchanged there; DetachFlyouts stays because Button really does keep its flyout subscription until the property changes.

Regression test

Adopted. TeardownRetentionTests closes a MainWindow the way the per-test teardown does (ResetAppStateAttribute.TearDownTestState, now exposed), rebuilds the container and waits for the window to become collectable. It goes red if any single anchor is put back (checked by leaving DetachFlyouts out). Making it hold inside the full suite surfaced three more anchors of the same family, all fixed in the same commit:

• RichNodeText and AnalyzerTreeNode cached the first container's exports in statics - which also meant later tests' analyzers ran against the first test's assembly list, and the first app graph stayed reachable for the run.
• A search still in flight when its container went away kept its drain DispatcherTimer and IsSearching == true (so the pane's indeterminate progress animation kept running on the render clock); about 30 search-test windows, ~400 MB, were retained that way. SearchPaneModel is now IDisposable, cancelling the run and clearing the flag when the container disposes it.

Three transient roots had to be accounted for so the canary does not false-positive: assembly-load completions posted to the dispatcher after teardown, the compositor's pending batch for the closed target (commits are throttled behind the previous batch's completion, which returns via the thread pool, and the headless render loop only ticks on request), and the Debug async frame - hence a Waiters.WaitForAsync poll with GC + ForceRenderTimerTick in the predicate.

Smaller items

• pane.DataContext.Should().BeSameAs(search) in the progress test: adopted (2881d8c).
• try/catch around the close loop: left out on purpose - openWindows is static, so a window whose Close throws is retried at every following teardown and the exception surfaces each time; nothing stays silently retained.
• DebugStepsPaneModel.Dispose idempotency: agreed, no change.
• Right_Clicking_A_Second_Row_Moves_The_Context_Highlight_To_It on Windows Debug: fixed-delay pointer test, ran clean repeatedly here; nothing ties it to this branch on the evidence so far. Making Dismiss()/RightClick() wait on ContextMenu.IsOpen is a reasonable separate hardening.

Full local suite after all of this: 1186 tests green, 5m59, peak private bytes 691 MB.

[christophwille]

Re-review after 7f0afb1 / 2881d8c: the macOS number reproduces, and two of my findings were wrong

I re-measured and re-derived everything from scratch on macOS 15.6 / arm64 / Debug. Two corrections to my earlier review, then two small findings which I have pushed fixes for.

Correction 1: the memory win does reproduce. My measurement predated the fix.

Full ILSpy.Tests suite, same machine, /usr/bin/time -l:

	master	c0a74fcd1 (what I measured)	HEAD 2881d8c01
peak RSS	10.7 GB	8.9 GB	1.78 GB
wall clock	4m34s	3m52s	2m59s

My "the headline number does not reproduce on macOS" was measuring the branch before 7f0afb147 existed - i.e. with the macOS-only anchor still in it. That was exactly the gap your follow-up identified. Withdrawn.

The mechanism holds up independently: AboutCommand holds DockWorkspace (ILSpy/Commands/AboutCommand.cs:55), it is [Shared] in the per-test container, and its NativeMenuItem was being parked in the process-wide Application.Current menu. Sensitivity check: commenting out both WithdrawPromotedHelpItems() call sites makes TeardownRetentionTests fail with the 10s timeout; restored, 1186/1186 green. The canary is genuinely load-bearing for this anchor, not just for DetachFlyouts.

Correction 2: you are right about the open ContextMenu; I was wrong.

MenuBase.OnDetachedFromVisualTree calls InteractionHandler.Detach(this) (Avalonia.Controls 12.1.1), so DetachCore - which disposes _inputManagerSubscription - runs when the window closes, not only via MenuBase.Close(). My claim that only Close() reaches it was simply wrong. No harness change needed; your dump result and the code agree.

7f0afb147 is correct Avalonia-wise on macOS

Verified against decompiled Avalonia 12.1.1 rather than assumed:

• __MicroComIAvnMenuItemProxy.Update calls _subMenu.Initialize(exporter, item.Menu, ...), which subscribes to the App.axaml NativeMenu.Items.CollectionChanged -> QueueReset -> DoLayoutReset -> Update, whose tail loop RemoveAndDisposes surplus proxies. So removals re-export exactly like insertions - the comment in the file is accurate for both directions, and the native side really does let go of the withdrawn items.
• NativeMenu's list validator throws when an item already has a parent, and ItemsChanged nulls Parent on removal. The withdraw / re-promote cycle is therefore legal and cannot throw.
• PopulateStandardOSXMenuItems appends the Services / Hide / Quit block into that same App.axaml instance, on first export only. Inserting at index 0 keeps Help above it, and the tracked list only ever holds our own items, so withdrawal never touches Avalonia's block.

Two findings, both fixed in eefef96

1. MainMenu.axaml.cs:77 - the Closed handler withdrew whatever was promoted at that moment, not what that window promoted. Not reachable today (MainWindow is [Shared], one per container, and Attach only runs from its ctor), but with two windows, closing the older one strips the newer one's About / Check for Updates out of the app menu permanently, since nothing re-promotes. PromoteHelpToMacAppMenu now returns the list it promoted and each window withdraws exactly that; removing an item that is already gone is a no-op, so a superseded window's close stays harmless. Red-checked: Closing_An_Earlier_Window_Leaves_A_Later_Window_Help_Items_In_Place fails against the old behaviour with {empty} to contain "About (second window)".

2. MainMenuTests.cs:104 - the new test left the process-wide app menu dirty. It is not macOS-gated and never restored, so "About (second window)" stayed in Application.Current's NativeMenu for the rest of the run. Benign only by alphabetical luck: MainMenu_top_level_items_are_File_View_Window_in_order, the one test that reads the app menu on macOS, sorts before Promoting_..., and any test that builds a MainWindow re-promotes anyway. On Windows and Linux nothing ever cleaned it up. Both promotion tests now restore the app menu in a finally.

Smaller items

• try/catch in the close loop: your rationale covers "silently retained", but not the other half - the foreach at ResetAppStateAttribute.cs:116 aborts, so every window after the throwing one is left open too, and because the thrower stays first in openWindows each later teardown aborts at the same point. Low severity and your call; noting it only so the trade-off is on the record.
• Windows Debug pointer test: CI is green on 2881d8c01 across every leg including Desktop (Windows) (Debug). Nothing further ties it to this branch. The ContextMenu.IsOpen wait remains reasonable separate hardening.
• No change needed: RichNodeText.Detach (line 131) now resolves GetLanguageSettings() fresh, so the unsubscribe only matches the subscribe while the same container is current. That holds today, because teardown closes windows before BeforeTest calls CreateContainer - just an ordering dependency the static cache did not have.

Full suite after eefef96e4: 1187 tests, 0 failures, 3m00s, peak RSS 1.36 GB.

[christophwille]

Follow-up: the AnalyzerTreeNode / RichNodeText static-cache removal (7f0afb1)

Commit 7f0afb1 replaced the ??= export caches in AnalyzerTreeNode (Language, CurrentAssemblyList, Analyzers) and RichNodeText.GetLanguageSettings with a GetExport on every access. That is a runtime change made for the test harness, so here is what actually depends on it and the options for restoring the caching, so any of them can be picked up later.

What the statics reach

Static	Read by	What a stale cache pins
AnalyzerTreeNode.Language	every Analyzed*TreeNode.Text, AnalyzerEntityTreeNode.CreateRichText, AnalyzerSearchTreeNode.RunAnalyzer	LanguageService -> its PropertyChanged subscribers (AssemblyTreeModel, DockWorkspace) -> the whole app graph
AnalyzerTreeNode.CurrentAssemblyList	AnalyzerSearchTreeNode.RunAnalyzer	AssemblyTreeModel -> tree + loaded assemblies
AnalyzerTreeNode.Analyzers	AnalyzerEntityTreeNode.LoadChildren, 4 tests directly	AnalyzerRegistry (harmless)
RichNodeText.GetLanguageSettings	OnNodeChanged / Detach for every tree row whose node is IRichTextNode - AssemblyTreeNode is one, so every shown assembly tree	LanguageSettings -> Parent SessionSettings, plus AssemblyListPane's never-unsubscribed handler (AssemblyListPane.axaml.cs:77) -> the window graph

Tests that depend on them

• Direct (AnalyzerTreeNode.Analyzers): AnalyzerConstructorUsesTests, AnalyzerPaneCopyResultsTests, AnalyzerSearchTreeNodeTests, AnalyzerTreeNodeTests.
• Indirect via analyzer nodes: all 15 files in ILSpy.Tests/Analyzers/*.cs (35 [AvaloniaTest]s) - anything that materializes an Analyzed*TreeNode, expands an entity node, or runs a search node.
• Indirect via RichNodeText: every test that shows a MainWindow / assembly tree - 153 of 236 test files (TestHarness.BootAsync / GetExport<MainWindow>).
• Not impacted: Analyzers/Library/* (23 tests; they build AnalyzerContext themselves), DockWorkspaceTests, ToolPaneRegistryTests, ILSpyXEventSourceTests.

Which tests change outcome (measured, caches restored in the working tree)

Variant	Full ILSpy.Tests (1183)	TeardownRetentionTests alone
both caches restored	all pass, canary included (ran at position 1036)	fails
only RichNodeText cache restored	-	fails
only AnalyzerTreeNode caches restored	-	passes

So no test's correctness depends on the removal. The caches pin exactly one app graph (the first test's, ~13 MB) for the run - bounded, not per-test growth. The canary trips only when its own window is the first to populate RichNodeText's static (a filtered run), and never for the AnalyzerTreeNode statics (a bare MainWindow does not touch them). The claim in the 7f0afb1 message that the canary "fails on any single anchor being restored" does not hold for these two hunks; the memory win in that commit is the macOS menu and the search drain timer. One latent hazard does remain with the AnalyzerTreeNode cache: later analyzer tests would search test #1's AssemblyList - harmless today only because every BootAsync loads the same list.

Options for keeping the runtime caching

1. Reinstate ??= and invalidate on container rebuild (my pick). AppComposition.CreateContainer already disposes the previous host; add public static event Action? ContainerReplaced; fired there, and have the static ctors of AnalyzerTreeNode / RichNodeText subscribe a static lambda that nulls the fields. Runtime path is the old one, the invalidation is a correct invariant of CreateContainer on its own (an export cached from a disposed host is stale) rather than a test hook, and the canary passes in every ordering because CreateContainer runs before the collectability check. ~8 lines.
2. Test-only reset: internal static void ResetExportCache() on both classes (InternalsVisibleTo already exists), called from ResetAppStateAttribute.TearDownTestState (must be there, not BeforeTest, since the canary calls TearDownTestState + CreateContainer directly). No new runtime API, but the harness carries a whitelist of statics that future caches must be added to - and as measured, the canary will not catch an unlisted AnalyzerTreeNode-style one.
3. Revert only the AnalyzerTreeNode hunk, keep the RichNodeText change. Smallest diff (no new code), green in every ordering - but keeps the stale-AssemblyList hazard for a future analyzer test that opens its own assembly.
4. Container-keyed cache (ReferenceEquals(cachedHost, AppComposition.Current)): no hooks, but the stale graph is only released on the next access, so the canary alone still fails for RichNodeText. Not worth it.
5. Keep HEAD as is. GetExport on a [Shared] part is a contract-dictionary lookup plus lifetime lock - what the other 51 AppComposition.Current.GetExport / TryGetExport call sites in ILSpy/ already do per access, and analyzer rows cache their RichText anyway. If kept, AnalyzerRegistry's doc comment ("resolves this registry once") is stale and needs a touch.
6. Instance-scoped (inject through the analyzer root / VM): correct by construction but touches every Analyzed*TreeNode constructor and the tests that new them. Not worth the diff.