diff --git a/ICSharpCode.ILSpyX/AssemblyListSnapshot.cs b/ICSharpCode.ILSpyX/AssemblyListSnapshot.cs index 5c520fc83..253c4174d 100644 --- a/ICSharpCode.ILSpyX/AssemblyListSnapshot.cs +++ b/ICSharpCode.ILSpyX/AssemblyListSnapshot.cs @@ -207,7 +207,7 @@ namespace ICSharpCode.ILSpyX } else if (result.Package != null) { - foreach (var descendant in EnumerateDescendants(result.Package.RootFolder)) + foreach (var descendant in EnumerateDescendants(result.Package.RootFolder, cancellationToken)) { yield return descendant; } @@ -218,11 +218,11 @@ namespace ICSharpCode.ILSpyX } } - static IEnumerable EnumerateDescendants(PackageFolder folder) + static IEnumerable EnumerateDescendants(PackageFolder folder, CancellationToken cancellationToken) { foreach (var subFolder in folder.Folders) { - foreach (var descendant in EnumerateDescendants(subFolder)) + foreach (var descendant in EnumerateDescendants(subFolder, cancellationToken)) { yield return descendant; } @@ -230,12 +230,15 @@ namespace ICSharpCode.ILSpyX foreach (var entry in folder.Entries) { + // Checked per entry, not per package: resolving one starts extracting it from + // the archive, so a walk the consumer has given up on must stop expanding. + cancellationToken.ThrowIfCancellationRequested(); if (!entry.Name.EndsWith(".dll", StringComparison.OrdinalIgnoreCase) && !entry.Name.EndsWith(".exe", StringComparison.OrdinalIgnoreCase)) continue; LoadedAssembly? asm; try { - asm = folder.ResolveFileName(entry.Name); + asm = folder.ResolveEntry(entry); } catch { diff --git a/ICSharpCode.ILSpyX/LoadedPackage.cs b/ICSharpCode.ILSpyX/LoadedPackage.cs index 84793c800..dfeecf797 100644 --- a/ICSharpCode.ILSpyX/LoadedPackage.cs +++ b/ICSharpCode.ILSpyX/LoadedPackage.cs @@ -349,35 +349,58 @@ namespace ICSharpCode.ILSpyX return Task.FromResult(null); } - readonly Dictionary assemblies = new Dictionary(StringComparer.OrdinalIgnoreCase); + // Keyed by entry name, ordinal: an assembly-reference lookup is case-insensitive, but two + // entries whose names differ only in case are two distinct files (archive entry names are + // case-sensitive) and must not share one LoadedAssembly. + readonly Dictionary assemblies = new Dictionary(StringComparer.Ordinal); public LoadedAssembly? ResolveFileName(string name) { + var entry = Entries.FirstOrDefault(e => string.Equals(name, e.Name, StringComparison.Ordinal)) + ?? Entries.FirstOrDefault(e => string.Equals(name, e.Name, StringComparison.OrdinalIgnoreCase)); + return entry == null ? null : ResolveEntry(entry); + } + + /// + /// The for one of this folder's entries, created on first use. + /// Creating it starts reading the entry out of the package, so call this only for entries + /// that are about to be inspected. + /// + public LoadedAssembly? ResolveEntry(PackageEntry entry) + { + ArgumentNullException.ThrowIfNull(entry); if (package.LoadedAssembly == null) return null; lock (assemblies) { - if (assemblies.TryGetValue(name, out var asm)) + if (assemblies.TryGetValue(entry.Name, out var asm)) return asm; - var entry = Entries.FirstOrDefault(e => string.Equals(name, e.Name, StringComparison.OrdinalIgnoreCase)); - if (entry != null) - { - asm = new LoadedAssembly( - package.LoadedAssembly, entry.Name, - fileLoaders: package.LoadedAssembly.AssemblyList.LoaderRegistry, - assemblyResolver: this, - stream: Task.Run(entry.TryOpenStream), - applyWinRTProjections: package.LoadedAssembly.AssemblyList.ApplyWinRTProjections, - useDebugSymbols: package.LoadedAssembly.AssemblyList.UseDebugSymbols - ); - } - else - { - asm = null; - } - assemblies.Add(name, asm); + // FullName is the package-relative path ("lib/net10.0/Foo.dll"), which is what makes + // the copies of one assembly in a multi-target package tellable apart wherever the + // file name is surfaced. ShortName/Text stay the bare file name either way. + asm = new LoadedAssembly( + package.LoadedAssembly, entry.FullName, + fileLoaders: package.LoadedAssembly.AssemblyList.LoaderRegistry, + assemblyResolver: this, + stream: Task.Run(entry.TryOpenStream), + applyWinRTProjections: package.LoadedAssembly.AssemblyList.ApplyWinRTProjections, + useDebugSymbols: package.LoadedAssembly.AssemblyList.UseDebugSymbols + ); + assemblies.Add(entry.Name, asm); return asm; } } + + /// + /// Whether this folder has already resolved from one of its + /// entries. Consults the resolution cache only -- nothing is loaded or extracted. + /// + public bool HasResolved(LoadedAssembly assembly) + { + lock (assemblies) + { + return assemblies.ContainsValue(assembly); + } + } } } diff --git a/ILSpy.Tests/AssemblyTree/AssemblyTreeModelTests.cs b/ILSpy.Tests/AssemblyTree/AssemblyTreeModelTests.cs index ceda979af..c7e9f879e 100644 --- a/ILSpy.Tests/AssemblyTree/AssemblyTreeModelTests.cs +++ b/ILSpy.Tests/AssemblyTree/AssemblyTreeModelTests.cs @@ -17,9 +17,11 @@ // DEALINGS IN THE SOFTWARE. using System; +using System.Collections.Generic; using System.IO; using System.IO.Compression; using System.Linq; +using System.Threading; using System.Threading.Tasks; using Avalonia.Headless.NUnit; @@ -71,30 +73,119 @@ public class AssemblyTreeModelTests "Initialize selects the (Default) list so the tree has something to render at startup."); } + static readonly string TempRoot = Path.Combine(Path.GetTempPath(), "ILSpy.Tests.Packages", Guid.NewGuid().ToString("N")); + + // Two assemblies in sibling folder chains, so a walk that reaches only one of them fails. + static string CreatePackage() + { + var dir = Directory.CreateDirectory(Path.Combine(TempRoot, Guid.NewGuid().ToString("N"))).FullName; + var zipPath = Path.Combine(dir, "package.zip"); + using var zip = ZipFile.Open(zipPath, ZipArchiveMode.Create); + zip.CreateEntryFromFile(FixtureAssembly.Emit("Nested"), "lib/net10.0/Nested.dll"); + zip.CreateEntryFromFile(FixtureAssembly.Emit("Sibling"), "runtimes/win-x64/Sibling.dll"); + return zipPath; + } + + [OneTimeTearDown] + public void DeleteTempPackages() + { + if (Directory.Exists(TempRoot)) + Directory.Delete(TempRoot, recursive: true); + } + + [AvaloniaTest] + public async Task EnumerateAllAssemblies_expands_a_package_into_its_entries() + { + // The search corpus is this walk, so an assembly it does not yield is an assembly no + // search can ever match. + var (_, vm) = await TestHarness.BootAsync(); + await vm.OpenAssemblyAsync(CreatePackage()); + + var nested = new List(); + await foreach (var asm in vm.AssemblyTreeModel.AssemblyList!.EnumerateAllAssemblies()) + { + if (asm.ParentBundle != null) + nested.Add(asm); + } + + nested.Select(a => a.FileName).Should().BeEquivalentTo( + new[] { "lib/net10.0/Nested.dll", "runtimes/win-x64/Sibling.dll" }, + "every .dll in the package is searchable, and the package-relative path is what " + + "distinguishes copies of one assembly built for several targets."); + } + + [AvaloniaTest] + public async Task EnumerateAllAssemblies_stops_expanding_a_package_once_cancelled() + { + // Both search panes restart on every keystroke, so an abandoned walk that keeps + // extracting package entries competes with the run the user is waiting for. + var (_, vm) = await TestHarness.BootAsync(); + await vm.OpenAssemblyAsync(CreatePackage()); + + using var cts = new CancellationTokenSource(); + var walk = vm.AssemblyTreeModel.AssemblyList!.EnumerateAllAssemblies(cts.Token).GetAsyncEnumerator(); + try + { + while (await walk.MoveNextAsync() && walk.Current.ParentBundle == null) + { + // Skip past the list's own assemblies to the package's first entry. + } + walk.Current.ParentBundle.Should().NotBeNull("the package's entries come last on the list."); + cts.Cancel(); + + var next = async () => await walk.MoveNextAsync(); + await next.Should().ThrowAsync( + "the second entry must not be extracted after the walk was cancelled."); + } + finally + { + await walk.DisposeAsync(); + } + } + [AvaloniaTest] public async Task FindTreeNode_resolves_a_type_inside_an_unexpanded_package() { // Search enumerates the contents of packages whether or not the user ever opened them in // the tree, so activating such a result has to reach a node that does not exist yet. var (_, vm) = await TestHarness.BootAsync(); + await vm.OpenAssemblyAsync(CreatePackage()); + + var nested = (await vm.AssemblyTreeModel.AssemblyList!.GetAllAssemblies()) + .Single(a => a.FileName == "lib/net10.0/Nested.dll"); + var type = nested.GetTypeSystemOrNull()!.MainModule.TypeDefinitions + .Single(t => t.Name == FixtureAssembly.TypeName); - var tempDir = Path.Combine(Path.GetTempPath(), "ILSpy.Tests", Guid.NewGuid().ToString("N")); - Directory.CreateDirectory(tempDir); - var zipPath = Path.Combine(tempDir, "package.zip"); - using (var zip = ZipFile.Open(zipPath, ZipArchiveMode.Create)) - zip.CreateEntryFromFile(FixtureAssembly.Emit("Nested"), "lib/net10.0/Nested.dll"); + var node = vm.AssemblyTreeModel.FindTreeNode(type); - await vm.OpenAssemblyAsync(zipPath); + // Assert the owning module, not just the node type: the fixture's type handle is + // 0x02000002, which resolves in nearly every assembly on the list, so a lookup that fell + // back to the first top-level node would still hand back some TypeTreeNode. + ((object?)node).Should().BeOfType() + .Which.Module.Should().BeSameAs(nested.GetMetadataFileOrNull(), + "the lookup must descend into the package's folders, expanding them on the way."); + } + + [AvaloniaTest] + public async Task FindTreeNode_leaves_package_folders_off_the_path_unexpanded() + { + // Expanding a package folder resolves and extracts every .dll it holds, so a lookup that + // swept the package depth-first would pay for entries the user never asked about. + var (_, vm) = await TestHarness.BootAsync(); + var package = await vm.OpenAssemblyAsync(CreatePackage()); var nested = (await vm.AssemblyTreeModel.AssemblyList!.GetAllAssemblies()) - .Single(a => a.ParentBundle != null); + .Single(a => a.FileName == "lib/net10.0/Nested.dll"); var type = nested.GetTypeSystemOrNull()!.MainModule.TypeDefinitions .Single(t => t.Name == FixtureAssembly.TypeName); - var node = vm.AssemblyTreeModel.FindTreeNode(type); + ((object?)vm.AssemblyTreeModel.FindTreeNode(type)).Should().NotBeNull(); - // Cast through object so the generic Should() resolves, not the SharpTreeNode shadow. - ((object?)node).Should().BeOfType( - "the lookup must descend into the package's folders, expanding them on the way"); + var packageNode = vm.AssemblyTreeModel.FindAssemblyNode(package); + ((object?)packageNode).Should().NotBeNull(); + var sibling = packageNode!.Children.OfType() + .Single(f => f.Text as string == "runtimes/win-x64"); + sibling.Children.Should().BeEmpty( + "only the folders on the path down to the target get expanded."); } } diff --git a/ILSpy/AssemblyTree/TreeNodeLocator.cs b/ILSpy/AssemblyTree/TreeNodeLocator.cs index 3b2fc49a0..872c72560 100644 --- a/ILSpy/AssemblyTree/TreeNodeLocator.cs +++ b/ILSpy/AssemblyTree/TreeNodeLocator.cs @@ -97,7 +97,7 @@ namespace ICSharpCode.ILSpy.AssemblyTree return FindMemberNode(root, member); case LoadedAssembly lasm: - return root.FindAssemblyNode(lasm); + return FindAssemblyNode(root, lasm); case MetadataFile metadataFile: return FindAssemblyNode(root, metadataFile); @@ -122,13 +122,15 @@ namespace ICSharpCode.ILSpy.AssemblyTree /// down, so this resolves even when the user has never opened the package in the tree. /// public static AssemblyTreeNode? FindAssemblyNode(AssemblyListTreeNode root, MetadataFile? module) + => FindAssemblyNode(root, module?.GetLoadedAssemblyOrNull()); + + /// + public static AssemblyTreeNode? FindAssemblyNode(AssemblyListTreeNode root, LoadedAssembly? assembly) { - // A package child records the bundle it came from, so walking up that chain leads - // straight to the one top-level node worth descending into. Searching the tree for a - // matching module instead would have to visit every namespace and type node already - // built, and would still miss nested assemblies that are not loaded yet. + // A package child records the bundle it came from, so walking up that chain names every + // package to descend into, outermost first, ending at the one top-level node. var nesting = new Stack(); - for (var current = module?.GetLoadedAssemblyOrNull(); current != null; current = current.ParentBundle) + for (var current = assembly; current != null; current = current.ParentBundle) nesting.Push(current); if (nesting.Count == 0) return null; @@ -139,22 +141,49 @@ namespace ICSharpCode.ILSpy.AssemblyTree return node; } - // Depth-first search for one assembly within a package node's folder structure. Only - // package folders are descended into, so the walk stays inside the package. - static AssemblyTreeNode? FindNestedAssemblyNode(SharpTreeNode packageNode, LoadedAssembly assembly) + // Finds one assembly inside a package node, expanding only the folders on the path down to + // it. Expanding a package folder resolves and extracts every .dll/.exe it holds, so the + // path is taken from the package's in-memory folder graph first -- that costs no tree node + // and reads no package entry. + static AssemblyTreeNode? FindNestedAssemblyNode(AssemblyTreeNode packageNode, LoadedAssembly assembly) { - packageNode.EnsureLazyChildren(); - foreach (var child in packageNode.Children) + var rootFolder = packageNode.LoadedAssembly.GetLoadResultAsync().GetAwaiter().GetResult().Package?.RootFolder; + if (rootFolder == null) + return null; + var path = new HashSet(); + if (!rootFolder.HasResolved(assembly) && !CollectFolderPath(rootFolder, assembly, path)) + return null; + + SharpTreeNode node = packageNode; + while (true) { - switch (child) + node.EnsureLazyChildren(); + if (node.Children.OfType().FirstOrDefault(n => n.LoadedAssembly == assembly) is { } nested) + return nested; + // A folder node stands for the deepest link of a collapsed single-child chain + // (a/b/c shows as one node), so match its folder against the whole path rather + // than against one expected next segment. + if (node.Children.OfType().FirstOrDefault(f => path.Contains(f.Folder)) is not { } next) + return null; + node = next; + } + } + + // Fills with the folders between (exclusive) + // and the one that already resolved . Every package-nested + // LoadedAssembly is produced by exactly one PackageFolder.ResolveEntry call, so the owning + // folder's resolution cache is what identifies it. + static bool CollectFolderPath(PackageFolder folder, LoadedAssembly assembly, HashSet path) + { + foreach (var subFolder in folder.Folders) + { + if (subFolder.HasResolved(assembly) || CollectFolderPath(subFolder, assembly, path)) { - case AssemblyTreeNode nested when nested.LoadedAssembly == assembly: - return nested; - case PackageFolderTreeNode folder when FindNestedAssemblyNode(folder, assembly) is { } found: - return found; + path.Add(subFolder); + return true; } } - return null; + return false; } // Resolves a resource (optionally a named sub-entry) to its tree node. Mirrors the previous @@ -185,19 +214,16 @@ namespace ICSharpCode.ILSpy.AssemblyTree return resourceNode.Children.OfType().FirstOrDefault(x => name.Equals(x.Text)) ?? resourceNode; } - // Resolves a namespace to its tree node within its contributing assembly. Mirrors the previous - // version's AssemblyListTreeNode.FindNamespaceNode. + // Resolves a namespace to its tree node within its contributing assembly. static NamespaceTreeNode? FindNamespaceNode(AssemblyListTreeNode root, INamespace ns) { var module = ns.ContributingModules.FirstOrDefault(); if (module?.MetadataFile == null) return null; - var assembly = FindAssemblyNode(root, module.MetadataFile); - if (assembly == null) - return null; - assembly.EnsureLazyChildren(); - return assembly.Children.OfType() - .FirstOrDefault(n => ns.FullName.Length == 0 || ns.FullName.Equals(n.Text)); + // The assembly node indexes every namespace it built by full, unescaped name. Matching + // against the node labels instead would fail in nested-namespace mode, where the node + // for "A.B.C" is a descendant and its label is only the last segment. + return FindAssemblyNode(root, module.MetadataFile)?.FindNamespaceNode(ns.FullName); } public static TypeTreeNode? FindTypeNode(AssemblyListTreeNode root, ITypeDefinition type) diff --git a/ILSpy/Controls/Omnibar/Omnibar.axaml.cs b/ILSpy/Controls/Omnibar/Omnibar.axaml.cs index c194d05c4..5deac32ff 100644 --- a/ILSpy/Controls/Omnibar/Omnibar.axaml.cs +++ b/ILSpy/Controls/Omnibar/Omnibar.axaml.cs @@ -21,6 +21,7 @@ using System.Collections.Specialized; using System.ComponentModel; using System.Linq; +using Avalonia; using Avalonia.Controls; using Avalonia.Input; using Avalonia.Threading; @@ -69,6 +70,14 @@ namespace ICSharpCode.ILSpy.Controls.Omnibar }); } + protected override void OnDetachedFromVisualTree(VisualTreeAttachmentEventArgs e) + { + // The hosting tab is gone (closed, or its content swapped out). Nothing will read the + // results, so end the run rather than let it walk the rest of the assembly list. + viewModel.CancelSearch(); + base.OnDetachedFromVisualTree(e); + } + void OnViewModelPropertyChanged(object? sender, PropertyChangedEventArgs e) { if (e.PropertyName == nameof(OmnibarViewModel.Mode)) diff --git a/ILSpy/Controls/Omnibar/OmnibarViewModel.cs b/ILSpy/Controls/Omnibar/OmnibarViewModel.cs index 3486429f0..33ab000cb 100644 --- a/ILSpy/Controls/Omnibar/OmnibarViewModel.cs +++ b/ILSpy/Controls/Omnibar/OmnibarViewModel.cs @@ -191,13 +191,23 @@ namespace ICSharpCode.ILSpy.Controls.Omnibar RunningSearch? currentSearch; - void RestartSearch() + /// + /// Ends the search in flight, if any. A run holds the assembly list and keeps expanding + /// packages until it finishes, so a bar that is going away has to stop its run instead of + /// leaving it to drain into a sink nobody can see. + /// + public void CancelSearch() { currentSearch?.Cancel(); currentSearch = null; + IsSearching = false; + } + + void RestartSearch() + { + CancelSearch(); searchSink.Clear(); Suggestions.Clear(); - IsSearching = false; var term = SearchText ?? string.Empty; if (string.IsNullOrWhiteSpace(term)) diff --git a/ILSpy/Metadata/MetadataNavigator.cs b/ILSpy/Metadata/MetadataNavigator.cs index 34680970e..ae46ebf97 100644 --- a/ILSpy/Metadata/MetadataNavigator.cs +++ b/ILSpy/Metadata/MetadataNavigator.cs @@ -71,10 +71,6 @@ namespace ICSharpCode.ILSpy.Metadata if (!TryReadRow(row, "Token", out var file, out var handle) || handle.IsNil) return null; - var owningAssembly = assemblyTreeModel.AssemblyList?.GetAssemblies() - .FirstOrDefault(a => ReferenceEquals(a.GetMetadataFileOrNull(), file)); - if (owningAssembly is null) - return null; if (file?.GetTypeSystemWithCurrentOptionsOrNull()?.MainModule is not MetadataModule metadataModule) return null; IEntity? entity; @@ -93,10 +89,9 @@ namespace ICSharpCode.ILSpy.Metadata /// public MetadataTableTreeNode? FindTableNode(MetadataTokenReference reference) { - var targetAssembly = assemblyTreeModel.Root?.Children - .OfType() - .FirstOrDefault(a => ReferenceEquals(a.LoadedAssembly.GetMetadataFileOrNull(), reference.MetadataFile)); - if (targetAssembly == null) + // FindTreeNode, not a scan of the root's children: the module may sit inside a package + // or bundle, whose assembly nodes are grandchildren of a folder node. + if (assemblyTreeModel.FindTreeNode(reference.MetadataFile) is not AssemblyTreeNode targetAssembly) return null; targetAssembly.EnsureLazyChildren(); var metaNode = targetAssembly.Children.OfType().FirstOrDefault(); diff --git a/ILSpy/Metadata/MetadataProtocolHandler.cs b/ILSpy/Metadata/MetadataProtocolHandler.cs index af1598e84..524de3943 100644 --- a/ILSpy/Metadata/MetadataProtocolHandler.cs +++ b/ILSpy/Metadata/MetadataProtocolHandler.cs @@ -52,13 +52,10 @@ namespace ICSharpCode.ILSpy.Metadata newTabPage = true; if (protocol != "metadata") return null; - // AssemblyTreeModel.FindTreeNode only resolves EntityReference/ITypeDefinition/IMember, - // not MetadataFile — walk the assembly-tree root manually here. Same lookup pattern - // FindTypeNode uses internally. - var assemblyNode = (assemblyTreeModel.Root as AssemblyListTreeNode)?.Children - .OfType() - .FirstOrDefault(a => a.LoadedAssembly.GetMetadataFileOrNull() == module); - if (assemblyNode == null) + // FindTreeNode resolves a MetadataFile to its assembly node, including one nested in a + // package or bundle, where the node is a grandchild of a folder node rather than a + // child of the root. + if (assemblyTreeModel.FindTreeNode(module) is not AssemblyTreeNode assemblyNode) return null; assemblyNode.EnsureLazyChildren(); var metadataNode = assemblyNode.Children.OfType().FirstOrDefault(); diff --git a/ILSpy/Search/RunningSearch.cs b/ILSpy/Search/RunningSearch.cs index 2801571fd..66cc06251 100644 --- a/ILSpy/Search/RunningSearch.cs +++ b/ILSpy/Search/RunningSearch.cs @@ -131,15 +131,9 @@ namespace ICSharpCode.ILSpy.Search var strategy = GetStrategy(request); if (strategy == null) return; - // Streaming enumeration: expanding bundles/packages into their contained - // assemblies triggers the lazy load of every entry on the list, so awaiting - // the whole set up front would mean no results at all until the last - // assembly is off disk. Enumerating starts the walk here on the worker - // thread, not at construction time on the UI thread. - // - // Serial walk: per-assembly metadata walk is allocation-dominated, and 4 - // parallel producers fighting for the ConcurrentQueue + the resulting UI - // batching jitter end up slower than serial in practice. + // The per-assembly metadata walk is allocation-dominated, and 4 parallel + // producers fighting for the ConcurrentQueue + the resulting UI batching + // jitter end up slower than walking the assemblies one at a time. await foreach (var assembly in assemblyList.EnumerateAllAssemblies(ct).ConfigureAwait(false)) { if (ct.IsCancellationRequested) diff --git a/ILSpy/TreeNodes/PackageFolderTreeNode.cs b/ILSpy/TreeNodes/PackageFolderTreeNode.cs index 08891b055..fe3b2f6d9 100644 --- a/ILSpy/TreeNodes/PackageFolderTreeNode.cs +++ b/ILSpy/TreeNodes/PackageFolderTreeNode.cs @@ -88,7 +88,7 @@ namespace ICSharpCode.ILSpy.TreeNodes if (entry.Name.EndsWith(".dll", StringComparison.OrdinalIgnoreCase) || entry.Name.EndsWith(".exe", StringComparison.OrdinalIgnoreCase)) { - var asm = root.ResolveFileName(entry.Name); + var asm = root.ResolveEntry(entry); if (asm != null) { yield return new AssemblyTreeNode(asm, entry);