Skip to content
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.

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

[release/6.0] add Http3LoopbackConnection.ShutdownAsync and use in AcceptConnectionAsync #58336

Closed
wants to merge 6 commits into from
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ internal sealed class Http3LoopbackConnection : GenericLoopbackConnection

// This is specifically request streams, not control streams
private readonly Dictionary<int, Http3LoopbackStream> _openStreams = new Dictionary<int, Http3LoopbackStream>();

private Http3LoopbackStream _currentStream;
// We can't retrieve the stream ID after the stream is disposed, so store it separately
// Initialize it to -4 so that the firstInvalidStreamId calculation will work even if we never process a request
private long _currentStreamId = -4;

private Http3LoopbackStream _inboundControlStream; // Inbound control stream from client
private Http3LoopbackStream _outboundControlStream; // Our outbound control stream
Expand Down Expand Up @@ -111,6 +115,7 @@ public async Task<Http3LoopbackStream> AcceptStreamAsync()
{
_openStreams.Add(checked((int)quicStream.StreamId), stream);
_currentStream = stream;
_currentStreamId = quicStream.StreamId;
}

return stream;
Expand Down Expand Up @@ -223,6 +228,8 @@ public override async Task<HttpRequestData> HandleRequestAsync(HttpStatusCode st

// We are about to close the connection, after we send the response.
// So, send a GOAWAY frame now so the client won't inadvertantly try to reuse the connection.
// Note that in HTTP3 (unlike HTTP2) there is no strict ordering between the GOAWAY and the response below;
// so the client may race in processing them and we need to handle this.
await _outboundControlStream.SendGoAwayFrameAsync(stream.StreamId + 4);

await stream.SendResponseAsync(statusCode, headers, content).ConfigureAwait(false);
Expand All @@ -232,6 +239,34 @@ public override async Task<HttpRequestData> HandleRequestAsync(HttpStatusCode st
return request;
}

public async Task ShutdownAsync()
{
try
{
long firstInvalidStreamId = _currentStreamId + 4;
await _outboundControlStream.SendGoAwayFrameAsync(firstInvalidStreamId);
}
catch (QuicConnectionAbortedException abortException) when (abortException.ErrorCode == H3_NO_ERROR)
{
// Client must have closed the connection already because the HttpClientHandler instance was disposed.
// So nothing to do.
return;
}
catch (OperationCanceledException)
{
// If the client is closing the connection at the same time we are trying to send the GOAWAY above,
// this can result in OperationCanceledException from QuicStream.WriteAsync.
// See https://github.com/dotnet/runtime/issues/58078
// I saw this consistently with GetAsync_EmptyResponseHeader_Success.
// To work around this, just eat the exception for now.
// Also, be aware of this issue as it will do weird things with OperationCanceledException and can
// make debugging this very confusing: https://github.com/dotnet/runtime/issues/58081
return;
}

await WaitForClientDisconnectAsync();
}

// Wait for the client to close the connection, e.g. after we send a GOAWAY, or after the HttpClient is disposed.
public async Task WaitForClientDisconnectAsync(bool refuseNewRequests = true)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public override void Dispose()
_cert.Dispose();
}

public override async Task<GenericLoopbackConnection> EstablishGenericConnectionAsync()
private async Task<Http3LoopbackConnection> EstablishHttp3ConnectionAsync()
{
QuicConnection con = await _listener.AcceptConnectionAsync().ConfigureAwait(false);
Http3LoopbackConnection connection = new Http3LoopbackConnection(con);
Expand All @@ -61,10 +61,16 @@ public override async Task<GenericLoopbackConnection> EstablishGenericConnection
return connection;
}

public override async Task<GenericLoopbackConnection> EstablishGenericConnectionAsync()
{
return await EstablishHttp3ConnectionAsync();
}

public override async Task AcceptConnectionAsync(Func<GenericLoopbackConnection, Task> funcAsync)
{
using GenericLoopbackConnection con = await EstablishGenericConnectionAsync().ConfigureAwait(false);
using Http3LoopbackConnection con = await EstablishHttp3ConnectionAsync().ConfigureAwait(false);
await funcAsync(con).ConfigureAwait(false);
await con.ShutdownAsync();
}

public override async Task<HttpRequestData> HandleRequestAsync(HttpStatusCode statusCode = HttpStatusCode.OK, IList<HttpHeaderData> headers = null, string content = "")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,11 @@ await LoopbackServerFactory.CreateServerAsync(async (server, url) =>
Task<HttpResponseMessage> getResponse = client.SendAsync(TestAsync, req, HttpCompletionOption.ResponseHeadersRead, cts.Token);
await ValidateClientCancellationAsync(async () =>
{
HttpResponseMessage resp = await getResponse;
// This 'using' shouldn't be necessary in general. However, HTTP3 does not remove the request stream from the
// active stream table until the user disposes the response (or it gets finalized).
// This means the connection will fail to shut down promptly.
// See https://github.com/dotnet/runtime/issues/58072
using HttpResponseMessage resp = await getResponse;
Stream respStream = await resp.Content.ReadAsStreamAsync(TestAsync);
Task readTask = readOrCopyToAsync ?
respStream.ReadAsync(new byte[1], 0, 1, cts.Token) :
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,9 @@ await LoopbackServer.CreateClientAndServerAsync(async proxyUri =>

public static IEnumerable<object[]> SecureAndNonSecure_IPBasedUri_MemberData() =>
from address in new[] { IPAddress.Loopback, IPAddress.IPv6Loopback }
from useSsl in BoolValues
from useSsl in BoolValues
// we could not create SslStream in browser, [ActiveIssue("https://github.com/dotnet/runtime/issues/37669", TestPlatforms.Browser)]
where PlatformDetection.IsNotBrowser || !useSsl
where PlatformDetection.IsNotBrowser || !useSsl
select new object[] { address, useSsl };

[Theory]
Expand Down Expand Up @@ -951,12 +951,6 @@ public async Task ReadAsStreamAsync_HandlerProducesWellBehavedResponseStream(boo
return;
}

if (UseVersion == HttpVersion30 && (chunked is null || chunked is false))
{
// [ActiveIssue("https://github.com/dotnet/runtime/issues/53087")]
return;
}

await LoopbackServerFactory.CreateClientAndServerAsync(async uri =>
{
var request = new HttpRequestMessage(HttpMethod.Get, uri) { Version = UseVersion };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@ private static Task GetAndDropResponse(HttpClient client, Uri url)
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))]
public async Task IncompleteResponseStream_ResponseDropped_CancelsRequestToServer()
{
if (UseVersion == HttpVersion30)
{
// [ActiveIssue("https://github.com/dotnet/runtime/issues/53089")]
return;
}

using (HttpClient client = CreateHttpClient())
{
bool stopGCs = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,12 +214,6 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri =>
[InlineData("RandomCustomHeader", 12345)]
public async Task GetAsync_LargeHeader_Success(string headerName, int headerValueLength)
{
if (UseVersion == HttpVersion.Version30)
{
// [ActiveIssue("https://github.com/dotnet/runtime/issues/55508")]
return;
}

var rand = new Random(42);
string headerValue = string.Concat(Enumerable.Range(0, headerValueLength).Select(_ => (char)('A' + rand.Next(26))));

Expand Down