From bb114b27a73d248d6639cba5b008ec6bbf0555c4 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 23 May 2026 19:24:02 +0200 Subject: [PATCH] Prevent FailFast when an assembly is unloaded mid-decompile When the user removes an assembly while a decompile is still in flight, the title-refresh paths (spinner tick, post-Task.Run InvokeAsync, StopSpinner) walk node.Text -> LoadedAssembly.Text -> metadata.GetAssemblyDefinition().Version, reading directly from the PE file's MemoryMappedFile. AssemblyList.Unload calls Dispose synchronously and unmaps that file; any continuation that lands on the UI thread after Dispose dereferences freed pages and the CLR reports the AV through FailFastIfCorruptingStateException -- a catch block cannot intervene in .NET 5+. Assisted-by: Claude:claude-opus-4-7:Claude Code --- ICSharpCode.ILSpyX/LoadedAssembly.cs | 36 +++++++++++++++---- ILSpy/Docking/DockWorkspace.cs | 45 ++++++++++++++++++------ ILSpy/TextView/DecompilerTabPageModel.cs | 24 ++++++++----- 3 files changed, 79 insertions(+), 26 deletions(-) diff --git a/ICSharpCode.ILSpyX/LoadedAssembly.cs b/ICSharpCode.ILSpyX/LoadedAssembly.cs index 6cf25e2cf..e68a1096a 100644 --- a/ICSharpCode.ILSpyX/LoadedAssembly.cs +++ b/ICSharpCode.ILSpyX/LoadedAssembly.cs @@ -283,13 +283,25 @@ namespace ICSharpCode.ILSpyX public string ShortName => shortName; + // Cached once a successful Text computation completes. Reads after Dispose return + // this value (or ShortName fallback) instead of re-walking the metadata, whose + // MemoryMappedFile may have been unmapped -- a dereference AVs and the CLR fails + // fast on the resulting CorruptingStateException. + string? cachedText; + volatile bool isDisposed; + public string Text { get { + if (cachedText is not null) + return cachedText; + if (isDisposed) + return ShortName; if (IsLoaded && !HasLoadError) { var result = GetLoadResultAsync().GetAwaiter().GetResult(); if (result.MetadataFile != null) { + string computed; switch (result.MetadataFile.Kind) { case MetadataFile.MetadataFileKind.PortableExecutable: @@ -306,16 +318,22 @@ namespace ICSharpCode.ILSpyX { versionOrInfo = ".netmodule"; } - if (versionOrInfo == null) - return ShortName; - return string.Format("{0} ({1})", ShortName, versionOrInfo); + computed = versionOrInfo == null + ? ShortName + : string.Format("{0} ({1})", ShortName, versionOrInfo); + break; case MetadataFile.MetadataFileKind.ProgramDebugDatabase: - return ShortName + " (Debug Metadata)"; + computed = ShortName + " (Debug Metadata)"; + break; case MetadataFile.MetadataFileKind.Metadata: - return ShortName + " (Metadata)"; + computed = ShortName + " (Metadata)"; + break; default: - return ShortName; + computed = ShortName; + break; } + cachedText = computed; + return computed; } } return ShortName; @@ -695,6 +713,12 @@ namespace ICSharpCode.ILSpyX public void Dispose() { + // Order matters: set the flag BEFORE unmapping the metadata, so any concurrent + // Text reader that hasn't yet cached a value bails to ShortName instead of + // dereferencing the about-to-be-freed MemoryMappedFile pages. Once set, the flag + // also short-circuits future Text getter walks even if cachedText is still null + // (load never completed). + isDisposed = true; // Only inspect the load task if it's been started. Disposing a never-loaded // LoadedAssembly must not synchronously kick off the load just to dispose its // (still-non-existent) MetadataFile. diff --git a/ILSpy/Docking/DockWorkspace.cs b/ILSpy/Docking/DockWorkspace.cs index 8c5bb5f4d..9a18d5420 100644 --- a/ILSpy/Docking/DockWorkspace.cs +++ b/ILSpy/Docking/DockWorkspace.cs @@ -257,23 +257,46 @@ namespace ILSpy.Docking foreach (var dockable in visible.OfType().ToArray()) { - // Skip the persistent MainTab — its content swaps with tree selection. Also - // skip static content (About page etc.) which doesn't reference assemblies. - if (ReferenceEquals(dockable, factory.MainTab)) - continue; if (dockable.Content is not TextView.DecompilerTabPageModel tab || tab.IsStaticContent) continue; var nodes = tab.CurrentNodes; if (nodes.Count == 0) continue; - // "Some node in the tab still lives in a non-removed assembly" → keep. - // "Every node points at a removed assembly" → close. - bool anyAlive = nodes.Any(n => - n.AncestorsAndSelf() - .OfType() - .LastOrDefault() is { } owner && !removed.Contains(owner.LoadedAssembly)); - if (!anyAlive) + bool anyTouchesRemoved = false; + bool anyAlive = false; + foreach (var n in nodes) + { + var owner = n.AncestorsAndSelf().OfType().LastOrDefault(); + if (owner is null) + continue; + if (removed.Contains(owner.LoadedAssembly)) + anyTouchesRemoved = true; + else + anyAlive = true; + } + if (!anyTouchesRemoved) + continue; + // Cancel synchronously, BEFORE AssemblyList.Unload reaches assembly.Dispose(). + // The decompile worker checks its CancellationToken between transforms; the + // spinner exits when its Task.Delay sees the cancellation. After this returns + // no managed reader should hold a live handle into the MetadataFile that's + // about to be unmapped. (LoadedAssembly.Text additionally returns its cached + // value once isDisposed flips, covering any residual cooperative-cancel race.) + tab.CancelDecompilationCommand.Execute(null); + if (anyAlive) + continue; + if (ReferenceEquals(dockable, factory.MainTab)) + { + // Persistent slot: empty it. The CurrentNodes setter unsubscribes from each + // node's PropertyChanged, so a delayed AssemblyTreeNode.RaisePropertyChanged + // post-Dispose has no listener to drag into a metadata read. + tab.CurrentNodes = System.Array.Empty(); + dockable.SourceNode = null; + } + else + { factory.CloseDockable(dockable); + } } } diff --git a/ILSpy/TextView/DecompilerTabPageModel.cs b/ILSpy/TextView/DecompilerTabPageModel.cs index b688a3da4..51331ed9f 100644 --- a/ILSpy/TextView/DecompilerTabPageModel.cs +++ b/ILSpy/TextView/DecompilerTabPageModel.cs @@ -232,6 +232,14 @@ namespace ILSpy.TextView IReadOnlyList currentNodes = System.Array.Empty(); + // Snapshot of the tab title derived from currentNodes' Text values, taken on the + // UI thread at the moment CurrentNodes is set (and refreshed when a node raises + // PropertyChanged(Text)). Spinner ticks, post-decompile InvokeAsync, and the + // cancel-cleanup path read this cached string instead of re-walking node.Text -> + // LoadedAssembly.Text -> metadata, which AVs when an assembly was unloaded + // between selection and the title update. + string cachedBaseTitle = "(unnamed)"; + /// /// True for tabs whose content is a static page (e.g. About) rather than the result /// of decompiling a tree-node selection. Static tabs are excluded from the lookup @@ -271,6 +279,7 @@ namespace ILSpy.TextView foreach (var n in currentNodes) n.PropertyChanged -= OnCurrentNodePropertyChanged; currentNodes = value.ToArray(); + cachedBaseTitle = ComposeBaseTitle(); foreach (var n in currentNodes) n.PropertyChanged += OnCurrentNodePropertyChanged; StartDecompile(); @@ -285,8 +294,8 @@ namespace ILSpy.TextView // just refresh the suffix; otherwise replace the title outright. if (e.PropertyName != nameof(ILSpyTreeNode.Text)) return; - var baseTitle = ComposeBaseTitle(); - Title = IsDecompiling ? ComposeSpinnerTitle(0, baseTitle) : baseTitle; + cachedBaseTitle = ComposeBaseTitle(); + Title = IsDecompiling ? ComposeSpinnerTitle(0, cachedBaseTitle) : cachedBaseTitle; } string ComposeBaseTitle() @@ -387,7 +396,7 @@ namespace ILSpy.TextView // Spinner appears as a glyph prefix on the tab title while the decompile runs; // editor state is left untouched so cancellation falls back cleanly. - Title = ComposeSpinnerTitle(0, ComposeBaseTitle()); + Title = ComposeSpinnerTitle(0, cachedBaseTitle); TaskbarProgress?.SetState(TaskbarProgressState.Indeterminate); _ = RunSpinnerAsync(cts.Token); @@ -463,10 +472,7 @@ namespace ILSpy.TextView ILSpy.AppEnv.AppLog.Mark($"DecompileAsync #{callNumber}: {rendered.Length} chars, {(collectedFoldings?.Count ?? 0)} foldings, {(collectedReferences?.Count ?? 0)} refs"); using (ILSpy.AppEnv.AppLog.Phase($"DecompileAsync #{callNumber}: Dispatcher.InvokeAsync (apply Text + props, triggers ApplyDocument)")) await Dispatcher.UIThread.InvokeAsync(() => { - // Re-read Text now (instead of capturing it before decompile started) — for - // freshly-opened assemblies, Text only has the rich "(version, tfm)" form - // after the load completes during decompile. - Title = ComposeBaseTitle(); + Title = cachedBaseTitle; SyntaxExtension = effectiveSyntaxExtension; HighlightingModel = model; Foldings = collectedFoldings; @@ -494,7 +500,7 @@ namespace ILSpy.TextView // If we cancelled before producing fresh output, drop the spinner glyph // from the title — the editor still shows the previous decompile. if (currentNodes.Count > 0) - Title = ComposeBaseTitle(); + Title = cachedBaseTitle; TaskbarProgress?.SetState(TaskbarProgressState.None); } if (Dispatcher.UIThread.CheckAccess()) @@ -528,7 +534,7 @@ namespace ILSpy.TextView } if (token.IsCancellationRequested || !IsDecompiling) return; - Title = ComposeSpinnerTitle(frame++, ComposeBaseTitle()); + Title = ComposeSpinnerTitle(frame++, cachedBaseTitle); } }