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

Fixed non-decreasing count of stream on H3 connection. #51742

Merged
merged 3 commits into from
Apr 27, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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 @@ -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,22 @@ private void OnMaximumStreamCountIncrease(long newMaximumStreamCount)
return;
}

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

private void IncreaseRemainingStreamCount(long delta)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if delta is negative "AdjustRemainingStreamCount" may be better name.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's never <= 0. I added an assert to express that.

{
Debug.Assert(Monitor.IsEntered(SyncObj));

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 +413,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