Skip to content

HTTP/3: Fix incorrectly pooling aborted streams #35434

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

Merged
merged 1 commit into from
Aug 18, 2021

Conversation

JamesNK
Copy link
Member

@JamesNK JamesNK commented Aug 18, 2021

@JamesNK JamesNK added area-runtime tell-mode Indicates a PR which is being merged during tell-mode labels Aug 18, 2021
@JamesNK JamesNK added this to the 6.0-rc1 milestone Aug 18, 2021
@JamesNK JamesNK changed the base branch from main to release/6.0-rc1 August 18, 2021 00:22
@@ -514,6 +514,69 @@ public async Task GET_MultipleRequestsInSequence_ReusedState()
}
}

[ConditionalFact]
[MsQuicSupported]
public async Task StreamResponseContent_DelayAndTrailers_ClientSuccess()
Copy link
Member Author

Choose a reason for hiding this comment

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

Un-related unit test. IMO no harm in adding it. These tests are all quarantined.

@JamesNK JamesNK removed this from the 6.0-rc1 milestone Aug 18, 2021
@JamesNK JamesNK changed the base branch from release/6.0-rc1 to main August 18, 2021 02:23
@@ -115,8 +115,7 @@ public async Task ClientCertificate_Required_NotSent_ConnectionAborted()

// https://github.com/dotnet/runtime/issues/57246 The accept still completes even though the connection was rejected, but it's already failed.
var serverContext = await connectionListener.AcceptAndAddFeatureAsync().DefaultTimeout();
qex = await Assert.ThrowsAsync<QuicException>(() => serverContext.ConnectAsync().DefaultTimeout());
Assert.Equal("Failed to open stream to peer. Error Code: INVALID_STATE", qex.Message);
await Assert.ThrowsAsync<QuicException>(() => serverContext.ConnectAsync().DefaultTimeout());
Copy link
Member

Choose a reason for hiding this comment

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

What's the new exception?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't remember exactly. It was something like "Stream creation error BLAH BLAH BLAH" 😄

@JamesNK JamesNK force-pushed the jamesnk/http3-quicstream-pooling-fix branch from 667fc58 to e244296 Compare August 18, 2021 20:30
@JamesNK JamesNK enabled auto-merge (squash) August 18, 2021 21:34
@JamesNK JamesNK merged commit 5fa81d8 into main Aug 18, 2021
@JamesNK JamesNK deleted the jamesnk/http3-quicstream-pooling-fix branch August 18, 2021 21:50
@ghost ghost added this to the 7.0-preview1 milestone Aug 18, 2021
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[HTTP/3] Server sometimes doesn't receive notification of the end of request stream
4 participants