From 743abc20aca237fcc09d0b06b099bf2db7288033 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Sat, 22 Aug 2026 10:27:02 +1000 Subject: [PATCH] Report cancellation as cancellation on .NET Framework Both clients cancel by closing the TcpClient, because net462 through net48 have no cancellable connect or write. .NET Framework's TcpClient.Dispose nulls its Client field, so a token that fires around the connect does not leave a torn down socket behind - it leaves a null one, and the next line dereferences it. The result was cancellation wearing the wrong exception type. In ViewerClient a NullReferenceException from Configure or HalfClose escaped the catch entirely and surfaced inside the caller's failing test; an ObjectDisposedException got swallowed as "no owner", after which a viewer was launched under a token that had already been cancelled. In PiperClient the same shape was reported as a send failure, so the cancel was lost. Ask the token straight after the connect, which is where the window is, and treat a NullReferenceException as what it is: ViewerClient adds it to the ignorable set beside ObjectDisposedException, and PiperClient maps it to OperationCanceledException when the token has fired. Verified only by the suite passing on net48 as well as net10.0. Landing the race itself needs the token to fire inside a window a few instructions wide. --- src/DiffEngine/Protocol/ViewerClient.cs | 11 +++++++++++ src/DiffEngine/Tray/PiperClient.cs | 19 +++++++++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) 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);