diff --git a/ILSpy.Tests/Processes/BlockedConnection.cs b/ILSpy.Tests/Processes/BlockedConnection.cs new file mode 100644 index 000000000..342dc3442 --- /dev/null +++ b/ILSpy.Tests/Processes/BlockedConnection.cs @@ -0,0 +1,68 @@ +// 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.Threading; +using System.Threading.Tasks; + +namespace ICSharpCode.ILSpy.Tests.Processes; + +/// +/// Stands in for the connection of an EventPipe session that has been granted but never +/// delivers: a read of it completes for one reason only, the stream being torn down, and then +/// it fails the way a transport whose far end is gone does. It is the shape of connection a +/// collection is left holding when the target dies before the session can be stopped. +/// +/// +/// Only the async read path is implemented, since that is all a copy out of this stream uses; +/// the synchronous one would have to block a thread to behave the same way and no caller needs +/// it. +/// +sealed class BlockedConnection : Stream +{ + readonly TaskCompletionSource tornDown = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public override bool CanRead => true; + public override bool CanSeek => false; + public override bool CanWrite => false; + public override long Length => throw new NotSupportedException(); + public override long Position { + get => throw new NotSupportedException(); + set => throw new NotSupportedException(); + } + + public override async ValueTask ReadAsync( + Memory buffer, CancellationToken cancellationToken = default) + { + await tornDown.Task.WaitAsync(cancellationToken).ConfigureAwait(false); + throw new IOException("The connection was torn down under a pending read."); + } + + public override int Read(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + public override void Flush() { } + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + public override void SetLength(long value) => throw new NotSupportedException(); + + protected override void Dispose(bool disposing) + { + tornDown.TrySetResult(); + base.Dispose(disposing); + } +} diff --git a/ILSpy.Tests/Processes/DiagnosticsIpcClientTests.cs b/ILSpy.Tests/Processes/DiagnosticsIpcClientTests.cs index a827305ca..95f494b20 100644 --- a/ILSpy.Tests/Processes/DiagnosticsIpcClientTests.cs +++ b/ILSpy.Tests/Processes/DiagnosticsIpcClientTests.cs @@ -137,6 +137,28 @@ public class DiagnosticsIpcClientTests "the dialog closing is not the target's fault"); } + [Test] + public async Task A_Collection_That_Fails_Midway_Waits_For_Its_Own_Drain() + { + // A target that exits partway through its rundown fails the stop command, and the + // dialog reports that. What must not follow is a second report of the same event: the + // task draining the trace connection faults as soon as that connection is torn down, + // and a faulted task nobody awaited reaches TaskScheduler.UnobservedTaskException by + // way of the finalizer - which this app turns into a crash dialog, long after the + // error bar already explained the failure correctly. + using var session = new BlockedConnection(); + var trace = new MemoryStream(); + Task drain = session.CopyToAsync(trace); + drain.IsCompleted.Should().BeFalse( + "the connection says nothing until it is torn down, which is the situation being wound up"); + + await DiagnosticsIpcClient.AbandonCollectionAsync(session, drain, trace); + + drain.IsCompleted.Should().BeTrue( + "a drain still running once the failure path is done with it is a drain nobody will ever await"); + trace.CanRead.Should().BeFalse("the half-copied trace of a failed collection is dropped"); + } + // 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/Processes/DiagnosticsIpcClient.cs b/ILSpy/Processes/DiagnosticsIpcClient.cs index 702cc31ba..1832ce1a5 100644 --- a/ILSpy/Processes/DiagnosticsIpcClient.cs +++ b/ILSpy/Processes/DiagnosticsIpcClient.cs @@ -202,7 +202,7 @@ namespace ICSharpCode.ILSpy.Processes } catch { - await trace.DisposeAsync().ConfigureAwait(false); + await AbandonCollectionAsync(session, drain, trace).ConfigureAwait(false); throw; } trace.Position = 0; @@ -210,6 +210,43 @@ namespace ICSharpCode.ILSpy.Processes } }); + /// + /// Winds up a collection that failed partway through, in the one order that works. The + /// session connection goes first, because the drain is parked on a read of it and after + /// the failure nothing else will ever end that read. Then the drain itself, whose own + /// failure is no more than a symptom of the one already on its way to the caller - but + /// which has to be awaited all the same: a faulted task nobody awaited is raised by the + /// finalizer as , so abandoning it + /// here reports the same failure a second time, minutes later and out of context. The + /// half-copied trace goes last, once nothing is writing into it any more. + /// + /// + /// Disposing the session twice is harmless - the caller's await using does it + /// again on the way out - and doing it here is what makes the drain finishable at all. + /// + internal static async Task AbandonCollectionAsync(Stream session, Task drain, MemoryStream trace) + { + await SuppressFailureAsync(session.DisposeAsync().AsTask()).ConfigureAwait(false); + await SuppressFailureAsync(drain).ConfigureAwait(false); + await trace.DisposeAsync().ConfigureAwait(false); + } + + /// + /// Awaits for its completion alone, discarding how it ended. + /// + static async Task SuppressFailureAsync(Task task) + { + try + { + await task.ConfigureAwait(false); + } + catch (Exception) + { + // Discarded by design: this runs while another failure is propagating, and the + // only purpose of the await is to leave nothing running or unobserved behind. + } + } + static async Task StopTracingAsync(int pid, ulong sessionId, CancellationToken cancellationToken) { Stream stream = await ConnectAsync(pid, cancellationToken).ConfigureAwait(false);