Skip to content

Fix potential Debug.Assert in QuicListener #104241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 2, 2024
Merged

Fix potential Debug.Assert in QuicListener #104241

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
42 changes: 19 additions & 23 deletions src/libraries/System.Net.Quic/src/System/Net/Quic/QuicListener.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,28 +249,6 @@ private async void StartConnectionHandshake(QuicConnection connection, SslClient
await connection.DisposeAsync().ConfigureAwait(false);
}
}
catch (OperationCanceledException) when (connection.ConnectionShutdownToken.IsCancellationRequested)
{
// Connection closed by peer
if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Info(connection, $"{connection} Connection closed by remote peer");
}

// Retrieve the exception which failed the handshake, the parameters are not going to be
// validated because the inner _connectedTcs is already transitioned to faulted state.
ValueTask task = connection.FinishHandshakeAsync(null!, null!, default);
Debug.Assert(task.IsFaulted);

// Unwrap AggregateException and propagate it to the accept queue.
Exception ex = task.AsTask().Exception!.InnerException!;

await connection.DisposeAsync().ConfigureAwait(false);
if (!_acceptQueue.Writer.TryWrite(ex))
{
// Channel has been closed, connection is already disposed, do nothing.
}
}
catch (OperationCanceledException) when (_disposeCts.IsCancellationRequested)
{
// Handshake stopped by QuicListener.DisposeAsync:
Expand All @@ -284,7 +262,7 @@ private async void StartConnectionHandshake(QuicConnection connection, SslClient

await connection.DisposeAsync().ConfigureAwait(false);
}
catch (OperationCanceledException oce) when (cancellationToken.IsCancellationRequested)
catch (OperationCanceledException oce) when (cancellationToken.IsCancellationRequested && !connection.ConnectionShutdownToken.IsCancellationRequested)
{
// Handshake cancelled by options.HandshakeTimeout, probably stalled:
// 1. Connection must be killed so dispose it and by that shut it down --> application error code doesn't matter here as this is a transport error.
Expand All @@ -308,6 +286,24 @@ private async void StartConnectionHandshake(QuicConnection connection, SslClient
// 1. Dispose the connection and by that shut it down --> application error code doesn't matter here as this is a transport error.
// 2. Connection cannot be handed out since it's useless --> propagate the exception as-is, listener will pass it to the caller.

if (wrapException && connection.ConnectionShutdownToken.IsCancellationRequested)
{
// The connection was closed while we were waiting for the connection options callback to complete
// ex is going to be an OperationCanceledException, but we want to propagate the original exception.
// Since the inner _connectedTcs is already transitioned to faulted state (because ConnectionShutdownToken
// fired), the parameters to FinishHandshakeAsync are not going to be validated and it will return the
// faulted task.
ValueTask task = connection.FinishHandshakeAsync(null!, null!, default);
Debug.Assert(task.IsCompleted);

// Unwrap AggregateException and propagate it to the accept queue.
if (task.AsTask().Exception?.InnerException is Exception handshakeException)
{
ex = handshakeException;
wrapException = false;
}
}

if (NetEventSource.Log.IsEnabled())
{
NetEventSource.Error(connection, $"{connection} Connection handshake failed: {ex}");
Expand Down