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.

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

Server and client are synchronized via TaskCompletionSource #35569

Merged
merged 1 commit into from
Apr 28, 2020
Merged

Server and client are synchronized via TaskCompletionSource #35569

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
9 changes: 6 additions & 3 deletions src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1636,6 +1636,7 @@ public async Task Http2_PendingSend_Cancellation()
public async Task Http2_PendingSend_SendsReset(bool waitForData)
{
var cts = new CancellationTokenSource();
var rstReceived = new TaskCompletionSource<bool>();

string content = new string('*', 300);
var stream = new CustomContent.SlowTestStream(Encoding.UTF8.GetBytes(content), null, count: 60);
Expand All @@ -1649,8 +1650,8 @@ await Http2LoopbackServer.CreateClientAndServerAsync(async url =>

await Assert.ThrowsAnyAsync<OperationCanceledException>(async () => await client.SendAsync(request, cts.Token));

// Delay for a bit to ensure that the RST_STREAM for the previous request is sent before the next request starts.
await Task.Delay(2000);
// Wait until the RST_STREAM for the previous request is received before the next request starts.
await rstReceived.Task.TimeoutAfter(TimeSpan.FromSeconds(60));
Copy link
Member

Choose a reason for hiding this comment

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

60

Do we have a standard timeout constant defined anywhere that we could use for consistency (and the ability to change all timeouts at once later) instead of hardcoding the 60?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I already tried searching for such constant, but with no luck.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On the second thought, I found Http2LoopbackServer.Timeout, but not sure if it's a right thing to use it here.

Copy link
Member

Choose a reason for hiding this comment

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

there is TestConfiguration.PassingTestTimeoutMilliseconds but that may be too long. That generally puts cap on test execution. I think using loopback timeout would be fine in this case as it does not need to precise, right @alnikola? Maybe we can add something DefaultTimeout to loopback and than use it when appropriate. (like the Http2LoopbackServer.Timeout) That would allows us to adjust it globally if we ever need to on slow platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will try to find all places where we use an explicit timeout and put there some well-known value (e.g. TestConfiguration.PassingTestTimeoutMilliseconds or Http2LoopbackServer.Timeout). Will create a separate PR for this.


// Send another request to verify that connection is still functional.
request = new HttpRequestMessage(HttpMethod.Get, url);
Expand Down Expand Up @@ -1678,7 +1679,9 @@ await Http2LoopbackServer.CreateClientAndServerAsync(async url =>
frameCount++;
} while (frame.Type != FrameType.RstStream);

Assert.Equal(1, frame.StreamId);
Assert.Equal(1, frame.StreamId);

rstReceived.SetResult(true);

frame = null;
(streamId, requestData) = await connection.ReadAndParseRequestHeaderAsync();
Expand Down