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

QUIC stream limits #52704

Merged
merged 24 commits into from
Jun 6, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
9b107cd
Stream limits are ints since they cannot be greater than ushort.MaxVa…
ManickaP May 6, 2021
fae4adc
Stream opening APIs made async.
ManickaP May 6, 2021
f74915e
Proper implementation of OpenStreamAsync, waiting for the msquic event.
ManickaP May 10, 2021
8e92adb
WaitForAvailable*StreamsAsync
ManickaP May 10, 2021
dbb626d
WaitForAvailable*StreamsAsync implementation
ManickaP May 11, 2021
20a5eee
WaitForAvailable*StreamsAsync usage in H3
ManickaP May 11, 2021
fdc4575
WaitForAvailable*StreamsAsync usage in H3 - fixed existing tests
ManickaP May 13, 2021
d9c087d
Added and fixed H/3 test.
ManickaP May 13, 2021
622aa8f
Merge branch 'main' into mapichov/32079_stream_limit
ManickaP May 14, 2021
30efc26
Post merge code fix
ManickaP May 14, 2021
47358ef
Added some more tests and fixed token source
ManickaP May 14, 2021
ea92437
Fixed mock implementation and fixed completion source in msquic one.
ManickaP May 20, 2021
36567a0
Merge branch 'main' into mapichov/32079_stream_limit
ManickaP May 21, 2021
04c559d
Fixed active stream counting in mock connection.
ManickaP May 21, 2021
6bd9ea7
Refactored stream limit work in mocks.
ManickaP May 21, 2021
78b526d
Addressed PR comments.
ManickaP May 24, 2021
9e64240
Merge branch 'main' into mapichov/32079_stream_limit
ManickaP May 24, 2021
754be6f
Addressed feedback about stream openning.
ManickaP May 25, 2021
c5bb854
Added quic tests for WaitForAvailableStreamAsync.
ManickaP May 26, 2021
910ab9f
Merge branch 'main' into mapichov/32079_stream_limit
ManickaP May 26, 2021
97c4e53
Fixed access to _connection only under lock.
ManickaP May 28, 2021
97b3185
Merge branch 'main' into mapichov/32079_stream_limit
ManickaP Jun 1, 2021
cbdd1df
Disabled one misbehaving test.
ManickaP Jun 3, 2021
7eb7fb1
Addressed feedback.
ManickaP Jun 3, 2021
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 @@ -63,7 +63,6 @@ private sealed class State
// Set once writes have been shutdown.
public readonly TaskCompletionSource ShutdownWriteCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);


public ShutdownState ShutdownState;

// Set once stream have been shutdown.
Expand Down
50 changes: 50 additions & 0 deletions src/libraries/System.Net.Quic/tests/FunctionalTests/MsQuicTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,56 @@ public async Task UnidirectionalAndBidirectionalChangeValues()
Assert.Equal(20, serverConnection.GetRemoteAvailableUnidirectionalStreamCount());
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/52048")]
Copy link
Member Author

Choose a reason for hiding this comment

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

These 2 new tests pass on their own but if run within the whole suite they cause the crash with stream being finalized after the connection. That should be fixed with #52800, I'll revisit it enabling them afterwards.

Copy link
Member

Choose a reason for hiding this comment

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

We discussed offline that it is caused by not accepted stream getting stuck inside accept queue and not being disposed there. You can try accepting it (after all test checks if it's important for the test) and disposing manually, or you may wait for Tomas' fix, your choice.

public async Task WaitForAvailableUnidirectionStreamsAsyncWorks()
{
using QuicListener listener = CreateQuicListener(maxUnidirectionalStreams: 1);
using QuicConnection clientConnection = CreateQuicConnection(listener.ListenEndPoint);

ValueTask clientTask = clientConnection.ConnectAsync();
using QuicConnection serverConnection = await listener.AcceptConnectionAsync();
await clientTask;

// No stream openned yet, should return immediately.
Assert.True(clientConnection.WaitForAvailableUnidirectionalStreamsAsync().IsCompletedSuccessfully);

// Open one stream, should wait till it closes.
QuicStream stream = clientConnection.OpenUnidirectionalStream();
ValueTask waitTask = clientConnection.WaitForAvailableUnidirectionalStreamsAsync();
Assert.False(waitTask.IsCompleted);
Assert.Throws<QuicException>(() => clientConnection.OpenUnidirectionalStream());

// Close the stream, the waitTask should finish as a result.
stream.Dispose();
await waitTask.AsTask().WaitAsync(TimeSpan.FromSeconds(10));
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/52048")]
public async Task WaitForAvailableBidirectionStreamsAsyncWorks()
{
using QuicListener listener = CreateQuicListener(maxBidirectionalStreams: 1);
using QuicConnection clientConnection = CreateQuicConnection(listener.ListenEndPoint);

ValueTask clientTask = clientConnection.ConnectAsync();
using QuicConnection serverConnection = await listener.AcceptConnectionAsync();
await clientTask;

// No stream openned yet, should return immediately.
Assert.True(clientConnection.WaitForAvailableBidirectionalStreamsAsync().IsCompletedSuccessfully);

// Open one stream, should wait till it closes.
QuicStream stream = clientConnection.OpenBidirectionalStream();
ValueTask waitTask = clientConnection.WaitForAvailableBidirectionalStreamsAsync();
Assert.False(waitTask.IsCompleted);
Assert.Throws<QuicException>(() => clientConnection.OpenBidirectionalStream());

// Close the stream, the waitTask should finish as a result.
stream.Dispose();
await waitTask.AsTask().WaitAsync(TimeSpan.FromSeconds(10));
}

[Fact]
[OuterLoop("May take several seconds")]
public async Task SetListenerTimeoutWorksWithSmallTimeout()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,30 @@ internal QuicConnection CreateQuicConnection(IPEndPoint endpoint)
return new QuicConnection(ImplementationProvider, endpoint, GetSslClientAuthenticationOptions());
}

internal QuicListener CreateQuicListener()
internal QuicListener CreateQuicListener(int maxUnidirectionalStreams = 100, int maxBidirectionalStreams = 100)
{
return CreateQuicListener(new IPEndPoint(IPAddress.Loopback, 0));
var options = new QuicListenerOptions()
{
ListenEndPoint = new IPEndPoint(IPAddress.Loopback, 0),
ServerAuthenticationOptions = GetSslServerAuthenticationOptions(),
MaxUnidirectionalStreams = maxUnidirectionalStreams,
MaxBidirectionalStreams = maxBidirectionalStreams
};
return CreateQuicListener(options);
}

internal QuicListener CreateQuicListener(IPEndPoint endpoint)
{
return new QuicListener(ImplementationProvider, endpoint, GetSslServerAuthenticationOptions());
var options = new QuicListenerOptions()
{
ListenEndPoint = endpoint,
ServerAuthenticationOptions = GetSslServerAuthenticationOptions()
};
return CreateQuicListener(options);
}

private QuicListener CreateQuicListener(QuicListenerOptions options) => new QuicListener(ImplementationProvider, options);

internal async Task RunClientServer(Func<QuicConnection, Task> clientFunction, Func<QuicConnection, Task> serverFunction, int iterations = 1, int millisecondsTimeout = 10_000)
{
using QuicListener listener = CreateQuicListener();
Expand Down