Skip to content

x/net/http2: connection-level flow control not returned if stream errors, causes server hang [1.15 backport] #41387

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
gopherbot opened this issue Sep 14, 2020 · 9 comments
Closed

x/net/http2: connection-level flow control not returned if stream errors, causes server hang [1.15 backport] #41387

gopherbot opened this issue Sep 14, 2020 · 9 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone
Go1.15.4

Comments

@gopherbot
Copy link
Contributor

@ianlancetaylor requested issue #40423 to be considered for backport to the next 1.15 minor release.

@gopherbot Please open backport issues.

This problem can reportedly cause an HTTP/2 connection to hang. There doesn't seem to be any reasonable workaround.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Sep 14, 2020
@gopherbot gopherbot mentioned this issue Sep 14, 2020
Closed
@gopherbot gopherbot added this to the Go1.15.3 milestone Sep 14, 2020
@gopherbot
Copy link
Contributor Author

Change https://golang.org/cl/258478 mentions this issue: [release-branch.go1.15] net/http2: send WINDOW_UPDATE on a body's write failure

@gopherbot
Copy link
Contributor Author

Change https://golang.org/cl/258540 mentions this issue: [release-branch.go1.15] net/http: backport HTTP2 send WINDOW_UPDATE on a body's write failure

@dmitshur
Copy link
Member

dmitshur commented Oct 8, 2020

We've considered this in a release meeting. Approving as it is a serious issue without a workaround. This backport applies to both 1.15 (this issue) and 1.14 (#41386).

@dmitshur dmitshur added CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Oct 8, 2020
@dmitshur dmitshur mentioned this issue Oct 8, 2020
Closed
@gopherbot

This comment has been minimized.

Sign in to view
gopherbot pushed a commit to golang/net that referenced this issue Oct 8, 2020
@fraenkel @dmitshur
[release-branch.go1.15] net/http2: send WINDOW_UPDATE on a body's wri…
a5fa9d4 
…te failure

When the body.Write fails during processData, the connection flow
control must be updated to account for the data received. The connection's
WINDOW_UPDATE should reflect the amount of data that was not successfully
written. The stream is about to be closed, so no update is required.

Updates golang/go#40423.
Fixes golang/go#41387.

Change-Id: I546597cedf3715e6617babcb3b62140bf1857a27
Reviewed-on: https://go-review.googlesource.com/c/net/+/245158
Reviewed-by: Emmanuel Odeke <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Emmanuel Odeke <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Trust: Emmanuel Odeke <[email protected]>
(cherry picked from commit 5d4f700)
Reviewed-on: https://go-review.googlesource.com/c/net/+/258478
Run-TryBot: Dmitri Shuralyov <[email protected]>
Trust: Dmitri Shuralyov <[email protected]>
@dmitshur
Copy link
Member

dmitshur commented Oct 9, 2020

Reopening for updating vendored and bundled copies in the standard library.

@dmitshur dmitshur reopened this Oct 9, 2020
@dmitshur dmitshur reopened this Oct 9, 2020
@gopherbot
Copy link
Contributor Author

Change https://golang.org/cl/261919 mentions this issue: [release-branch.go1.15] src, net/http: update vendor, h2_bundle.go regeneration

@toothrot toothrot modified the milestones: Go1.15.3, Go1.15.4 Oct 14, 2020
@dmitshur dmitshur mentioned this issue Oct 21, 2020
Closed
@gopherbot
Copy link
Contributor Author

Change https://golang.org/cl/264058 mentions this issue: [release-branch.go1.15-bundle] net/http2: send WINDOW_UPDATE on a body's write failure

@gopherbot
Copy link
Contributor Author

Change https://golang.org/cl/264217 mentions this issue: [release-branch.go1.15-bundle] icmp, webdav/internal/xml: avoid string(int)

gopherbot pushed a commit to golang/net that referenced this issue Oct 22, 2020
@bradfitz @dmitshur
[release-branch.go1.15-bundle] icmp, webdav/internal/xml: avoid strin…
d64cff4 
…g(int)

Updates golang/go#32479.
For golang/go#41387.

Change-Id: Ife0c3a2f10afb676af5f2110a9681216122c8808
Reviewed-on: https://go-review.googlesource.com/c/net/+/233900
Run-TryBot: Brad Fitzpatrick <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
(cherry picked from commit a91f071)
Reviewed-on: https://go-review.googlesource.com/c/net/+/264217
Trust: Dmitri Shuralyov <[email protected]>
Run-TryBot: Dmitri Shuralyov <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Alexander Rakoczy <[email protected]>
gopherbot pushed a commit to golang/net that referenced this issue Oct 22, 2020
@fraenkel @dmitshur
[release-branch.go1.15-bundle] net/http2: send WINDOW_UPDATE on a bod…
abf26a1 
…y's write failure

When the body.Write fails during processData, the connection flow
control must be updated to account for the data received. The connection's
WINDOW_UPDATE should reflect the amount of data that was not successfully
written. The stream is about to be closed, so no update is required.

Updates golang/go#40423.
For golang/go#41387.

Change-Id: I546597cedf3715e6617babcb3b62140bf1857a27
Reviewed-on: https://go-review.googlesource.com/c/net/+/245158
Reviewed-by: Emmanuel Odeke <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Emmanuel Odeke <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Trust: Emmanuel Odeke <[email protected]>
(cherry picked from commit 5d4f700)
Reviewed-on: https://go-review.googlesource.com/c/net/+/264058
Run-TryBot: Dmitri Shuralyov <[email protected]>
Reviewed-by: Alexander Rakoczy <[email protected]>
Trust: Dmitri Shuralyov <[email protected]>
gopherbot pushed a commit that referenced this issue Oct 22, 2020
@odeke-em @dmitshur
[release-branch.go1.15] src, net/http: update vendor, regenerate h2_b…
f68af19 
…undle.go

Features CL:

    net/http2: send WINDOW_UPDATE on a body's write failure
    https://golang.org/cl/258478 (updates #41387)

Created by:

go mod edit -replace=golang.org/x/net=golang.org/x/[email protected]
GOFLAGS='-mod=mod' go generate -run=bundle std
go mod edit -dropreplace=golang.org/x/net
go get -d golang.org/x/[email protected]
go mod tidy
go mod vendor

Updates #40423
Fixes #41387

Change-Id: I052037d6b6ed38b9d9782e19b8ce283875354c92
Reviewed-on: https://go-review.googlesource.com/c/go/+/258540
Run-TryBot: Emmanuel Odeke <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Trust: Emmanuel Odeke <[email protected]>
@dmitshur
Copy link
Member

Closed by merging f68af19 to release-branch.go1.15.

@golang golang locked and limited conversation to collaborators Oct 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Projects
None yet
Milestone
Go1.15.4
Development

No branches or pull requests
3 participants
@toothrot @dmitshur @gopherbot