Browse Source

Stop the assembly list from disposing removed assemblies

Unload / Clear / ReloadAssembly / HotReplaceAssembly all called LoadedAssembly.Dispose() on the assembly they removed, which disposes its MetadataFile and unmaps the underlying file. But open document tabs and tree nodes can still hold that MetadataFile, and the list has no safe point at which to know those references are gone -- so disposing risked unmapping a file out from under a live reader (use-after-dispose). Drop the removed assembly and let the GC reclaim it once nothing references it instead. The DockWorkspace cancel-on-remove is now a courtesy, not a guard against an unmap race.

(cherry picked from commit c5f8834e57)
release/10.1
Siegfried Pammer 3 months ago committed by Christoph Wille
parent
commit
9c760da9dd
  1. 20
      ICSharpCode.ILSpyX/AssemblyList.cs

20
ICSharpCode.ILSpyX/AssemblyList.cs

@ -340,7 +340,6 @@ namespace ICSharpCode.ILSpyX @@ -340,7 +340,6 @@ namespace ICSharpCode.ILSpyX
{
VerifyAccess();
file = Path.GetFullPath(file);
LoadedAssembly evicted;
LoadedAssembly newAsm;
lock (lockObj)
{
@ -358,9 +357,11 @@ namespace ICSharpCode.ILSpyX @@ -358,9 +357,11 @@ namespace ICSharpCode.ILSpyX
Debug.Assert(newAsm.FileName == file);
byFilename[file] = newAsm;
this.assemblies[index] = newAsm;
evicted = target;
}
evicted.Dispose();
// The replaced assembly is intentionally NOT disposed: its MetadataFile may still be
// referenced by open document tabs / tree nodes, and there is no safe point at which
// the list can know those references are gone. Dropping it lets the GC reclaim it
// once nothing holds it, rather than risk a use-after-dispose.
return newAsm;
}
@ -391,7 +392,8 @@ namespace ICSharpCode.ILSpyX @@ -391,7 +392,8 @@ namespace ICSharpCode.ILSpyX
this.assemblies.Remove(target);
this.assemblies.Insert(index, newAsm);
}
target.Dispose();
// Not disposed on purpose -- see HotReplaceAssembly. The old MetadataFile may still be
// live in the UI; let the GC reclaim it instead of risking a use-after-dispose.
return newAsm;
}
@ -403,21 +405,21 @@ namespace ICSharpCode.ILSpyX @@ -403,21 +405,21 @@ namespace ICSharpCode.ILSpyX
assemblies.Remove(assembly);
byFilename.Remove(assembly.FileName);
}
assembly.Dispose();
// Removed from the list but NOT disposed: open tabs / tree nodes may still hold its
// MetadataFile and there's no safe point to know they don't. The GC reclaims it once
// the last reference is gone.
}
public void Clear()
{
VerifyAccess();
LoadedAssembly[] removed;
lock (lockObj)
{
removed = assemblies.ToArray();
assemblies.Clear();
byFilename.Clear();
}
foreach (var asm in removed)
asm.Dispose();
// Cleared but not disposed -- see Unload. Lingering references in the UI must not see
// a disposed MetadataFile.
}
public void Sort(IComparer<LoadedAssembly> comparer)
{

Loading…
Cancel
Save