Browse Source

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
pull/3755/head
Siegfried Pammer 2 months ago
parent
commit
bb114b27a7
  1. 36
      ICSharpCode.ILSpyX/LoadedAssembly.cs
  2. 45
      ILSpy/Docking/DockWorkspace.cs
  3. 24
      ILSpy/TextView/DecompilerTabPageModel.cs

36
ICSharpCode.ILSpyX/LoadedAssembly.cs

@ -283,13 +283,25 @@ namespace ICSharpCode.ILSpyX @@ -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 @@ -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 @@ -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.

45
ILSpy/Docking/DockWorkspace.cs

@ -257,23 +257,46 @@ namespace ILSpy.Docking @@ -257,23 +257,46 @@ namespace ILSpy.Docking
foreach (var dockable in visible.OfType<ViewModels.ContentTabPage>().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<TreeNodes.AssemblyTreeNode>()
.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<TreeNodes.AssemblyTreeNode>().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<ILSpyTreeNode>();
dockable.SourceNode = null;
}
else
{
factory.CloseDockable(dockable);
}
}
}

24
ILSpy/TextView/DecompilerTabPageModel.cs

@ -232,6 +232,14 @@ namespace ILSpy.TextView @@ -232,6 +232,14 @@ namespace ILSpy.TextView
IReadOnlyList<ILSpyTreeNode> currentNodes = System.Array.Empty<ILSpyTreeNode>();
// 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)";
/// <summary>
/// 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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -528,7 +534,7 @@ namespace ILSpy.TextView
}
if (token.IsCancellationRequested || !IsDecompiling)
return;
Title = ComposeSpinnerTitle(frame++, ComposeBaseTitle());
Title = ComposeSpinnerTitle(frame++, cachedBaseTitle);
}
}

Loading…
Cancel
Save