Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Closed
tombergan opened this issue May 23, 2017 · 5 comments
Closed

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

tombergan opened this issue May 23, 2017 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.9Maybe

Comments

@tombergan
Copy link
Contributor

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
Copy link
Contributor

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 The path to resolution is known, but the work has not been done. label Jun 14, 2017
@bradfitz
Copy link
Contributor

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

@tombergan
Copy link
Contributor Author

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

@gopherbot
Copy link
Contributor

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

gopherbot pushed a commit to golang/net that referenced this issue Jun 29, 2017
@tombergan
http2: refund connection flow control on DATA frames received after r…
d4223d6 
…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
Copy link
Contributor

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

c3mb0 pushed a commit to c3mb0/net that referenced this issue Apr 2, 2018
@tombergan
http2: refund connection flow control on DATA frames received after r…
21ea30d 
…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.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.9Maybe
Development

No branches or pull requests
3 participants
@bradfitz @gopherbot @tombergan