From d814f4a47ebb452387d82e3512677432f8357e33 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Tue, 8 Sep 2026 14:20:32 +0200 Subject: [PATCH 1/3] Remove a set of assemblies from the list in one step Deleting assemblies one at a time is not equivalent to deleting them together: each removal raises its own collection change, and the listeners of the assembly list react to one of those by pruning navigation history, restarting a running search, re-querying every bound command and re-decompiling whatever is still selected. Selecting an expanded list and pressing Delete therefore paid for all of that once per assembly, and froze the UI. Emptying the list still reports a Remove rather than the Reset that Clear() raises, because consumers read Reset as "re-read everything" and several short-circuit their per-removal cleanup on it, which would leave tabs and navigation history pointing at unloaded assemblies. The tree's removal is reordered for the same reason: it detached and announced one node at a time, so a listener reacting to the first notification still found the rest of the same removal attached and took them for a selection the user had made. Assisted-by: Claude:claude-opus-5[1m]:Claude Code Assisted-by: OpenCode:openai/gpt-5.5:OpenCode --- ICSharpCode.ILSpyX/AssemblyList.cs | 66 +++++++++++++++++++- ICSharpCode.ILSpyX/TreeView/SharpTreeNode.cs | 19 ++++++ ILSpy/Controls/TreeView/SharpTreeView.cs | 3 +- ILSpy/TreeNodes/AssemblyTreeNode.cs | 22 ++++++- 4 files changed, 104 insertions(+), 6 deletions(-) diff --git a/ICSharpCode.ILSpyX/AssemblyList.cs b/ICSharpCode.ILSpyX/AssemblyList.cs index 49d9fbae31d..c23bedbd560 100644 --- a/ICSharpCode.ILSpyX/AssemblyList.cs +++ b/ICSharpCode.ILSpyX/AssemblyList.cs @@ -57,7 +57,7 @@ public sealed class AssemblyList /// Technically read accesses need locking when done on non-GUI threads... but whenever possible, use the /// thread-safe method. /// - readonly ObservableCollection assemblies = new ObservableCollection(); + readonly AssemblyCollection assemblies = new AssemblyCollection(); /// /// Assembly lookup by filename. @@ -428,6 +428,70 @@ public void Unload(LoadedAssembly assembly) // the last reference is gone. } + /// + /// Removes every assembly in in one step. + /// + /// + /// Removing them one at a time is not equivalent: each removal raises its own collection + /// change, and consumers react to one of those by pruning navigation history, restarting a + /// running search, re-querying every command and re-decompiling the surviving selection. + /// Repeating that per assembly is what made clearing an expanded list freeze the UI. + /// Contiguous runs are removed as ranges, so listeners keep the indices and the removed + /// items they need to splice their own state. Emptying the list this way still reports a + /// Remove rather than the Reset raises: consumers treat Reset as + /// "re-read everything" and several of them short-circuit their per-removal cleanup on it, + /// which would leave tabs and navigation history pointing at unloaded assemblies. + /// + public void UnloadRange(IEnumerable assembliesToUnload) + { + ArgumentNullException.ThrowIfNull(assembliesToUnload); + VerifyAccess(); + var doomed = new HashSet(assembliesToUnload); + if (doomed.Count == 0) + return; + lock (lockObj) + { + // Only the named entries leave byFilename, so an assembly a background thread is + // still loading survives -- unlike Clear(), which drops the lookup wholesale. + foreach (var assembly in doomed) + byFilename.Remove(assembly.FileName); + // Backwards, so the indices of the runs not yet visited stay valid. + for (int end = assemblies.Count - 1; end >= 0;) + { + if (!doomed.Contains(assemblies[end])) + { + end--; + continue; + } + int start = end; + while (start > 0 && doomed.Contains(assemblies[start - 1])) + start--; + assemblies.RemoveAssemblies(start, end - start + 1); + end = start - 1; + } + } + // Removed from the list but NOT disposed -- see Unload. + } + + sealed class AssemblyCollection : ObservableCollection + { + public void RemoveAssemblies(int index, int count) + { + if (count <= 0) + return; + CheckReentrancy(); + var removed = new List(count); + for (int i = 0; i < count; i++) + removed.Add(Items[index + i]); + for (int i = 0; i < count; i++) + Items.RemoveAt(index); + OnPropertyChanged(new System.ComponentModel.PropertyChangedEventArgs(nameof(Count))); + OnPropertyChanged(new System.ComponentModel.PropertyChangedEventArgs("Item[]")); + OnCollectionChanged(new NotifyCollectionChangedEventArgs( + NotifyCollectionChangedAction.Remove, removed, index)); + } + } + public void Clear() { VerifyAccess(); diff --git a/ICSharpCode.ILSpyX/TreeView/SharpTreeNode.cs b/ICSharpCode.ILSpyX/TreeView/SharpTreeNode.cs index b7ff6f71607..405621a7f59 100644 --- a/ICSharpCode.ILSpyX/TreeView/SharpTreeNode.cs +++ b/ICSharpCode.ILSpyX/TreeView/SharpTreeNode.cs @@ -212,11 +212,18 @@ public virtual void OnChildrenChanged(NotifyCollectionChangedEventArgs e) } if (e.OldItems != null) { + // Detach the whole set before announcing any of it. The per-node notifications + // below reach the selection, and a listener that reacts to the first of them would + // otherwise still find the rest of the same removal attached, and treat those as a + // selection the user made -- ending with the last node of the set on its own. foreach (SharpTreeNode node in e.OldItems) { Debug.Assert(node.modelParent == this); node.modelParent = null; node.OnParentChanged(); + } + foreach (SharpTreeNode node in e.OldItems) + { Debug.WriteLine("Removing {0} from {1}", node, this); SharpTreeNode removeEnd = node; while (removeEnd.modelChildren != null && removeEnd.modelChildren.Count > 0) @@ -704,6 +711,18 @@ public virtual void DeleteCore() throw new NotSupportedException(GetType().Name + " does not support deletion"); } + /// + /// Deletes -- the whole set the user asked to remove, this node + /// included. Override when the underlying model can drop a set in one operation, so its + /// listeners see one change instead of one per node. + /// + public virtual void Delete(SharpTreeNode[] nodes) + { + ArgumentNullException.ThrowIfNull(nodes); + foreach (var node in nodes) + node.Delete(); + } + public virtual IPlatformDataObject Copy(SharpTreeNode[] nodes) { throw new NotSupportedException(GetType().Name + " does not support copy/paste or drag'n'drop"); diff --git a/ILSpy/Controls/TreeView/SharpTreeView.cs b/ILSpy/Controls/TreeView/SharpTreeView.cs index bc0ad7316f7..54b241f381d 100644 --- a/ILSpy/Controls/TreeView/SharpTreeView.cs +++ b/ILSpy/Controls/TreeView/SharpTreeView.cs @@ -535,8 +535,7 @@ bool DeleteSelection() if (nodes.Length == 0 || !nodes.All(n => n.CanDelete())) return false; int index = nodes.Min(flattener.IndexOf); - foreach (var node in nodes) - node.Delete(); + nodes[0].Delete(nodes); // The deleted rows leave the selection with the source; pick the nearest survivor. if (SelectedItems!.Count == 0 && flattener.Count > 0) SelectAndFocus((SharpTreeNode)flattener[Math.Clamp(index, 0, flattener.Count - 1)]!); diff --git a/ILSpy/TreeNodes/AssemblyTreeNode.cs b/ILSpy/TreeNodes/AssemblyTreeNode.cs index 65101697375..f2e8bc57607 100644 --- a/ILSpy/TreeNodes/AssemblyTreeNode.cs +++ b/ILSpy/TreeNodes/AssemblyTreeNode.cs @@ -178,6 +178,21 @@ void InitFromLoadResult() public override void DeleteCore() => assembly.AssemblyList.Unload(assembly); + // Removing the assemblies one at a time makes every listener of the list (navigation + // history, running search, command re-query, open tabs) do its full reaction once per + // assembly; UnloadRange raises a single change for the whole set instead. + public override void Delete(SharpTreeNode[] nodes) + { + ArgumentNullException.ThrowIfNull(nodes); + assembly.AssemblyList.UnloadRange(nodes.OfType().Select(n => n.assembly)); + // A set that also holds other deletable node types still has to lose those. + foreach (var node in nodes) + { + if (node is not AssemblyTreeNode) + node.Delete(); + } + } + public override bool Save() { // Intercept the File → Save Code flow for valid managed assemblies whose active @@ -647,9 +662,10 @@ public void Execute(TextViewContext context) { if (context.SelectedTreeNodes == null) return; - // Snapshot before mutation — Unload reshapes the tree and the live selection. - foreach (var node in context.SelectedTreeNodes.OfType().ToArray()) - node.Delete(); + // Snapshot before mutation — unloading reshapes the tree and the live selection. + var nodes = context.SelectedTreeNodes.OfType().ToArray(); + if (nodes.Length > 0) + nodes[0].Delete(nodes); } } From b66fc20038ff436cd23e14ef993aa5d0f0492184 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Tue, 8 Sep 2026 14:21:06 +0200 Subject: [PATCH 2/3] Coalesce the assembly tree's selection fan-out A selection change reaches a full application-wide command re-query, a session-settings write, a message-bus broadcast and a decompile of the new selection. The model already had a flag to hold that off during a bulk edit, but only its own SelectNodes ever set it: the binder that mirrors the tree's selection into the model added and removed one node at a time, so selecting every row cost one fan-out per row. The flag becomes a scope any bulk edit can enter, and the binder uses it. Membership now comes from a set rather than a scan of the model selection per node, which was quadratic once a large part of the tree was selected. DockWorkspace subscribed to both the selection collection and the SelectedItem property, so it showed the selection twice per change and once per node during a bulk edit; the property already fires when the selection settles, so the collection subscription goes. Assisted-by: Claude:claude-opus-5[1m]:Claude Code Assisted-by: OpenCode:openai/gpt-5.5:OpenCode --- .../SelectAllDeleteBatchingTests.cs | 132 ++++++++++++++++++ ILSpy/AssemblyTree/AssemblyListPane.axaml.cs | 3 +- ILSpy/AssemblyTree/AssemblyTreeModel.cs | 67 +++++++-- ILSpy/Controls/TreeView/SharpTreeView.cs | 13 +- .../Controls/TreeView/TreeSelectionBinder.cs | 22 ++- ILSpy/Docking/DockWorkspace.cs | 8 +- 6 files changed, 221 insertions(+), 24 deletions(-) create mode 100644 ILSpy.Tests/AssemblyList/SelectAllDeleteBatchingTests.cs diff --git a/ILSpy.Tests/AssemblyList/SelectAllDeleteBatchingTests.cs b/ILSpy.Tests/AssemblyList/SelectAllDeleteBatchingTests.cs new file mode 100644 index 00000000000..1f2b3c2c420 --- /dev/null +++ b/ILSpy.Tests/AssemblyList/SelectAllDeleteBatchingTests.cs @@ -0,0 +1,132 @@ +// Copyright (c) 2026 Siegfried Pammer +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System.Collections.Specialized; +using System.ComponentModel; +using System.Threading.Tasks; + +using Avalonia.Controls; +using Avalonia.Headless.NUnit; +using Avalonia.Input; + +using AwesomeAssertions; + +using ICSharpCode.ILSpy.AssemblyTree; +using ICSharpCode.ILSpy.Controls.TreeView; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests; + +/// +/// Ctrl+A followed by Delete must cost one batch, not one fan-out per row. Every selection +/// change reaches a full application-wide command re-query, a history prune, and a decompile +/// kick-off; repeating that per node (and again per removed assembly) is what made clearing an +/// expanded list freeze the UI. These pin the batching so the cost stays proportional to the +/// number of user actions rather than to the number of selected nodes. +/// +[TestFixture] +public class SelectAllDeleteBatchingTests +{ + static void RaiseKey(Control target, Key key, KeyModifiers modifiers) + { + target.RaiseEvent(new KeyEventArgs { + Key = key, + KeyModifiers = modifiers, + RoutedEvent = InputElement.KeyDownEvent, + Source = target, + }); + } + + // Deliberately not WaitForIdleAsync: a select-all starts a decompile of everything selected, + // and its progress spinner keeps posting to the dispatcher until that finishes, so the app is + // never idle. Everything asserted here -- the selection, the fan-out count, the list change -- + // lands synchronously while the key is handled, so pumping once is the right synchronization + // point. The tree is left collapsed for the same reason: expanding an assembly first would put + // every type in it into the selection, and decompiling those dominates the run without making + // the assertions any sharper. One fan-out versus one per row already separates the two + // behaviours at three rows. + static void SettleInput() => Waiters.PumpUI(); + + [AvaloniaTest] + public async Task Select_All_Fans_Out_Once_Not_Once_Per_Node() + { + var (window, vm) = await TestHarness.BootAsync(3); + var pane = await window.WaitForComponent(); + var tree = pane.FindControl("Tree")!; + var model = vm.AssemblyTreeModel; + + int rows = ((System.Collections.IList)tree.ItemsSource!).Count; + rows.Should().BeGreaterThan(2, "the fan-out only shows up on a multi-row selection"); + + int fanOuts = 0; + void OnChanged(object? _, PropertyChangedEventArgs e) + { + if (e.PropertyName == nameof(AssemblyTreeModel.SelectedItem)) + fanOuts++; + } + model.PropertyChanged += OnChanged; + try + { + RaiseKey(tree, Key.A, KeyModifiers.Control); + SettleInput(); + } + finally + { + model.PropertyChanged -= OnChanged; + } + + model.SelectedItems.Should().HaveCount(rows, "Ctrl+A selects every visible row"); + fanOuts.Should().Be(1, + "selecting N rows fans out once, not once per row -- each fan-out runs a full command " + + "re-query, a session-settings write and a decompile of the selection"); + } + + [AvaloniaTest] + public async Task Deleting_Every_Assembly_Raises_One_List_Change_Not_One_Per_Assembly() + { + var (window, vm) = await TestHarness.BootAsync(3); + var pane = await window.WaitForComponent(); + var tree = pane.FindControl("Tree")!; + var model = vm.AssemblyTreeModel; + var list = model.AssemblyList!; + + int assemblies = list.Count; + assemblies.Should().BeGreaterThan(2); + + int listChanges = 0; + void OnListChanged(object? _, NotifyCollectionChangedEventArgs e) => listChanges++; + list.CollectionChanged += OnListChanged; + try + { + RaiseKey(tree, Key.A, KeyModifiers.Control); + SettleInput(); + RaiseKey(tree, Key.Delete, KeyModifiers.None); + SettleInput(); + } + finally + { + list.CollectionChanged -= OnListChanged; + } + + list.Count.Should().Be(0, "Delete on a select-all removes every assembly"); + listChanges.Should().Be(1, + "removing N assemblies must raise one batched collection change, not one per assembly -- " + + "every consumer (history prune, search restart, command re-query, tab prune) runs per event"); + } +} diff --git a/ILSpy/AssemblyTree/AssemblyListPane.axaml.cs b/ILSpy/AssemblyTree/AssemblyListPane.axaml.cs index 9f8202fc647..724a35599bf 100644 --- a/ILSpy/AssemblyTree/AssemblyListPane.axaml.cs +++ b/ILSpy/AssemblyTree/AssemblyListPane.axaml.cs @@ -272,7 +272,8 @@ protected override void OnDataContextChanged(EventArgs e) Tree.Root = model.Root; WireDropSelection(model); } - selectionBinder = new ICSharpCode.ILSpy.Controls.TreeView.TreeSelectionBinder(Tree, model.SelectedItems); + selectionBinder = new ICSharpCode.ILSpy.Controls.TreeView.TreeSelectionBinder( + Tree, model.SelectedItems, model.BatchSelectionChange); } } diff --git a/ILSpy/AssemblyTree/AssemblyTreeModel.cs b/ILSpy/AssemblyTree/AssemblyTreeModel.cs index a57d7f07f14..b3ffa70c804 100644 --- a/ILSpy/AssemblyTree/AssemblyTreeModel.cs +++ b/ILSpy/AssemblyTree/AssemblyTreeModel.cs @@ -109,21 +109,16 @@ public void SelectNodes(IReadOnlyList nodes) ArgumentNullException.ThrowIfNull(nodes); if (SelectionMatches(nodes)) return; - batchingSelectionChange = true; - try + using (BatchSelectionChange()) { SelectedItems.Clear(); + var seen = new HashSet(); foreach (var node in nodes) { - if (node != null && !SelectedItems.Contains(node)) + if (node != null && seen.Add(node)) SelectedItems.Add(node); } } - finally - { - batchingSelectionChange = false; - } - RaiseSelectionChanged(); } bool SelectionMatches(IReadOnlyList nodes) @@ -204,10 +199,49 @@ void OnNavigateToReference(object? sender, Util.NavigateToReferenceEventArgs e) decompTab.HighlightedReference = e.Source; } - // True while the SelectedItem setter is replacing the collection via Clear()+Add(); - // suppresses the selection-changed fan-out until the final state is in place so consumers - // never observe the transient empty/multi mid-replace. - bool batchingSelectionChange; + // Greater than zero while a bulk edit is rewriting SelectedItems; suppresses the + // selection-changed fan-out until the final state is in place so consumers never observe a + // transient empty/multi mid-replace, and so the fan-out costs one pass rather than one per + // node. RaiseSelectionChanged reaches a full command re-query, a session-settings write, a + // message-bus broadcast and a decompile of the new selection, so running it per node is + // what made selecting a large subtree freeze the UI. + int selectionBatchDepth; + + // Set when a change actually arrived while a batch was open, so a batch that rewrote the + // selection to what it already was closes without notifying anyone. + bool selectionChangedDuringBatch; + + /// + /// Suppresses the selection fan-out for the lifetime of the returned scope, then raises it + /// exactly once. Nests; only the outermost scope raises. + /// + public IDisposable BatchSelectionChange() => new SelectionBatchScope(this); + + sealed class SelectionBatchScope : IDisposable + { + readonly AssemblyTreeModel model; + bool disposed; + + public SelectionBatchScope(AssemblyTreeModel model) + { + this.model = model; + model.selectionBatchDepth++; + } + + public void Dispose() + { + if (disposed) + return; + disposed = true; + if (--model.selectionBatchDepth > 0) + return; + if (model.selectionChangedDuringBatch) + { + model.selectionChangedDuringBatch = false; + model.RaiseSelectionChanged(); + } + } + } void OnSelectedItemsChanged(object? sender, NotifyCollectionChangedEventArgs e) { @@ -218,10 +252,13 @@ void OnSelectedItemsChanged(object? sender, NotifyCollectionChangedEventArgs e) foreach (SharpTreeNode n in e.OldItems) n.IsSelected = false; - // During a SelectedItem-setter batch the fan-out is deferred to the single - // RaiseSelectionChanged() the setter issues once the final selection is in place. - if (batchingSelectionChange) + // Inside a batch the fan-out is deferred to the single RaiseSelectionChanged() the + // outermost scope issues once the final selection is in place. + if (selectionBatchDepth > 0) + { + selectionChangedDuringBatch = true; return; + } RaiseSelectionChanged(); } diff --git a/ILSpy/Controls/TreeView/SharpTreeView.cs b/ILSpy/Controls/TreeView/SharpTreeView.cs index 54b241f381d..844f2cd35e3 100644 --- a/ILSpy/Controls/TreeView/SharpTreeView.cs +++ b/ILSpy/Controls/TreeView/SharpTreeView.cs @@ -304,7 +304,7 @@ public void FocusNode(SharpTreeNode node, bool scroll = true) public bool IsNodeFullyVisible(SharpTreeNode node) { ArgumentNullException.ThrowIfNull(node); - var scrollViewer = this.GetVisualDescendants().OfType().FirstOrDefault(); + var scrollViewer = ScrollHost; if (scrollViewer is null) return false; if (ContainerFromItem(node) is Control row && row.IsVisible @@ -313,6 +313,17 @@ public bool IsNodeFullyVisible(SharpTreeNode node) return false; } + ScrollViewer? scrollHost; + + /// + /// The template's , looked up once. Callers that ask per node -- + /// a selection sync checks every selected row -- would otherwise walk the visual tree once + /// per call. Re-resolved while null so a query made before the template is applied does not + /// cache the miss. + /// + ScrollViewer? ScrollHost + => scrollHost ??= this.GetVisualDescendants().OfType().FirstOrDefault(); + /// Moves the (single) selection to and focuses it. /// Used by keyboard navigation where selection must follow the caret. void SelectAndFocus(SharpTreeNode node) diff --git a/ILSpy/Controls/TreeView/TreeSelectionBinder.cs b/ILSpy/Controls/TreeView/TreeSelectionBinder.cs index 2201b91078b..ba167261ff7 100644 --- a/ILSpy/Controls/TreeView/TreeSelectionBinder.cs +++ b/ILSpy/Controls/TreeView/TreeSelectionBinder.cs @@ -40,12 +40,20 @@ public sealed class TreeSelectionBinder : IDisposable { readonly SharpTreeView tree; readonly ObservableCollection modelSelection; + readonly Func? batchSelectionChange; bool syncing; - public TreeSelectionBinder(SharpTreeView tree, ObservableCollection modelSelection) + /// + /// Optional: opens a scope over which the view-model coalesces its selection fan-out, so a + /// sync that touches many rows costs one notification instead of one per row. Panes whose + /// model has no such scope pass null and get the per-item behaviour. + /// + public TreeSelectionBinder(SharpTreeView tree, ObservableCollection modelSelection, + Func? batchSelectionChange = null) { this.tree = tree ?? throw new ArgumentNullException(nameof(tree)); this.modelSelection = modelSelection ?? throw new ArgumentNullException(nameof(modelSelection)); + this.batchSelectionChange = batchSelectionChange; tree.SelectionChanged += OnTreeSelectionChanged; tree.Loaded += OnTreeLoaded; modelSelection.CollectionChanged += OnModelSelectionChanged; @@ -83,14 +91,22 @@ void OnTreeSelectionChanged(object? sender, SelectionChangedEventArgs e) try { var current = tree.SelectedItems!.OfType().ToHashSet(); + // One batch for the whole reconciliation: each add/remove otherwise fans out into a + // full command re-query and a decompile of the intermediate selection. + using var batch = batchSelectionChange?.Invoke(); + // Membership comes from a set, not a scan of modelSelection per node -- with every + // row selected the linear scan made this quadratic. + var kept = new HashSet(); for (int i = modelSelection.Count - 1; i >= 0; i--) { - if (!current.Contains(modelSelection[i])) + if (current.Contains(modelSelection[i])) + kept.Add(modelSelection[i]); + else modelSelection.RemoveAt(i); } foreach (var node in current) { - if (!modelSelection.Contains(node)) + if (!kept.Contains(node)) modelSelection.Add(node); } } diff --git a/ILSpy/Docking/DockWorkspace.cs b/ILSpy/Docking/DockWorkspace.cs index 37c88ff1d9b..b1b6b612094 100644 --- a/ILSpy/Docking/DockWorkspace.cs +++ b/ILSpy/Docking/DockWorkspace.cs @@ -202,11 +202,11 @@ public DockWorkspace( factory.InitLayout(Layout); } + // A selection change reaches ShowSelectedNode through OnAssemblyTreePropertyChanged: + // the model raises SelectedItem whenever the selection settles, including once at the + // end of a bulk edit. Subscribing to SelectedItems.CollectionChanged as well would run + // it a second time, and once per node during a bulk edit. assemblyTreeModel.PropertyChanged += OnAssemblyTreePropertyChanged; - assemblyTreeModel.SelectedItems.CollectionChanged += (_, _) => { - if (!syncingTreeFromActiveTab) - ShowSelectedNode(); - }; languageService.PropertyChanged += OnLanguagePropertyChanged; ToolPaneMenuItems = toolPaneRegistry.Panes From 359d40ba684e0694dc865bab0ce2c165aa1ad8e3 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Tue, 29 Sep 2026 08:52:49 +0200 Subject: [PATCH 3/3] Ignore stale tree selections after removal Batched selection fan-out can briefly replay a selection for nodes whose assemblies were already removed. Ignore those stale selections and version decompiler-tab updates so late spinner or output continuations cannot overwrite an explicit clear. Assisted-by: OpenCode:openai/gpt-5.5:OpenCode --- ILSpy/Docking/DockWorkspace.cs | 19 ++++++++++ ILSpy/TextView/DecompilerTabPageModel.cs | 44 +++++++++++++++++++----- 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/ILSpy/Docking/DockWorkspace.cs b/ILSpy/Docking/DockWorkspace.cs index b1b6b612094..6587e91b922 100644 --- a/ILSpy/Docking/DockWorkspace.cs +++ b/ILSpy/Docking/DockWorkspace.cs @@ -799,6 +799,16 @@ void ShowSelectedNode() lastShownNodes = null; return; } + var activeAssemblies = new HashSet( + assemblyTreeModel.AssemblyList?.GetAssemblies() ?? []); + if (nodes.Any(n => n.AncestorsAndSelf() + .OfType() + .Any(a => !activeAssemblies.Contains(a.LoadedAssembly)))) + { + lastShownNodes = null; + ClearActiveDecompilerTab(); + return; + } // SelectedItems.CollectionChanged and SelectedItem PropertyChanged both fan into // here on a single click, so dedupe to avoid creating two TabPageModels for the // same selection — the second one's columns would replace the first's, but the @@ -1046,6 +1056,15 @@ static void CopyContentState(ContentPageModel source, ContentPageModel target) public DecompilerTabPageModel? ActiveDecompilerTab => factory.MainTab?.Content as DecompilerTabPageModel is { IsStaticContent: false } d ? d : null; + public void ClearActiveDecompilerTab() + { + if (ActiveDecompilerTab is not { } tab) + return; + tab.ClearContent(); + if (factory.MainTab is { } main) + main.SourceNode = null; + } + /// /// Forwards to on the active /// decompiler tab. Convenience wrapper used by long-running commands (Create Diagram, diff --git a/ILSpy/TextView/DecompilerTabPageModel.cs b/ILSpy/TextView/DecompilerTabPageModel.cs index b60982761e2..d4fb52d4f96 100644 --- a/ILSpy/TextView/DecompilerTabPageModel.cs +++ b/ILSpy/TextView/DecompilerTabPageModel.cs @@ -317,6 +317,7 @@ internal bool RaiseOpenUriRequested(System.Uri uri) // LoadedAssembly.Text -> metadata, which AVs when an assembly was unloaded // between selection and the title update. string cachedBaseTitle = "(unnamed)"; + int titleVersion; // IsStaticContent is inherited from ContentPageModel: true for static pages (e.g. About) // excludes this tab from the "current decompile target" lookup, so later tree-node @@ -372,6 +373,30 @@ public IReadOnlyList CurrentNodes { } } + internal void ClearContent() + { + titleVersion++; + activeCts?.Cancel(); + foreach (var n in currentNodes) + n.PropertyChanged -= OnCurrentNodePropertyChanged; + currentNodes = System.Array.Empty(); + cachedBaseTitle = ComposeBaseTitle(); + Title = cachedBaseTitle; + HighlightingModel = null; + HighlightingSpans = null; + Foldings = null; + References = null; + DefinitionLookup = null; + DebugInfo = null; + DebugStepHighlight = null; + UIElements = null; + Text = string.Empty; + IsDecompiling = false; + TaskbarProgress?.SetState(TaskbarProgressState.None); + OnPropertyChanged(nameof(CurrentNodes)); + OnPropertyChanged(nameof(CurrentNode)); + } + void OnCurrentNodePropertyChanged(object? sender, PropertyChangedEventArgs e) { // Tree-node Text can change after we capture the title (e.g. AssemblyTreeNode swaps @@ -531,10 +556,12 @@ async Task DecompileAsync() using var _phase = ICSharpCode.ILSpy.AppEnv.AppLog.Phase($"DecompileAsync #{callNumber}"); activeCts?.Cancel(); var cts = activeCts = new CancellationTokenSource(); + int requestTitleVersion = ++titleVersion; var nodes = currentNodes; var language = Language; if (nodes.Count == 0 || language == null) { + Title = cachedBaseTitle; // Clear the per-decompile artefacts alongside Text. If we leave Foldings / // HighlightingModel / References pointing at the previous decompile's state, // the next ApplyDocument fires (via SyntaxExtension or Text setter) tries to @@ -562,7 +589,7 @@ async Task DecompileAsync() // editor state is left untouched so cancellation falls back cleanly. Title = ComposeSpinnerTitle(0, cachedBaseTitle); TaskbarProgress?.SetState(TaskbarProgressState.Indeterminate); - _ = RunSpinnerAsync(cts.Token); + _ = RunSpinnerAsync(cts.Token, requestTitleVersion); try { @@ -654,7 +681,7 @@ await Dispatcher.UIThread.InvokeAsync(() => { // that window (the assembly was removed from the list, the user selected // something else) must not let the finished output overwrite whatever state // the tab has been put into since. - if (cts.Token.IsCancellationRequested) + if (cts.Token.IsCancellationRequested || requestTitleVersion != titleVersion) return; Title = cachedBaseTitle; ApplyOutput(output, effectiveSyntaxExtension, rendered); @@ -670,7 +697,7 @@ await Dispatcher.UIThread.InvokeAsync(() => { // "Decompiling…" overlay is far worse than leaving the previous output visible. // Skip the reset if a newer request has already taken over (activeCts is rotated // at the top of DecompileAsync). - if (ReferenceEquals(activeCts, cts)) + if (ReferenceEquals(activeCts, cts) && requestTitleVersion == titleVersion) { void StopSpinner() { @@ -697,10 +724,10 @@ void StopSpinner() static string ComposeSpinnerTitle(int frame, string baseTitle) => $"{SpinnerFrames[frame % SpinnerFrames.Length]} {baseTitle}"; - async Task RunSpinnerAsync(CancellationToken token) + async Task RunSpinnerAsync(CancellationToken token, int spinnerTitleVersion) { int frame = 1; - while (!token.IsCancellationRequested) + while (!token.IsCancellationRequested && spinnerTitleVersion == titleVersion) { try { @@ -710,7 +737,7 @@ async Task RunSpinnerAsync(CancellationToken token) { return; } - if (token.IsCancellationRequested || !IsDecompiling) + if (token.IsCancellationRequested || !IsDecompiling || spinnerTitleVersion != titleVersion) return; Title = ComposeSpinnerTitle(frame++, cachedBaseTitle); } @@ -731,6 +758,7 @@ public async Task RunWithCancellation( ArgumentNullException.ThrowIfNull(taskCreation); activeCts?.Cancel(); var cts = activeCts = new CancellationTokenSource(); + int requestTitleVersion = ++titleVersion; ProgressTitle = progressTitle ?? ICSharpCode.ILSpy.Properties.Resources.Decompiling; ResetProgress(); IsDecompiling = true; @@ -739,14 +767,14 @@ public async Task RunWithCancellation( // strip advertises the running work, exactly like an in-place decompile does. cachedBaseTitle = Title; Title = ComposeSpinnerTitle(0, cachedBaseTitle); - _ = RunSpinnerAsync(cts.Token); + _ = RunSpinnerAsync(cts.Token, requestTitleVersion); try { return await taskCreation(cts.Token).ConfigureAwait(true); } finally { - if (ReferenceEquals(activeCts, cts)) + if (ReferenceEquals(activeCts, cts) && requestTitleVersion == titleVersion) { void StopSpinner() {