Skip to content

x/net/http2: half-closed streams not counted against client stream limit #48564

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

Closed
neild opened this issue Sep 23, 2021 · 2 comments
Closed

x/net/http2: half-closed streams not counted against client stream limit #48564

neild opened this issue Sep 23, 2021 · 2 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.

Comments

@neild
Copy link
Contributor

neild commented Sep 23, 2021

(*clientConnReadLoop).processData removes a stream from the ClientConn.streams map when receiving a DATA frame with the END_STREAM flag set:
https://go.googlesource.com/net/+/978cfadd31cf6299758dbb5165d451ce91989f1a/http2/transport.go#2306

Removing the stream from the map opens up a new slot for a stream, since ClientConn considers len(cc.streams) to be the number of streams counting against the connection stream limit:
https://go.googlesource.com/net/+/978cfadd31cf6299758dbb5165d451ce91989f1a/http2/transport.go#1257

However, the stream may be in the half-closed state at this time. A stream only becomes fully closed once both sides send an END_STREAM flag or at least one side sends a RST_STREAM frame. RFC 7540, 5.1.2 states that half-closed streams count against the stream limit.

The client should instead continue to count the stream against the limit until it has been closed on both sides.

@neild
Copy link
Contributor Author

neild commented Sep 23, 2021

Upon closer inspection, I suspect that closing out the stream here may be incorrect.

If we receive a frame with the END_STREAM flag set, that indicates that the remote endpoint has stopped sending us data. However, this doesn't mean the remote endpoint isn't still reading the data we're sending to it. In the case where we're writing a request body, we shouldn't necessarily stop just because the other side is done with its response. (If they wanted us to stop sending, they're presumably have sent RST_STREAM instead.)

@dr2chase dr2chase added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 24, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/353870 mentions this issue: all: update golang.org/x/net to pull in CL 353390

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

3 participants