-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[HTTP/3] Fix handling cancelation during QUIC connection establishment #87971
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
Conversation
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsWhen a new Discovered in stress test as they randomly cancel requests. Fixes #87957
|
|
/azp run runtime-libraries stress-http |
|
Azure Pipelines successfully started running 1 pipeline(s). |
CarnaViire
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
Outdated
Show resolved
Hide resolved
|
/azp run runtime-libraries stress-http |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-libraries stress-http |
|
Azure Pipelines successfully started running 1 pipeline(s). |
wfurt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Stress is green. |
When a new
QuicConnectionis being established for HTTP/3 and it fails, we block the remote authority and remember why. Any further attempt to connect to that authority gets failed with that reason for a while.We shouldn't do that if the failure is because we decided to stop the attempt, i.e. we (timeout) or user cancelled the passed in token.
Discovered in stress test as they randomly cancel requests.
Fixes #87957