Resolve package entries by identity and end walks nobody reads

Searching a package resolves its entries by file name against a case-insensitive
cache, which is right for an assembly reference but wrong for an archive entry:
two entries differing only in case are two files, and they collapsed onto one
LoadedAssembly, so one was searched twice and the other never. Keying the cache
by the entry itself separates them, and the entry's package-relative path
becomes the assembly's file name, which is what tells the copies of one assembly
in a multi-target package apart wherever a search result shows a location.

Cancellation was only checked between top-level list entries, so a walk the user
had already replaced by typing another character kept extracting package entries
alongside the run they were waiting for. The omnibar had no way to end its run at
all: its view model is per document tab and nothing cancelled it when the tab
went away.

Assisted-by: Claude:claude-opus-5[1m]:Claude Code

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<LoadedAssembly> EnumerateDescendants(PackageFolder folder)
+			static IEnumerable<LoadedAssembly> 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
 					{

ICSharpCode.ILSpyX/LoadedPackage.cs

@@ -349,35 +349,58 @@ namespace ICSharpCode.ILSpyX
 			return Task.FromResult<MetadataFile?>(null);
 		}
 
-		readonly Dictionary<string, LoadedAssembly?> assemblies = new Dictionary<string, LoadedAssembly?>(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<string, LoadedAssembly> assemblies = new Dictionary<string, LoadedAssembly>(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);
+		}
+
+		/// <summary>
+		/// The <see cref="LoadedAssembly"/> 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.
+		/// </summary>
+		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;
 			}
 		}
+
+		/// <summary>
+		/// Whether this folder has already resolved <paramref name="assembly"/> from one of its
+		/// entries. Consults the resolution cache only -- nothing is loaded or extracted.
+		/// </summary>
+		public bool HasResolved(LoadedAssembly assembly)
+		{
+			lock (assemblies)
+			{
+				return assemblies.ContainsValue(assembly);
+			}
+		}
 	}
 }

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<LoadedAssembly>();
+		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<OperationCanceledException>(
+				"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()
 	{

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))

ILSpy/Controls/Omnibar/OmnibarViewModel.cs

@@ -191,13 +191,23 @@ namespace ICSharpCode.ILSpy.Controls.Omnibar
 
 		RunningSearch? currentSearch;
 
-		void RestartSearch()
+		/// <summary>
+		/// 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.
+		/// </summary>
+		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))

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)

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);