Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/DiffEngine/Protocol/ViewerClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,12 @@ public static async Task<bool> 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();
Expand Down Expand Up @@ -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
Expand Down
19 changes: 17 additions & 2 deletions src/DiffEngine/Tray/PiperClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down
Loading