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]

Two robustness gaps in this loop.

Nothing closes an open menu before the window dies. DetachFlyouts covers Button.Flyout, but a ContextMenu left open by a test keeps DefaultMenuInteractionHandler subscribed to the process-global InputManager.Instance.Process, and that handler holds Menu.TopLevel - i.e. this window and everything under it - for the rest of the run. Several tests here open a tree context menu and never dismiss it. Closing menus in the same pass is a one-line extension and removes a retention root of the same size as the flyout one:

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

No error containment. If any Closing/Closed handler throws for the first window (MainWindow.OnClosing guards its own SaveLayout, but view OnDetachedFromVisualTree handlers are not guarded), the foreach aborts: every remaining window stays open and stays in the static openWindows list for the whole run - the exact retention this PR removes - and the exception surfaces as a teardown error on a test that otherwise passed. Wrap the body in try/catch and openWindows.Remove(window) unconditionally so the tracking list can never become the thing that retains a window.

[christophwille]

None of the four leak fixes has a regression test, which is at odds with the repo's TDD rule for new behaviour. Concretely: if someone later drops IDisposable from DebugStepsPaneModel, reverts this tracking, or restores IsIndeterminate="True" in the XAML, the suite stays green and the runner quietly goes back to paging - the failure mode is an intermittent 60s timeout in a different project, which is the hardest possible signal to trace back here.

A single cheap guard covers most of it: hold a WeakReference to the container's MainWindow (and to the DebugStepsPaneModel) at the end of a test, let the next BeforeTest rebuild the container, force a full GC, and assert the reference is dead. DecompilerViewTests already uses the GC.Collect()/WaitForPendingFinalizers() pattern, so there is precedent.

[christophwille]

This assertion is now vacuous in the failure case it is supposed to catch. ProgressBar.IsIndeterminate defaults to false, so if the pane's DataContext has not been set to this SearchPaneModel yet (or is a different instance than the one resolved from composition), line 124 passes for the wrong reason and line 127 fails with "the indicator runs in indeterminate mode..." - which points at the binding direction rather than at the missing DataContext.

Add pane.DataContext.Should().BeSameAs(search) before line 124, so the test fails where the real problem is. The single RunJobs() on line 126 is also load-bearing now where the old assertion read a static XAML value; a short Waiters.WaitForAsync(() => progress.IsIndeterminate) would take the timing out of it entirely.

[christophwille]

Confirmed this is actually wired: System.Composition.TypedParts.ActivationFeatures.DisposalFeature.RewriteActivator registers every IDisposable part via LifetimeContext.AddBoundInstance, and LifetimeContext.Dispose walks that list - so AppComposition.CreateContainer()'s current?.Dispose() does reach this. No change needed.

One note for a follow-up: Dispose() is not idempotent-safe against re-use. DebugStepsPaneModel is a [Shared] singleton that ShowToolPane can re-surface, so if anything ever starts disposing dockables on pane close (Dock does not today - I checked Dock.Model/Dock.Avalonia for IDisposable usage), closing and reopening the pane would leave it permanently dead: unsubscribed from the language service, the message bus and the writing options, with nothing that re-subscribes. A one-line disposed guard plus re-subscription in TryAttachToCurrentLanguage would make that safe, or a comment stating the container is the only owner.

[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.