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

x/net/http2: refund server's connection-level flow control for DATA frames the transport receives after sending RST_STREAM #20469

Closed
tombergan opened this issue May 23, 2017 · 5 comments
Assignees
Milestone

Comments

@tombergan
Copy link
Contributor

@tombergan tombergan commented May 23, 2017

The transport writes DATA frames to bufPipe if the transport had not previously sent a RST_STREAM. Flow control will be returned to the server after the caller reads that data out of the transport.

If the transport had previously sent a RST_STREAM, it does nothing when it receives a DATA frame. We should refund the server's connection-level flow control in this case. (We don't need to refund stream-level flow control because the stream has been reset.)

Some discussion here:
https://go-review.googlesource.com/c/43810/5/http2/transport.go#1728

/cc @bradfitz

@tombergan tombergan self-assigned this May 23, 2017
@bradfitz bradfitz added this to the Go1.9 milestone May 23, 2017
@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Jun 14, 2017

Tom, did you want to do this for Go 1.9 still?

@bradfitz bradfitz modified the milestones: Go1.9Maybe, Go1.9 Jun 14, 2017
@bradfitz bradfitz added the NeedsFix label Jun 14, 2017
@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Jun 22, 2017

Tom, welcome back from vacation. This is still game for 1.9 if the fix is easy.

@tombergan

This comment has been minimized.

Copy link
Contributor Author

@tombergan tombergan commented Jun 23, 2017

Thanks for the ping. I'll take a look tomorrow ... should be easy.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jun 23, 2017

CL https://golang.org/cl/46591 mentions this issue.

gopherbot pushed a commit to golang/net that referenced this issue Jun 29, 2017
…eset

If the transport had previously sent a RST_STREAM but had not yet
deleted the stream from its list of active streams, we should refund
connection-level flow control for any DATA frame received as such
DATA frames will never be read.

We already refund connection-level flow control if a stream closes with
unread data in bufPipe. However, when we receive a DATA frame after
reset, we don't bother writing it to bufPipe, so we have to refund the
flow control separately.

Updates golang/go#20469

Change-Id: I5a9810a5d6b1bd7e291173af53646246545a6665
Reviewed-on: https://go-review.googlesource.com/46591
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@bradfitz

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Jun 29, 2017

Fixed by https://golang.org/cl/47096 (which had the wrong bug reference).

@bradfitz bradfitz closed this Jun 29, 2017
c3mb0 pushed a commit to c3mb0/net that referenced this issue Apr 2, 2018
…eset

If the transport had previously sent a RST_STREAM but had not yet
deleted the stream from its list of active streams, we should refund
connection-level flow control for any DATA frame received as such
DATA frames will never be read.

We already refund connection-level flow control if a stream closes with
unread data in bufPipe. However, when we receive a DATA frame after
reset, we don't bother writing it to bufPipe, so we have to refund the
flow control separately.

Updates golang/go#20469

Change-Id: I5a9810a5d6b1bd7e291173af53646246545a6665
Reviewed-on: https://go-review.googlesource.com/46591
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators Jun 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.