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

Conversation

ManickaP
Copy link
Member

@ManickaP ManickaP commented Apr 23, 2021

Note that this fixes the problem only for the initially set limit of 100 streams. The more general fix and API will follow with #32079. This is mainly to unblock ASP.NET core scenarios.

Fixes #49617

cc: @JamesNK

Fixed testing H/3 loopback stream to properly close. Otherwise, the stream will end up in a bad state and the connection will eventually run out of available streams.
@ghost
Copy link

ghost commented Apr 23, 2021

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Note that this fixes the problem only for initially set limit of 100 streams. The more general fix and API will follow with #32079. This is mainly to unblock ASP.NET core scenarios.

Fixes #49617

cc: @JamesNK

Author: ManickaP
Assignees: -
Labels:

area-System.Net.Http

Milestone: -

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM

}
}

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.

Copy link
Member

@JamesNK JamesNK left a comment

Choose a reason for hiding this comment

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

I'm not familiar with client code. Approving to show my enthusiasm to have this fixed 😄

Once there is a daily SDK with this fix available then I'll test it out.

@ManickaP ManickaP merged commit d3c77a5 into dotnet:main Apr 27, 2021
@ManickaP ManickaP deleted the mapichov/49617_fix branch April 27, 2021 16:13
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HTTP/3: Hang after 100 requests on a connection
4 participants