diff --git a/src/DiffEngine/Protocol/ViewerClient.cs b/src/DiffEngine/Protocol/ViewerClient.cs index 9f36941a..d6f60bad 100644 --- a/src/DiffEngine/Protocol/ViewerClient.cs +++ b/src/DiffEngine/Protocol/ViewerClient.cs @@ -137,6 +137,12 @@ public static async Task TrySendAsync( #else token.ThrowIfCancellationRequested(); await client.ConnectAsync(IPAddress.Loopback, endpointPort); + // The abort registration cancels by closing the client, and .NET Framework's + // TcpClient.Dispose nulls its Client - so a token that fires around here leaves + // Configure and HalfClose dereferencing null rather than reporting cancellation. + // Asking the token directly is how that becomes the OperationCanceledException the + // caller is written against + token.ThrowIfCancellationRequested(); #endif Configure(client, timeToWait); var stream = client.GetStream(); @@ -211,6 +217,11 @@ exception is SocketException or IOException or ObjectDisposedException or + // .NET Framework's TcpClient.Dispose nulls Client, so the abort registration closing + // the socket mid exchange leaves Configure or HalfClose dereferencing null. It is a + // torn down connection wearing the wrong exception type, and letting it escape turned + // an absent owner into a crash in the caller's test + NullReferenceException or AggregateException { InnerException: SocketException or IOException diff --git a/src/DiffEngine/Tray/PiperClient.cs b/src/DiffEngine/Tray/PiperClient.cs index 5f52320a..0c513f52 100644 --- a/src/DiffEngine/Tray/PiperClient.cs +++ b/src/DiffEngine/Tray/PiperClient.cs @@ -97,8 +97,18 @@ static async Task SendAsync(string payload, Cancel cancel) { await InnerSendAsync(payload, cancel); } - // Let cancellation surface to the caller; only genuine send failures are swallowed. - catch (Exception exception) when (exception is not OperationCanceledException) + // Let cancellation surface to the caller; only genuine send failures are swallowed. A + // NullReferenceException under a cancelled token is cancellation too - .NET Framework's + // Dispose nulls Client - and reporting it as a send failure would lose the cancel + catch (OperationCanceledException) + { + throw; + } + catch (NullReferenceException) when (cancel.IsCancellationRequested) + { + throw new OperationCanceledException(cancel); + } + catch (Exception exception) { HandleSendException(payload, exception); } @@ -152,6 +162,11 @@ static async Task InnerSendAsync(string payload, Cancel cancel) using (cancel.Register(client.Close)) { await client.ConnectAsync(endpoint.Address, endpoint.Port); + // Dispose nulls Client on .NET Framework, so a token that fires around the connect + // leaves GetStream dereferencing null instead of reporting cancellation. Asking + // the token directly is what makes that an OperationCanceledException, which the + // caller lets through rather than swallowing as a send failure + cancel.ThrowIfCancellationRequested(); using var stream = client.GetStream(); using var writer = new StreamWriter(stream); await writer.WriteAsync(payload);