diff --git a/ILSpy.Tests.Windows/Processes/NetFrameworkProcessesTests.cs b/ILSpy.Tests.Windows/Processes/NetFrameworkProcessesTests.cs new file mode 100644 index 000000000..35e8ece8a --- /dev/null +++ b/ILSpy.Tests.Windows/Processes/NetFrameworkProcessesTests.cs @@ -0,0 +1,214 @@ +// Copyright (c) 2026 Christoph Wille +// +// 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; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; + +using AwesomeAssertions; + +using ICSharpCode.ILSpy.Processes; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests.Windows.Processes; + +/// +/// The .NET Framework half of the process explorer, which exists on Windows only: those +/// processes predate the diagnostics endpoint every CoreCLR process serves, so they are found +/// by the desktop CLR in their OS module list and their assemblies are read from that same +/// list. Windows PowerShell is the fixture - a .NET Framework 4.x application present on +/// every Windows installation, including the CI runner. +/// +[TestFixture] +[Platform("Win", Reason = ".NET Framework and the OS module list this path reads exist on Windows only.")] +public class NetFrameworkProcessesTests +{ + static Process? host; + + static string WindowsPowerShellPath => Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.System), + "WindowsPowerShell", "v1.0", "powershell.exe"); + + [OneTimeSetUp] + public void StartADotNetFrameworkProcess() + { + File.Exists(WindowsPowerShellPath).Should().BeTrue( + "Windows PowerShell 5.1 is the .NET Framework process this fixture inspects"); + + host = Process.Start(new ProcessStartInfo(WindowsPowerShellPath, + "-NoProfile -NonInteractive -Command \"Start-Sleep -Seconds 300\"") { + UseShellExecute = false, + CreateNoWindow = true, + }); + host.Should().NotBeNull(); + + // The runtime is loaded a moment after the process exists, and nothing about this path + // works before it is. + WaitUntilTheDesktopClrIsLoaded(host!); + } + + [OneTimeTearDown] + public void StopIt() + { + if (host is { HasExited: false }) + host.Kill(entireProcessTree: true); + host?.Dispose(); + } + + static void WaitUntilTheDesktopClrIsLoaded(Process process) + { + var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(60); + while (DateTime.UtcNow < deadline) + { + process.Refresh(); + if (!process.HasExited && LoadedModuleNames(process).Contains("clr.dll")) + return; + Thread.Sleep(100); + } + Assert.Fail("Windows PowerShell did not load clr.dll within a minute."); + } + + static HashSet LoadedModuleNames(Process process) + { + try + { + return process.Modules.Cast() + .Select(m => m.ModuleName) + .ToHashSet(StringComparer.OrdinalIgnoreCase); + } + catch (Exception ex) when (ex is System.ComponentModel.Win32Exception or InvalidOperationException) + { + return new HashSet(StringComparer.OrdinalIgnoreCase); + } + } + + static RunningDotNetProcess TheHostAsListed(ISet? alreadyListed = null) + { + var listed = NetFrameworkProcesses.Enumerate( + alreadyListed ?? new HashSet(), CancellationToken.None).ToList(); + return listed.Should().ContainSingle(p => p.Pid == host!.Id, + "a running .NET Framework process is what this path exists to find").Subject; + } + + [Test] + public void A_Desktop_Clr_Process_Is_Found_And_Described() + { + var listed = TheHostAsListed(); + + listed.Kind.Should().Be(RuntimeKind.NetFramework); + listed.ProcessName.Should().Be("powershell"); + listed.RuntimeVersion.Should().StartWith("4.", "the desktop CLR in use is version 4"); + listed.Architecture.Should().Be(Environment.Is64BitOperatingSystem ? "x64" : "x86", + "the architecture follows from which Framework directory the CLR was loaded out of"); + // Unlike a modern app, a .NET Framework executable is itself managed - there is no + // native host in front of it, so the entry assembly is the exe. + listed.EntryAssemblyName.Should().Be("powershell"); + } + + [Test] + public void Processes_The_Diagnostics_Scan_Already_Listed_Are_Not_Listed_Twice() + { + // The two halves of the explorer are merged, and a process that answered on the + // diagnostics endpoint must not appear again from the module scan. + var listed = NetFrameworkProcesses.Enumerate( + new HashSet { host!.Id }, CancellationToken.None); + + listed.Should().NotContain(p => p.Pid == host!.Id); + } + + [Test] + public void Only_The_Managed_Modules_Of_A_Desktop_Clr_Process_Are_Listed() + { + var modules = NetFrameworkProcesses.GetModules(host!.Id); + + modules.Should().Contain(m => IsAssembly(m, "System.Management.Automation"), + "PowerShell's own assembly is loaded from the GAC, not from beside the exe"); + + // The OS module list mixes native libraries in, and clr.dll is the very entry that + // identified the process - so this proves the filtering runs, not merely that it exists. + LoadedModuleNames(host!).Should().Contain("clr.dll"); + modules.Should().NotContain(m => string.Equals(m.Name, "clr.dll", StringComparison.OrdinalIgnoreCase)); + + // Stated as the property itself rather than as a list of names to keep out: any native + // library the filter starts admitting fails this, not just the ones thought of here. + foreach (var module in modules) + { + module.IsInMemory.Should().BeFalse("everything on this path comes from a file"); + ProcessExplorer.IsManagedAssembly(module.Path!).Should().BeTrue( + $"'{module.Name}' was listed as an assembly loaded in the process"); + } + } + + [Test] + public void The_Core_Library_Is_Listed_However_The_Runtime_Loaded_It() + { + var modules = NetFrameworkProcesses.GetModules(host!.Id); + + var mscorlib = modules.Should().ContainSingle(m => IsAssembly(m, "mscorlib"), + "every .NET Framework process has the core library loaded").Subject; + + // Normally this is mscorlib.ni.dll out of the NGen cache rather than the IL assembly + // in the GAC, because Windows ships pre-compiled native images for the framework. The + // image carries metadata and is genuinely the module the process loaded, so it is + // listed - but its IL lives in the assembly it was compiled from, and the OS module + // list does not say where that is. Opening a native image is therefore of limited use, + // which is the fidelity gap of this path rather than a defect in it. + ProcessExplorer.IsManagedAssembly(mscorlib.Path!).Should().BeTrue(); + File.Exists(mscorlib.Path).Should().BeTrue(); + } + + /// + /// Whether a listed module is the given assembly, in either of the two forms the desktop + /// loader reports: the IL assembly, or the NGen native image compiled from it. + /// + static bool IsAssembly(ProcessModuleInfo module, string simpleName) + => string.Equals(module.Name, simpleName + ".dll", StringComparison.OrdinalIgnoreCase) + || string.Equals(module.Name, simpleName + ".ni.dll", StringComparison.OrdinalIgnoreCase); + + [Test] + public async Task The_Explorer_Routes_A_Framework_Process_To_The_Module_Scan() + { + // The facade decides per process which mechanism answers; a .NET Framework process + // must not be sent down the diagnostics path, where it has no endpoint at all. + var explorer = new ProcessExplorer(); + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(60)); + var processes = await explorer.GetProcessesAsync(cts.Token); + var listed = processes.Should().ContainSingle(p => p.Pid == host!.Id).Subject; + listed.Kind.Should().Be(RuntimeKind.NetFramework); + + var modules = await explorer.GetModulesAsync(listed, cts.Token); + + modules.Should().Contain(m => IsAssembly(m, "mscorlib")); + } + + [Test] + public void A_Process_That_Has_Exited_Yields_No_Modules() + { + using var shortLived = Process.Start(new ProcessStartInfo(WindowsPowerShellPath, + "-NoProfile -NonInteractive -Command \"exit\"") { UseShellExecute = false, CreateNoWindow = true })!; + shortLived.WaitForExit(); + + NetFrameworkProcesses.GetModules(shortLived.Id).Should().BeEmpty( + "a process that exits mid-scan is skipped, not reported as an error"); + } +} diff --git a/ILSpy.Tests/Processes/DiagnosticsIpcClientTests.cs b/ILSpy.Tests/Processes/DiagnosticsIpcClientTests.cs index 925276993..a827305ca 100644 --- a/ILSpy.Tests/Processes/DiagnosticsIpcClientTests.cs +++ b/ILSpy.Tests/Processes/DiagnosticsIpcClientTests.cs @@ -108,4 +108,35 @@ public class DiagnosticsIpcClientTests "ProcessInfo2 reports the managed entry-point assembly, not the native host"); info.ClrVersion.Should().NotBeNullOrWhiteSpace(); } + + [Test] + public async Task An_Endpoint_That_Accepts_But_Never_Answers_Is_Reported_As_A_Timeout() + { + // A suspended runtime behaves this way, and so does one whose diagnostics server is + // wedged. The budget must expire into something that names the process, rather than + // into a bare cancellation that reads like the caller changed its mind. + using var endpoint = new HungDiagnosticsEndpoint(pid: HungEndpointPid); + + var query = async () => await DiagnosticsIpcClient.GetProcessInfoAsync( + HungEndpointPid, TimeSpan.FromMilliseconds(250), CancellationToken.None); + + (await query.Should().ThrowAsync()) + .WithMessage($"*{HungEndpointPid}*", "the message must say which process went quiet"); + } + + [Test] + public async Task Caller_Cancellation_Is_Not_Disguised_As_A_Timeout() + { + using var endpoint = new HungDiagnosticsEndpoint(pid: HungEndpointPid + 1); + using var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(100)); + + var query = async () => await DiagnosticsIpcClient.GetProcessInfoAsync( + HungEndpointPid + 1, TimeSpan.FromMinutes(5), cts.Token); + + await query.Should().ThrowAsync( + "the dialog closing is not the target's fault"); + } + + // No process has this id; the endpoint below is a fake one standing at its name. + const int HungEndpointPid = 0x7FFF_0000; } diff --git a/ILSpy.Tests/Processes/FakeProcessExplorer.cs b/ILSpy.Tests/Processes/FakeProcessExplorer.cs index 3f6d27081..be57407f6 100644 --- a/ILSpy.Tests/Processes/FakeProcessExplorer.cs +++ b/ILSpy.Tests/Processes/FakeProcessExplorer.cs @@ -41,7 +41,12 @@ sealed class FakeProcessExplorer : IProcessExplorer public Exception? ProcessesException { get; set; } public Exception? ModulesException { get; set; } - /// When set, module queries wait for it before returning. + /// + /// When set, module queries wait for it before returning. The wait deliberately ignores + /// the cancellation token: the real explorer parks on socket I/O on a thread-pool thread, + /// so cancelling it does not complete the query inline on the cancelling thread. A gate + /// that did would hide state a cancelled query leaves behind. + /// public TaskCompletionSource? ModulesGate { get; set; } public CancellationToken LastModulesToken { get; private set; } @@ -69,7 +74,7 @@ sealed class FakeProcessExplorer : IProcessExplorer if (ModulesException != null) throw ModulesException; if (ModulesGate != null) - await ModulesGate.Task.WaitAsync(cancellationToken); + await ModulesGate.Task; await Task.Yield(); cancellationToken.ThrowIfCancellationRequested(); return ModulesByPid.TryGetValue(process.Pid, out var modules) diff --git a/ILSpy.Tests/Processes/HungDiagnosticsEndpoint.cs b/ILSpy.Tests/Processes/HungDiagnosticsEndpoint.cs new file mode 100644 index 000000000..9007220da --- /dev/null +++ b/ILSpy.Tests/Processes/HungDiagnosticsEndpoint.cs @@ -0,0 +1,71 @@ +// Copyright (c) 2026 Christoph Wille +// +// 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; +using System.IO; +using System.IO.Pipes; +using System.Net.Sockets; + +namespace ICSharpCode.ILSpy.Tests.Processes; + +/// +/// A diagnostics endpoint that accepts a connection and then says nothing, standing at the +/// transport name a CoreCLR process of the given pid would use. It is how a suspended or +/// wedged runtime behaves, and the only way to make the client's command budget expire +/// without waiting out the production one. +/// +/// +/// Both transports are created unconditionally rather than under #if, matching how the +/// production scanner keeps its Windows and unix halves in one always-compiled type; only the +/// one this OS uses is actually opened. +/// +sealed class HungDiagnosticsEndpoint : IDisposable +{ + readonly NamedPipeServerStream? pipe; + readonly Socket? socket; + readonly string? socketPath; + + public HungDiagnosticsEndpoint(int pid) + { + if (OperatingSystem.IsWindows()) + { + pipe = new NamedPipeServerStream("dotnet-diagnostic-" + pid, PipeDirection.InOut, + maxNumberOfServerInstances: 1, PipeTransmissionMode.Byte, PipeOptions.Asynchronous); + // Accepting is all it does: the connected client waits for a response that never + // comes. The task is deliberately not awaited and needs no result. + _ = pipe.WaitForConnectionAsync(); + } + else + { + socketPath = Path.Combine(Path.GetTempPath(), $"dotnet-diagnostic-{pid}-1-socket"); + File.Delete(socketPath); + socket = new Socket(AddressFamily.Unix, SocketType.Stream, ProtocolType.Unspecified); + socket.Bind(new UnixDomainSocketEndPoint(socketPath)); + socket.Listen(1); + _ = socket.AcceptAsync(); + } + } + + public void Dispose() + { + pipe?.Dispose(); + socket?.Dispose(); + if (socketPath != null) + File.Delete(socketPath); + } +} diff --git a/ILSpy.Tests/Processes/NettraceRundownReaderTests.cs b/ILSpy.Tests/Processes/NettraceRundownReaderTests.cs index a4af9397c..77b97ce98 100644 --- a/ILSpy.Tests/Processes/NettraceRundownReaderTests.cs +++ b/ILSpy.Tests/Processes/NettraceRundownReaderTests.cs @@ -20,6 +20,7 @@ using System; using System.IO; using System.Linq; using System.Reflection; +using System.Reflection.Emit; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -43,16 +44,38 @@ public class NettraceRundownReaderTests { static MemoryStream? rundown; + // Kept alive for the fixture's lifetime: an assembly the runtime has collected is no + // longer in the rundown. + static Assembly? inMemoryFixture; + + const string InMemoryFixtureName = "ILSpy.Tests.InMemoryRundownFixture"; + /// - /// Collecting a rundown takes a moment, so every test in this fixture shares one. + /// Collecting a rundown takes a moment, so every test in this fixture shares one. The + /// in-memory assembly is emitted first, so the one rundown covers both the ordinary + /// file-backed modules and an assembly that has no file anywhere. /// [OneTimeSetUp] public async Task CollectRundownOfTheTestHost() { + inMemoryFixture = EmitAssemblyWithNoFile(); using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(60)); rundown = await DiagnosticsIpcClient.CollectModuleRundownAsync(Environment.ProcessId, cts.Token); } + /// + /// An assembly that exists only in this process's memory - the case a user hits with + /// Assembly.Load(byte[]), a dynamic proxy, or a source generator's scratch + /// assembly. It is what the reader must classify as unopenable. + /// + static Assembly EmitAssemblyWithNoFile() + { + var name = new AssemblyName(InMemoryFixtureName); + var builder = AssemblyBuilder.DefineDynamicAssembly(name, AssemblyBuilderAccess.Run); + builder.DefineDynamicModule(InMemoryFixtureName); + return builder; + } + [OneTimeTearDown] public void DisposeRundown() => rundown?.Dispose(); @@ -106,12 +129,51 @@ public class NettraceRundownReaderTests // the rundown rather than enumerating native modules. File.Exists(NativeRuntimeHost.FullPath).Should().BeTrue( "the runtime host of the current process is the native module to look for"); + modules.Should().NotContain(m => string.Equals(m.Path, NativeRuntimeHost.FullPath, StringComparison.OrdinalIgnoreCase)); + + // Stated as the property itself rather than as a list of names to keep out: any native + // module the reader starts admitting fails this, not just the ones thought of here. + foreach (var module in modules.Where(m => !m.IsInMemory)) + { + ProcessExplorer.IsManagedAssembly(module.Path!).Should().BeTrue( + $"'{module.Name}' was listed as an assembly loaded in the process"); + } + } + + [Test] + public void An_Assembly_With_No_File_Is_Listed_But_Marked_Unopenable() + { + var modules = NettraceRundownReader.ReadModules(Rundown()); + + inMemoryFixture.Should().NotBeNull("the fixture assembly is emitted before the rundown is collected"); + var emitted = modules.Should().ContainSingle(m => m.Name.StartsWith(InMemoryFixtureName, StringComparison.Ordinal), + "an assembly the runtime holds only in memory is still worth showing").Subject; + emitted.IsInMemory.Should().BeTrue(); + emitted.Path.Should().BeNull("there is no file to open, which is what the dialog tells the user"); + } + + [Test] + public void A_Payload_That_Ends_Inside_A_String_Is_Rejected() + { + // "ab", filling the payload exactly, with no terminator - what a payload whose layout + // does not match what its event id promised looks like. The reader must stop at the + // payload's end rather than hunt for a zero word through the rest of the stream. + var unterminated = new byte[] { 0x61, 0, 0x62, 0 }; + using var reader = new BinaryReader(new MemoryStream(unterminated)); + + var read = () => NettraceRundownReader.ReadUtf16NullTerminated(reader, unterminated.Length); + + read.Should().Throw().WithMessage("*unterminated*"); + } + + [Test] + public void A_Terminated_String_Inside_Its_Payload_Reads_Back() + { + var terminated = new byte[] { 0x61, 0, 0x62, 0, 0, 0, 0xFF, 0xFF }; + using var reader = new BinaryReader(new MemoryStream(terminated)); - modules.Select(m => m.Name).Should().NotContain( - name => name.StartsWith("coreclr", StringComparison.OrdinalIgnoreCase) - || name.StartsWith("libcoreclr", StringComparison.OrdinalIgnoreCase) - || name.StartsWith("hostfxr", StringComparison.OrdinalIgnoreCase) - || name.StartsWith("kernel32", StringComparison.OrdinalIgnoreCase)); + NettraceRundownReader.ReadUtf16NullTerminated(reader, terminated.Length).Should().Be("ab"); + reader.BaseStream.Position.Should().Be(6, "the terminator is consumed and the rest is left alone"); } [Test] diff --git a/ILSpy.Tests/Processes/OpenFromProcessDialogViewModelTests.cs b/ILSpy.Tests/Processes/OpenFromProcessDialogViewModelTests.cs index 1cc2a6350..2ccc5874d 100644 --- a/ILSpy.Tests/Processes/OpenFromProcessDialogViewModelTests.cs +++ b/ILSpy.Tests/Processes/OpenFromProcessDialogViewModelTests.cs @@ -260,6 +260,47 @@ public class OpenFromProcessDialogViewModelTests closed.Should().BeFalse(); } + [AvaloniaTest] + public async Task Refreshing_While_Assemblies_Are_Loading_Stops_The_Progress_Bar() + { + var (vm, explorer) = CreateViewModel(); + vm.RefreshCommand.Execute(null); + await Waiters.WaitForAsync(() => vm.Processes.Count == 2); + explorer.ModulesGate = new TaskCompletionSource(); + vm.SelectedProcess = vm.Processes[0]; + await Waiters.WaitForAsync(() => vm.IsLoadingModules); + + // Refresh drops the selection, so no assembly list is being loaded any more - while + // the query that was in flight has been superseded and can no longer report that it + // stopped. Something has to turn the indicator off, or it animates over an empty pane + // until the user selects another process. + explorer.ModulesGate = null; + vm.RefreshCommand.Execute(null); + await Waiters.WaitForAsync(() => vm.SelectedProcess == null && vm.Processes.Count == 2); + + vm.IsLoadingModules.Should().BeFalse("nothing is loading, so nothing may animate"); + } + + [AvaloniaTest] + public async Task A_Failed_Scan_Leaves_Nothing_Selected_To_Act_On() + { + var (vm, explorer) = CreateViewModel(); + vm.RefreshCommand.Execute(null); + await Waiters.WaitForAsync(() => vm.Processes.Count == 2); + vm.SelectedProcess = vm.Processes[0]; + await Waiters.WaitForAsync(() => vm.AddEntryAssemblyCommand.CanExecute(null)); + + explorer.ProcessesException = new IOException("the diagnostics port is unreachable"); + vm.RefreshCommand.Execute(null); + await Waiters.WaitForAsync(() => vm.ErrorMessage != null); + + // The list the selection pointed into has just been emptied; leaving the selection + // behind keeps the entry-assembly button armed against a row nobody can see. + vm.Processes.Should().BeEmpty(); + vm.SelectedProcess.Should().BeNull(); + vm.AddEntryAssemblyCommand.CanExecute(null).Should().BeFalse(); + } + [AvaloniaTest] public async Task Closing_The_Dialog_Cancels_Work_Still_In_Flight() { diff --git a/ILSpy.Tests/Processes/ProcessExplorerTests.cs b/ILSpy.Tests/Processes/ProcessExplorerTests.cs index d1838b6a8..d85276b4f 100644 --- a/ILSpy.Tests/Processes/ProcessExplorerTests.cs +++ b/ILSpy.Tests/Processes/ProcessExplorerTests.cs @@ -20,6 +20,7 @@ using System; using System.Collections.Generic; using System.IO; using System.Linq; +using System.Net.Sockets; using System.Reflection; using System.Threading; using System.Threading.Tasks; @@ -52,8 +53,77 @@ public class ProcessExplorerTests var self = processes.Should().ContainSingle(p => p.Pid == Environment.ProcessId).Subject; self.Kind.Should().Be(RuntimeKind.CoreClr); self.ProcessName.Should().NotBeNullOrWhiteSpace(); - self.EntryAssemblyName.Should().Be(Assembly.GetEntryAssembly()!.GetName().Name); - self.RuntimeVersion.Should().NotBeNullOrWhiteSpace(); + // A null here means the process was listed but did not answer: the diagnostics query + // failed or ran out its budget. It is not an "unknown entry assembly" - the runtime + // always knows its own. + self.EntryAssemblyName.Should().Be(Assembly.GetEntryAssembly()!.GetName().Name, + "the runtime answered the process-info query"); + self.RuntimeVersion.Should().NotBeNullOrWhiteSpace( + "the runtime answered the process-info query"); + } + + [Test] + public void Every_Way_An_Endpoint_Can_Fail_To_Answer_Counts_As_Unreachable() + { + // One process that cannot be reached must cost that one row, not the listing: the + // queries run concurrently, so an exception that escapes the classification fails the + // whole enumeration and the dialog shows an empty machine. + ProcessExplorer.IsUnreachable(new SocketException((int)SocketError.ConnectionRefused)) + .Should().BeTrue("a refused unix socket - a stale file, or a runtime already torn down"); + ProcessExplorer.IsUnreachable(new IOException("broken pipe")).Should().BeTrue(); + ProcessExplorer.IsUnreachable(new EndOfStreamException()).Should().BeTrue(); + ProcessExplorer.IsUnreachable(new TimeoutException()).Should().BeTrue("a named pipe with no server"); + ProcessExplorer.IsUnreachable(new UnauthorizedAccessException()).Should().BeTrue("another user's endpoint"); + + ProcessExplorer.IsUnreachable(new InvalidDataException("the reader misread a block")) + .Should().BeFalse("a defect in this code must not be disguised as an unreachable process"); + } + + [Test] + [Platform(Exclude = "Win", Reason = "Unix domain sockets are the transport being planted; Windows uses named pipes.")] + public async Task A_Socket_That_Refuses_The_Connection_Costs_One_Row_Not_The_Listing() + { + // The reachable case is covered above; this is the same scan with one endpoint that + // exists as a file but has nobody listening behind it - what a process leaves behind + // when it exits between the port scan and the connect. + using var child = StartNonDotNetChildProcess(); + string socketPath = Path.Combine(Path.GetTempPath(), $"dotnet-diagnostic-{child.Id}-1-socket"); + using var abandoned = new Socket(AddressFamily.Unix, SocketType.Stream, ProtocolType.Unspecified); + abandoned.Bind(new UnixDomainSocketEndPoint(socketPath)); + try + { + // Not listening, so a connect is refused. Asserted directly, because a planted + // socket that failed some other way would make the scan below prove nothing. + var connect = async () => await DiagnosticsIpcClient.ConnectAsync(child.Id, CancellationToken.None); + await connect.Should().ThrowAsync( + "a bound socket with no listener refuses connections"); + + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(60)); + var processes = await Explorer.GetProcessesAsync(cts.Token); + + processes.Should().Contain(p => p.Pid == Environment.ProcessId, + "the reachable processes must still be listed"); + processes.Should().Contain(p => p.Pid == child.Id && p.EntryAssemblyName == null, + "the unreachable one is listed without the metadata only its runtime could give"); + } + finally + { + abandoned.Close(); + File.Delete(socketPath); + child.Kill(entireProcessTree: true); + } + } + + /// + /// A live process that hosts no .NET runtime, so the only diagnostics endpoint it appears + /// to have is the one the test plants for it. + /// + static System.Diagnostics.Process StartNonDotNetChildProcess() + { + var child = System.Diagnostics.Process.Start(new System.Diagnostics.ProcessStartInfo( + "/bin/sh", "-c \"sleep 120\"") { UseShellExecute = false }); + child.Should().NotBeNull(); + return child!; } [Test] diff --git a/ILSpy.Tests/Views/MainMenuTests.cs b/ILSpy.Tests/Views/MainMenuTests.cs index 52987b6d1..7ab170c5f 100644 --- a/ILSpy.Tests/Views/MainMenuTests.cs +++ b/ILSpy.Tests/Views/MainMenuTests.cs @@ -91,6 +91,7 @@ public class MainMenuTests process.Should().NotBeNull("the File menu must contain an 'Open from running process' item"); process!.IsEnabled.Should().BeTrue( "the runtime's diagnostics endpoint answers on Windows, Linux and macOS alike"); + process.Icon.Should().NotBeNull("the other entries of the open-from group all carry an icon"); var fileSubmenu = menu!.Items.OfType() .Select(i => i.Menu) diff --git a/ILSpy.Tests/Views/OpenFromProcessDialogStructureTests.cs b/ILSpy.Tests/Views/OpenFromProcessDialogStructureTests.cs index 456068aa1..a357461ac 100644 --- a/ILSpy.Tests/Views/OpenFromProcessDialogStructureTests.cs +++ b/ILSpy.Tests/Views/OpenFromProcessDialogStructureTests.cs @@ -127,6 +127,55 @@ public class OpenFromProcessDialogStructureTests "the grid's selection is what the Add button acts on"); } + [AvaloniaTest] + public async Task Filtering_Keeps_A_Selected_Process_That_Still_Matches() + { + // Only reproducible with the grid attached: the view model alone never sees the + // selection being written back, because it is the grid that pushes it. + var explorer = new FakeProcessExplorer(); + explorer.ProcessesToReturn.Add(FakeProcessExplorer.Process(100, "ILSpy", "ILSpy")); + explorer.ProcessesToReturn.Add(FakeProcessExplorer.Process(200, "dotnet", "MyTool")); + explorer.ModulesByPid[100] = new[] { + new ICSharpCode.ILSpy.Processes.ProcessModuleInfo("A.dll", @"C:\a\A.dll", IsInMemory: false), + }; + var dialog = CreateDialog(explorer); + dialog.Show(); + var vm = (OpenFromProcessDialogViewModel)dialog.DataContext!; + await Waiters.WaitForAsync(() => vm.Processes.Count == 2); + dialog.FindControl("ProcessesGrid")!.SelectedItem = vm.Processes[0]; + await Waiters.WaitForAsync(() => vm.Modules.Count == 1); + + // One keystroke that narrows the list without excluding the selected row. + vm.FilterText = "ILSpy"; + + vm.Processes.Should().ContainSingle().Which.Pid.Should().Be(100); + vm.SelectedProcess.Should().NotBeNull("the selected process still matches what was typed"); + vm.Modules.Should().ContainSingle("its assembly list must survive a keystroke"); + } + + [AvaloniaTest] + public async Task Filtering_Out_The_Selected_Process_Clears_Its_Assemblies() + { + var explorer = new FakeProcessExplorer(); + explorer.ProcessesToReturn.Add(FakeProcessExplorer.Process(100, "ILSpy", "ILSpy")); + explorer.ProcessesToReturn.Add(FakeProcessExplorer.Process(200, "dotnet", "MyTool")); + explorer.ModulesByPid[100] = new[] { + new ICSharpCode.ILSpy.Processes.ProcessModuleInfo("A.dll", @"C:\a\A.dll", IsInMemory: false), + }; + var dialog = CreateDialog(explorer); + dialog.Show(); + var vm = (OpenFromProcessDialogViewModel)dialog.DataContext!; + await Waiters.WaitForAsync(() => vm.Processes.Count == 2); + dialog.FindControl("ProcessesGrid")!.SelectedItem = vm.Processes[0]; + await Waiters.WaitForAsync(() => vm.Modules.Count == 1); + + vm.FilterText = "dotnet"; + + await Waiters.WaitForAsync(() => vm.SelectedProcess == null); + vm.Modules.Should().BeEmpty("the assemblies of a process that is no longer listed are stale"); + vm.IsLoadingModules.Should().BeFalse(); + } + [AvaloniaTest] public async Task Error_Bar_Shows_The_ViewModels_Error_Message() { diff --git a/ILSpy/Assets/Icons/Process.svg b/ILSpy/Assets/Icons/Process.svg new file mode 100644 index 000000000..a1d4bff4f --- /dev/null +++ b/ILSpy/Assets/Icons/Process.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/ILSpy/Commands/FileCommands.cs b/ILSpy/Commands/FileCommands.cs index eeff82e7d..d89626d27 100644 --- a/ILSpy/Commands/FileCommands.cs +++ b/ILSpy/Commands/FileCommands.cs @@ -154,7 +154,7 @@ namespace ICSharpCode.ILSpy.Commands } } - [ExportMainMenuCommand(ParentMenuID = nameof(Resources._File), Header = nameof(Resources.OpenFrom_RunningProcess), MenuCategory = nameof(Resources.Open), MenuOrder = 1.7)] + [ExportMainMenuCommand(ParentMenuID = nameof(Resources._File), Header = nameof(Resources.OpenFrom_RunningProcess), MenuIcon = "Images/Process", MenuCategory = nameof(Resources.Open), MenuOrder = 1.7)] [Shared] [method: ImportingConstructor] sealed class OpenFromRunningProcessCommand(AssemblyTreeModel assemblyTreeModel) : SimpleCommand diff --git a/ILSpy/Images.cs b/ILSpy/Images.cs index b3508dc68..586f2f7a1 100644 --- a/ILSpy/Images.cs +++ b/ILSpy/Images.cs @@ -69,6 +69,7 @@ namespace ICSharpCode.ILSpy public static readonly IImage Search = LoadSvg(nameof(Search)); public static readonly IImage Library = LoadSvg(nameof(Library)); public static readonly IImage NuGet = LoadPng(nameof(NuGet)); + public static readonly IImage Process = LoadSvg(nameof(Process)); public static readonly IImage MetadataFile = LoadSvg(nameof(MetadataFile)); public static readonly IImage WebAssemblyFile = LoadSvg("WebAssembly"); public static readonly IImage ProgramDebugDatabase = LoadSvg(nameof(ProgramDebugDatabase)); diff --git a/ILSpy/Processes/DiagnosticsIpcClient.cs b/ILSpy/Processes/DiagnosticsIpcClient.cs index 3e73b252d..702cc31ba 100644 --- a/ILSpy/Processes/DiagnosticsIpcClient.cs +++ b/ILSpy/Processes/DiagnosticsIpcClient.cs @@ -74,52 +74,85 @@ namespace ICSharpCode.ILSpy.Processes // ample; oversizing it would make the target runtime reserve memory for nothing. const uint CircularBufferMB = 16; - static readonly TimeSpan CommandTimeout = TimeSpan.FromSeconds(2); + // Generous for what it covers - a healthy runtime answers a status query in + // milliseconds - because the only thing on the other side of it is a runtime that will + // never answer at all: one suspended on a startup diagnostic port, or wedged. A tighter + // budget buys nothing and turns a loaded machine into a process listed without its + // metadata. + static readonly TimeSpan CommandTimeout = TimeSpan.FromSeconds(10); // A rundown walks every loaded module and assembly, so it takes measurably longer // than a status query - and longer still on a large process. static readonly TimeSpan RundownTimeout = TimeSpan.FromSeconds(30); - public static async Task GetProcessInfoAsync(int pid, CancellationToken cancellationToken) + public static Task GetProcessInfoAsync(int pid, CancellationToken cancellationToken) + => GetProcessInfoAsync(pid, CommandTimeout, cancellationToken); + + /// + /// Overload with an explicit budget, so a test can pin the timeout behavior without + /// waiting out the production one. + /// + internal static async Task GetProcessInfoAsync( + int pid, TimeSpan commandTimeout, CancellationToken cancellationToken) { try { - return await QueryProcessInfoAsync(pid, ProcessInfo2CommandId, cancellationToken).ConfigureAwait(false); + return await QueryProcessInfoAsync(pid, ProcessInfo2CommandId, commandTimeout, cancellationToken).ConfigureAwait(false); } catch (Exception ex) when (ex is IOException or EndOfStreamException) { // ProcessInfo2 needs .NET 6+; a .NET Core 3.x/5 runtime answers with an // unknown-command error. The original ProcessInfo works from 3.0 on. - return await QueryProcessInfoAsync(pid, ProcessInfoCommandId, cancellationToken).ConfigureAwait(false); + return await QueryProcessInfoAsync(pid, ProcessInfoCommandId, commandTimeout, cancellationToken).ConfigureAwait(false); } } - static async Task QueryProcessInfoAsync(int pid, byte commandId, CancellationToken cancellationToken) + static Task QueryProcessInfoAsync( + int pid, byte commandId, TimeSpan commandTimeout, CancellationToken cancellationToken) + => WithBudgetAsync(pid, commandTimeout, cancellationToken, async token => { + Stream stream = await ConnectAsync(pid, token).ConfigureAwait(false); + await using (stream.ConfigureAwait(false)) + { + byte[] request = DiagnosticsIpcMessage.EncodeRequest(ProcessCommandSet, commandId, ReadOnlySpan.Empty); + await stream.WriteAsync(request, token).ConfigureAwait(false); + byte[] payload = await DiagnosticsIpcMessage.ReadResponseAsync(stream, token).ConfigureAwait(false); + + using var reader = new BinaryReader(new MemoryStream(payload)); + long reportedPid = reader.ReadInt64(); + var runtimeCookie = new Guid(reader.ReadBytes(16)); + string? commandLine = DiagnosticsIpcMessage.ReadString(reader); + string? operatingSystem = DiagnosticsIpcMessage.ReadString(reader); + string? architecture = DiagnosticsIpcMessage.ReadString(reader); + string? entryAssembly = null; + string? clrVersion = null; + if (commandId == ProcessInfo2CommandId) + { + entryAssembly = DiagnosticsIpcMessage.ReadString(reader); + clrVersion = DiagnosticsIpcMessage.ReadString(reader); + } + return new DotNetProcessInfo(reportedPid, runtimeCookie, commandLine, + operatingSystem, architecture, entryAssembly, clrVersion); + } + }); + + /// + /// Runs one command under a budget, turning an expired budget into a + /// that names the process. The conversion is what keeps + /// the two outcomes apart: callers are expected to swallow cancellation, because that + /// is the dialog closing, and a target that never answered must not vanish with it. + /// + static async Task WithBudgetAsync(int pid, TimeSpan budget, + CancellationToken cancellationToken, Func> command) { using var timeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - timeout.CancelAfter(CommandTimeout); - - Stream stream = await ConnectAsync(pid, timeout.Token).ConfigureAwait(false); - await using (stream.ConfigureAwait(false)) + timeout.CancelAfter(budget); + try { - byte[] request = DiagnosticsIpcMessage.EncodeRequest(ProcessCommandSet, commandId, ReadOnlySpan.Empty); - await stream.WriteAsync(request, timeout.Token).ConfigureAwait(false); - byte[] payload = await DiagnosticsIpcMessage.ReadResponseAsync(stream, timeout.Token).ConfigureAwait(false); - - using var reader = new BinaryReader(new MemoryStream(payload)); - long reportedPid = reader.ReadInt64(); - var runtimeCookie = new Guid(reader.ReadBytes(16)); - string? commandLine = DiagnosticsIpcMessage.ReadString(reader); - string? operatingSystem = DiagnosticsIpcMessage.ReadString(reader); - string? architecture = DiagnosticsIpcMessage.ReadString(reader); - string? entryAssembly = null; - string? clrVersion = null; - if (commandId == ProcessInfo2CommandId) - { - entryAssembly = DiagnosticsIpcMessage.ReadString(reader); - clrVersion = DiagnosticsIpcMessage.ReadString(reader); - } - return new DotNetProcessInfo(reportedPid, runtimeCookie, commandLine, - operatingSystem, architecture, entryAssembly, clrVersion); + return await command(timeout.Token).ConfigureAwait(false); + } + catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) + { + throw new TimeoutException( + $"Process {pid} did not answer its diagnostics endpoint within {budget.TotalSeconds:0.###} seconds."); } } @@ -143,42 +176,39 @@ namespace ICSharpCode.ILSpy.Processes } } - static async Task CollectAsync(int pid, byte commandId, CancellationToken cancellationToken) - { - using var timeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - timeout.CancelAfter(RundownTimeout); - - Stream session = await ConnectAsync(pid, timeout.Token).ConfigureAwait(false); - await using (session.ConfigureAwait(false)) - { - byte[] request = DiagnosticsIpcMessage.EncodeRequest( - EventPipeCommandSet, commandId, BuildCollectTracingPayload(commandId)); - await session.WriteAsync(request, timeout.Token).ConfigureAwait(false); + static Task CollectAsync(int pid, byte commandId, CancellationToken cancellationToken) + => WithBudgetAsync(pid, RundownTimeout, cancellationToken, async token => { + Stream session = await ConnectAsync(pid, token).ConfigureAwait(false); + await using (session.ConfigureAwait(false)) + { + byte[] request = DiagnosticsIpcMessage.EncodeRequest( + EventPipeCommandSet, commandId, BuildCollectTracingPayload(commandId)); + await session.WriteAsync(request, token).ConfigureAwait(false); - byte[] response = await DiagnosticsIpcMessage.ReadResponseAsync(session, timeout.Token).ConfigureAwait(false); - if (response.Length < sizeof(ulong)) - throw new IOException("The runtime did not return an EventPipe session id."); - ulong sessionId = BinaryPrimitives.ReadUInt64LittleEndian(response); + byte[] response = await DiagnosticsIpcMessage.ReadResponseAsync(session, token).ConfigureAwait(false); + if (response.Length < sizeof(ulong)) + throw new IOException("The runtime did not return an EventPipe session id."); + ulong sessionId = BinaryPrimitives.ReadUInt64LittleEndian(response); - // The nettrace stream must be drained while the session is stopped: the - // runtime writes the rundown into the same connection, and a full buffer - // would block the stop from completing. - var trace = new MemoryStream(); - Task drain = session.CopyToAsync(trace, timeout.Token); - try - { - await StopTracingAsync(pid, sessionId, timeout.Token).ConfigureAwait(false); - await drain.ConfigureAwait(false); + // The nettrace stream must be drained while the session is stopped: the + // runtime writes the rundown into the same connection, and a full buffer + // would block the stop from completing. + var trace = new MemoryStream(); + Task drain = session.CopyToAsync(trace, token); + try + { + await StopTracingAsync(pid, sessionId, token).ConfigureAwait(false); + await drain.ConfigureAwait(false); + } + catch + { + await trace.DisposeAsync().ConfigureAwait(false); + throw; + } + trace.Position = 0; + return trace; } - catch - { - await trace.DisposeAsync().ConfigureAwait(false); - throw; - } - trace.Position = 0; - return trace; - } - } + }); static async Task StopTracingAsync(int pid, ulong sessionId, CancellationToken cancellationToken) { diff --git a/ILSpy/Processes/NettraceRundownReader.cs b/ILSpy/Processes/NettraceRundownReader.cs index 260e47c03..37c74eee7 100644 --- a/ILSpy/Processes/NettraceRundownReader.cs +++ b/ILSpy/Processes/NettraceRundownReader.cs @@ -102,10 +102,10 @@ namespace ICSharpCode.ILSpy.Processes SkipTraceObject(reader); break; case "MetadataBlock": - ReadBlock(reader, (payload, id, _) => ReadMetadataEvent(payload, id, metadata)); + ReadBlock(reader, (payload, id, end) => ReadMetadataEvent(payload, id, end, metadata)); break; case "EventBlock": - ReadBlock(reader, (payload, id, _) => ReadEvent(payload, id, metadata, modules, assemblyNames)); + ReadBlock(reader, (payload, id, end) => ReadEvent(payload, id, end, metadata, modules, assemblyNames)); break; default: // StackBlock, SPBlock and any block type added later: the block is @@ -182,9 +182,10 @@ namespace ICSharpCode.ILSpy.Processes /// /// Walks the event blobs of a metadata or event block, handing each one's payload to - /// together with its metadata id. + /// together with its metadata id and the stream position + /// where that payload ends - the bound every field read out of it must respect. /// - static void ReadBlock(BinaryReader reader, Action onEvent) + static void ReadBlock(BinaryReader reader, Action onEvent) { long blockEnd = BeginBlock(reader, out bool compressed); @@ -203,7 +204,7 @@ namespace ICSharpCode.ILSpy.Processes long payloadEnd = reader.BaseStream.Position + payloadSize; if (payloadEnd > blockEnd) throw new InvalidDataException("A Nettrace event payload runs past the end of its block."); - onEvent(reader, metadataId, payloadSize); + onEvent(reader, metadataId, payloadEnd); reader.BaseStream.Seek(payloadEnd, SeekOrigin.Begin); if (!compressed) @@ -307,20 +308,20 @@ namespace ICSharpCode.ILSpy.Processes /// An event in a metadata block describes an event type: which provider and event /// name a metadata id stands for in the event blocks that follow. /// - static void ReadMetadataEvent(BinaryReader reader, int metadataId, Dictionary metadata) + static void ReadMetadataEvent(BinaryReader reader, int metadataId, long payloadEnd, Dictionary metadata) { if (metadataId != 0) return; // Only the metadata records themselves are of interest here. int id = reader.ReadInt32(); - string providerName = ReadUtf16NullTerminated(reader); + string providerName = ReadUtf16NullTerminated(reader, payloadEnd); int eventId = reader.ReadInt32(); - ReadUtf16NullTerminated(reader); // event name, empty for manifest-based providers + ReadUtf16NullTerminated(reader, payloadEnd); // event name, empty for manifest-based providers reader.ReadInt64(); // keywords int version = reader.ReadInt32(); metadata[id] = new EventMetadata(providerName, eventId, version); } - static void ReadEvent(BinaryReader reader, int metadataId, Dictionary metadata, + static void ReadEvent(BinaryReader reader, int metadataId, long payloadEnd, Dictionary metadata, List modules, Dictionary assemblyNames) { if (!metadata.TryGetValue(metadataId, out var meta)) @@ -335,8 +336,8 @@ namespace ICSharpCode.ILSpy.Processes long assemblyId = reader.ReadInt64(); reader.ReadInt32(); // module flags reader.ReadInt32(); // reserved - string ilPath = ReadUtf16NullTerminated(reader); - string nativePath = ReadUtf16NullTerminated(reader); + string ilPath = ReadUtf16NullTerminated(reader, payloadEnd); + string nativePath = ReadUtf16NullTerminated(reader, payloadEnd); modules.Add(new ModuleRecord(assemblyId, ilPath, nativePath)); } else if (IsAssemblyEvent(meta)) @@ -346,7 +347,7 @@ namespace ICSharpCode.ILSpy.Processes if (meta.Version >= 1) reader.ReadInt64(); // binding id reader.ReadInt32(); // assembly flags - string fullName = ReadUtf16NullTerminated(reader); + string fullName = ReadUtf16NullTerminated(reader, payloadEnd); if (fullName.Length > 0) { // "Foo, Version=1.0.0.0, Culture=..." - the simple name is enough to @@ -369,16 +370,23 @@ namespace ICSharpCode.ILSpy.Processes _ => false, }; - static string ReadUtf16NullTerminated(BinaryReader reader) + /// + /// Reads a UTF-16 string terminated by a zero word, refusing to read past + /// . The bound is what keeps a payload whose layout does + /// not match what its event id promised from being followed out of its own event and + /// through the rest of the stream in search of a terminator. + /// + internal static string ReadUtf16NullTerminated(BinaryReader reader, long payloadEnd) { var builder = new StringBuilder(); - while (true) + while (reader.BaseStream.Position + sizeof(ushort) <= payloadEnd) { ushort c = reader.ReadUInt16(); if (c == 0) return builder.ToString(); builder.Append((char)c); } + throw new InvalidDataException("A Nettrace event payload ends inside an unterminated string."); } static IReadOnlyList BuildModuleList( diff --git a/ILSpy/Processes/ProcessExplorer.cs b/ILSpy/Processes/ProcessExplorer.cs index f3e873f04..25e9e6df1 100644 --- a/ILSpy/Processes/ProcessExplorer.cs +++ b/ILSpy/Processes/ProcessExplorer.cs @@ -18,6 +18,7 @@ using System; using System.Collections.Generic; +using System.ComponentModel; using System.Diagnostics; using System.IO; using System.Linq; @@ -76,19 +77,13 @@ namespace ICSharpCode.ILSpy.Processes return new RunningDotNetProcess(pid, processName, RuntimeKind.CoreClr, info.ClrVersion, info.Architecture, info.CommandLine, info.EntryAssemblyName); } - catch (Exception ex) when (ex is IOException or TimeoutException or UnauthorizedAccessException) + catch (Exception ex) when (IsUnreachable(ex)) { // The endpoint exists but did not answer - a suspended runtime, a stale // transport, or a process shutting down. Still worth listing. return new RunningDotNetProcess(pid, processName, RuntimeKind.CoreClr, RuntimeVersion: null, Architecture: null, CommandLine: null, EntryAssemblyName: null); } - catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) - { - // The per-command timeout fired rather than the caller cancelling. - return new RunningDotNetProcess(pid, processName, RuntimeKind.CoreClr, - RuntimeVersion: null, Architecture: null, CommandLine: null, EntryAssemblyName: null); - } } static async Task> EnumerateModules( @@ -119,6 +114,23 @@ namespace ICSharpCode.ILSpy.Processes } } + /// + /// Whether an exception means "that process did not answer" rather than "this code is + /// wrong". Such a process is still listed, only without the metadata that none but its + /// own runtime could supply; anything else propagates, because a process explorer that + /// swallows its own defects reports an empty machine. + /// + /// + /// The endpoints are queried concurrently, so this classification decides between + /// losing one row and losing the listing. Both transports are covered: + /// is the base of + /// and thus not an - a unix socket file whose runtime is gone + /// refuses the connection and raises it - while a Windows named pipe with no server + /// behind it surfaces as . + /// + internal static bool IsUnreachable(Exception ex) + => ex is IOException or Win32Exception or TimeoutException or UnauthorizedAccessException; + /// /// Whether the file at is a managed assembly, i.e. a PE image /// carrying a CLI header. Used to keep native libraries out of the module list of a diff --git a/ILSpy/ViewModels/OpenFromProcessDialogViewModel.cs b/ILSpy/ViewModels/OpenFromProcessDialogViewModel.cs index ffe616fb2..dc41ad948 100644 --- a/ILSpy/ViewModels/OpenFromProcessDialogViewModel.cs +++ b/ILSpy/ViewModels/OpenFromProcessDialogViewModel.cs @@ -90,10 +90,11 @@ namespace ICSharpCode.ILSpy.ViewModels partial void OnFilterTextChanged(string value) => ApplyFilter(); - partial void OnSelectedProcessChanged(ProcessRowViewModel? value) => _ = LoadModulesAsync(value); + partial void OnSelectedProcessChanged(ProcessRowViewModel? value) + => LoadModulesAsync(value).HandleExceptions(); [RelayCommand] - void Refresh() => _ = RefreshAsync(); + void Refresh() => RefreshAsync().HandleExceptions(); bool CanAddSelectedModules => SelectedModules.Any(m => !m.IsInMemory); @@ -120,13 +121,28 @@ namespace ICSharpCode.ILSpy.ViewModels /// Cancels every in-flight query; called when the dialog closes. public void CancelAllOperations() { - refreshCts?.Cancel(); - modulesCts?.Cancel(); + CancelAndDispose(ref refreshCts); + CancelAndDispose(ref modulesCts); + } + + /// + /// Cancels a query's token source and lets go of it. Disposing straight after + /// cancelling is safe because the cancellation callbacks have already run by then, and + /// the method that owns the source only ever reads + /// afterwards, which + /// stays valid on a disposed source. + /// + static void CancelAndDispose(ref CancellationTokenSource? source) + { + var previous = source; + source = null; + previous?.Cancel(); + previous?.Dispose(); } async Task RefreshAsync() { - refreshCts?.Cancel(); + CancelAndDispose(ref refreshCts); var cts = refreshCts = new CancellationTokenSource(); IsLoadingProcesses = true; try @@ -136,10 +152,7 @@ namespace ICSharpCode.ILSpy.ViewModels return; ErrorMessage = null; - SelectedProcess = null; - allProcesses.Clear(); - allProcesses.AddRange(processes.Select(p => new ProcessRowViewModel(p))); - ApplyFilter(); + ReplaceProcesses(processes); } catch (OperationCanceledException) { @@ -148,8 +161,7 @@ namespace ICSharpCode.ILSpy.ViewModels catch (Exception ex) { ErrorMessage = ex.Message; - allProcesses.Clear(); - ApplyFilter(); + ReplaceProcesses(Array.Empty()); } finally { @@ -158,16 +170,54 @@ namespace ICSharpCode.ILSpy.ViewModels } } + /// + /// Puts a freshly scanned list in place of the current one. The selection goes first: + /// the rows it pointed into are about to be gone, and a selection left behind keeps the + /// entry-assembly button armed against a process nobody can see any more. The failure + /// path empties the list the same way, for the same reason. + /// + void ReplaceProcesses(IReadOnlyList processes) + { + SelectedProcess = null; + allProcesses.Clear(); + allProcesses.AddRange(processes.Select(p => new ProcessRowViewModel(p))); + ApplyFilter(); + } + + /// + /// Brings the bound collection in line with what the filter admits, by removing and + /// inserting rows rather than rebuilding it. Clearing it would make the grid drop its + /// selection and write that null back through the two-way binding - discarding the + /// assembly list of a process the newly typed filter still matches. + /// void ApplyFilter() { - Processes.Clear(); - foreach (var process in allProcesses.Where(p => p.Matches(FilterText))) - Processes.Add(process); + var matching = allProcesses.Where(p => p.Matches(FilterText)).ToList(); + var admitted = new HashSet(matching); + // A selection the filter no longer admits is dropped here rather than left to the + // grid, which holds on to a removed row: the assembly pane must not go on + // describing a process that is no longer in the list. + if (SelectedProcess != null && !admitted.Contains(SelectedProcess)) + SelectedProcess = null; + for (int i = Processes.Count - 1; i >= 0; i--) + { + if (!admitted.Contains(Processes[i])) + Processes.RemoveAt(i); + } + // What is left is the matching rows in their original relative order, so any row + // missing at position i belongs exactly there. + for (int i = 0; i < matching.Count; i++) + { + if (i >= Processes.Count) + Processes.Add(matching[i]); + else if (!ReferenceEquals(Processes[i], matching[i])) + Processes.Insert(i, matching[i]); + } } async Task LoadModulesAsync(ProcessRowViewModel? process) { - modulesCts?.Cancel(); + CancelAndDispose(ref modulesCts); var cts = modulesCts = new CancellationTokenSource(); int generation = ++modulesGeneration; @@ -175,7 +225,13 @@ namespace ICSharpCode.ILSpy.ViewModels SelectedModules.Clear(); SetEntryAssemblyPath(null); if (process == null) + { + // Nothing is selected, so nothing is loading. The query just cancelled cannot + // say so on its way out: its generation has been superseded, which is what the + // guard below tests. + IsLoadingModules = false; return; + } IsLoadingModules = true; try diff --git a/ILSpy/ViewModels/ProcessRowViewModel.cs b/ILSpy/ViewModels/ProcessRowViewModel.cs index 0dbdfe65a..c651932d7 100644 --- a/ILSpy/ViewModels/ProcessRowViewModel.cs +++ b/ILSpy/ViewModels/ProcessRowViewModel.cs @@ -44,8 +44,6 @@ namespace ICSharpCode.ILSpy.ViewModels public string? EntryAssembly => Process.EntryAssemblyName; - public string? CommandLine => Process.CommandLine; - /// /// The runtime flavor and version, e.g. ".NET 10.0.3" or ".NET Framework 4.8.9032.0". ///