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..f1629c0ce 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,6 +73,76 @@ 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() { 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/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);