Skip to content

x/net/http2: TestUnreadFlowControlReturned_Server failures with stream error "NO_ERROR" since 2021-10-05 [1.17 backport] #49662

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 Nov 18, 2021 · 6 comments
Closed

x/net/http2: TestUnreadFlowControlReturned_Server failures with stream error "NO_ERROR" since 2021-10-05 [1.17 backport] #49662

gopherbot opened this issue Nov 18, 2021 · 6 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone
Go1.17.4

Comments

@gopherbot
Copy link
Contributor

@neild requested issue #49645 to be considered for backport to the next 1.17 minor release.

@gopherbot Please open backport issues for 1.16 and 1.17. This creates test flakiness.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Nov 18, 2021
@gopherbot gopherbot mentioned this issue Nov 18, 2021
Closed
@gopherbot gopherbot added this to the Go1.17.4 milestone Nov 18, 2021
@dmitshur
Copy link
Contributor

dmitshur commented Dec 1, 2021

We've discussed this and 4 more net/http (and x/net/http2) backports in a release meeting, and agreed to backport them. They are smaller regressions from earlier backports for serious net/http issues. Both Go 1.17 and 1.16 are getting these fixes.

@dmitshur dmitshur added CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Dec 1, 2021
@gopherbot
Copy link
Contributor Author

Change https://golang.org/cl/368379 mentions this issue: [internal-branch.go1.17-vendor] http2: avoid spurious RoundTrip error when server closes and resets stream

gopherbot pushed a commit to golang/net that referenced this issue Dec 1, 2021
@neild @mknyszek
[internal-branch.go1.17-vendor] http2: avoid spurious RoundTrip error…
94fb2bc 
… when server closes and resets stream

Avoid a race condition between RoundTrip and the read loop when the
read loop reads a response followed by an immediate stream reset.

For golang/go#49645
Fixes golang/go#49662

Change-Id: Ifb34e2dcb8cc443d3ff5d562cc032edf09da5307
Reviewed-on: https://go-review.googlesource.com/c/net/+/364834
Trust: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 6a13c67)
Reviewed-on: https://go-review.googlesource.com/c/net/+/368379
Reviewed-by: Michael Knyszek <mknyszek@google.com>
@mknyszek
Copy link
Contributor

mknyszek commented Dec 1, 2021

Landed the fix.

@mknyszek mknyszek closed this as completed Dec 1, 2021
@dmitshur
Copy link
Contributor

dmitshur commented Dec 1, 2021

@mknyszek For visibility, the backports in x/net's internal-branch.go1.x-vendor branch will also need to make their way into the main Go repository's release-branches as described in https://go.dev/wiki/MinorReleases#cherry-pick-cls-for-vendored-golangorgx-packages.

It might be convenient to rely on the Fixes golang/go#49662. line of the main repo's release-branch CL that pulls in the fix to close this backport issue.

@mknyszek
Copy link
Contributor

mknyszek commented Dec 1, 2021

Ah, shoot OK. I missed that.

@mknyszek mknyszek reopened this Dec 1, 2021
@gopherbot
Copy link
Contributor Author

Change https://golang.org/cl/368574 mentions this issue: [release-branch.go1.17] net/http: update bundled golang.org/x/net/http2

@mknyszek mknyszek closed this as completed Dec 2, 2021
gopherbot pushed a commit that referenced this issue Dec 2, 2021
@mknyszek
[release-branch.go1.17] net/http: update bundled golang.org/x/net/http2
0177913 
Pull in approved backports to golang.org/x/net/http2:

    85e122b net/http2: Fix handling of expect continue
    1dc0aec http2: don't count aborted streams as active in tests
    e973a42 ipv6: OpenBSD does not appear to support multicast loopback
    9592dd5 http2: avoid busy loop when ResponseHeaderTimeout is set
    94fb2bc http2: avoid spurious RoundTrip error when server closes and resets stream
    e108c19 http2: close conns after use when req.Close is set

By doing:

    $ go get -d golang.org/x/net@internal-branch.go1.17-vendor
    go: upgraded golang.org/x/net v0.0.0-20211101194204-95aca89e93de => v0.0.0-20211201233630-85e122b1a9b3
    $ go mod tidy
    $ go mod vendor
    $ go generate -run=bundle std

Fixes #49561.
Fixes #49624.
Fixes #49662.
Fixes #49905.
Fixes #49909.
Fixes #49911.

Change-Id: Ia8f432bd3ea77d24e63d46c8ed2ac8d275406b52
Reviewed-on: https://go-review.googlesource.com/c/go/+/368574
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
fedosgad pushed a commit to fedosgad/oohttp that referenced this issue Jun 22, 2022
@neild @mknyszek
[internal-branch.go1.17-vendor] http2: avoid spurious RoundTrip error…
64ac61a 
… when server closes and resets stream

Avoid a race condition between RoundTrip and the read loop when the
read loop reads a response followed by an immediate stream reset.

For golang/go#49645
Fixes golang/go#49662

Change-Id: Ifb34e2dcb8cc443d3ff5d562cc032edf09da5307
Reviewed-on: https://go-review.googlesource.com/c/net/+/364834
Trust: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 6a13c67c3ce400be1b91076053a994c2d1ebf01b)
Reviewed-on: https://go-review.googlesource.com/c/net/+/368379
Reviewed-by: Michael Knyszek <mknyszek@google.com>
@golang golang locked and limited conversation to collaborators Dec 2, 2022
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.17.4
Development

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