Skip to content

Commit

Permalink
Fixed non-decreasing count of stream on H3 connection. (#51742)
Browse files Browse the repository at this point in the history
* Fixed HTTP/3 client hang after subsequent 100 streams.

Fixed testing H/3 loopback stream to properly close. Otherwise, the stream will end up in a bad state and the connection will eventually run out of available streams.

* Added assert.
  • Loading branch information
ManickaP committed Apr 27, 2021
1 parent 0ee5cbd commit d3c77a5
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ public async Task SendResponseBodyAsync(byte[] content, bool isFinal = true)
if (isFinal)
{
await ShutdownSendAsync().ConfigureAwait(false);
await _stream.ShutdownCompleted().ConfigureAwait(false);
Dispose();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ private void CancelWaiters()
}
}

// TODO: how do we get this event?
// TODO: how do we get this event? -> HandleEventStreamsAvailable reports currently available Uni/Bi streams
private void OnMaximumStreamCountIncrease(long newMaximumStreamCount)
{
lock (SyncObj)
Expand All @@ -295,15 +295,23 @@ private void OnMaximumStreamCountIncrease(long newMaximumStreamCount)
return;
}

_requestStreamsRemaining += (newMaximumStreamCount - _maximumRequestStreams);
IncreaseRemainingStreamCount(newMaximumStreamCount - _maximumRequestStreams);
_maximumRequestStreams = newMaximumStreamCount;
}
}

private void IncreaseRemainingStreamCount(long delta)
{
Debug.Assert(Monitor.IsEntered(SyncObj));
Debug.Assert(delta > 0);

while (_requestStreamsRemaining != 0 && _waitingRequests.TryDequeue(out TaskCompletionSourceWithCancellation<bool>? tcs))
_requestStreamsRemaining += delta;

while (_requestStreamsRemaining != 0 && _waitingRequests.TryDequeue(out TaskCompletionSourceWithCancellation<bool>? tcs))
{
if (tcs.TrySetResult(true))
{
if (tcs.TrySetResult(true))
{
--_requestStreamsRemaining;
}
--_requestStreamsRemaining;
}
}
}
Expand Down Expand Up @@ -406,6 +414,8 @@ public void RemoveStream(QuicStream stream)
bool removed = _activeRequests.Remove(stream);
Debug.Assert(removed == true);

IncreaseRemainingStreamCount(1);

if (ShuttingDown)
{
CheckForShutdown();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ private async Task ReadResponseAsync(CancellationToken cancellationToken)
{
if (NetEventSource.Log.IsEnabled())
{
Trace($"Expected HEADERS as first response frame; recieved {frameType}.");
Trace($"Expected HEADERS as first response frame; received {frameType}.");
}
throw new HttpRequestException(SR.net_http_invalid_response);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,43 @@ public async Task ClientSettingsReceived_Success(int headerSizeLimit)
await new[] { clientTask, serverTask }.WhenAllOrAnyFailed(20_000);
}

[Theory]
[InlineData(100)]
public async Task SendMoreThanStreamLimitRequests_Succeeds(int streamLimit)
{
using Http3LoopbackServer server = CreateHttp3LoopbackServer();

Task serverTask = Task.Run(async () =>
{
using Http3LoopbackConnection connection = (Http3LoopbackConnection)await server.EstablishGenericConnectionAsync();
for (int i = 0; i < streamLimit + 1; ++i)
{
using Http3LoopbackStream stream = await connection.AcceptRequestStreamAsync();
await stream.HandleRequestAsync();
}
});

Task clientTask = Task.Run(async () =>
{
using HttpClient client = CreateHttpClient();
for (int i = 0; i < streamLimit + 1; ++i)
{
using HttpRequestMessage request = new()
{
Method = HttpMethod.Get,
RequestUri = server.Address,
Version = HttpVersion30,
VersionPolicy = HttpVersionPolicy.RequestVersionExact
};
using var response = await client.SendAsync(request).WaitAsync(TimeSpan.FromSeconds(10));
}
});

await new[] { clientTask, serverTask }.WhenAllOrAnyFailed(20_000);
}


[Fact]
public async Task ReservedFrameType_Throws()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ internal override async ValueTask<int> ReadAsync(Memory<byte> destination, Cance
}
}, _state);

// TODO there could potentially be a perf gain by storing the buffer from the inital read
// TODO there could potentially be a perf gain by storing the buffer from the initial read
// This reduces the amount of async calls, however it makes it so MsQuic holds onto the buffers
// longer than it needs to. We will need to benchmark this.
int length = (int)await _state.ReceiveResettableCompletionSource.GetValueTask().ConfigureAwait(false);
Expand Down

0 comments on commit d3c77a5

Please sign in to comment.