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: failures with "client: missing connection" in TestTransportBodyReadError_* #44304

Closed
bcmills opened this issue Feb 16, 2021 · 13 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Feb 16, 2021

--- FAIL: TestTransportBodyReadError_Immediately (0.00s)
    transport_test.go:4590: server: ReadFrame = [FrameHeader WINDOW_UPDATE len=4], <nil>
    transport_test.go:774: client: missing connection
    transport_test.go:4590: server: ReadFrame = [FrameHeader HEADERS flags=END_HEADERS stream=1 len=36], <nil>
    transport_test.go:4590: server: ReadFrame = <nil>, read tcp4 127.0.0.1:51605->127.0.0.1:39284: use of closed network connection
FAIL
exitcode=1FAIL	golang.org/x/net/http2	22.996s

2021-01-19T19:43:25-5f4716e/android-amd64-emu
2020-10-02T20:24:02-0a1ea39/android-amd64-emu
2020-05-20T00:47:42-59133d7/android-amd64-emu
2019-12-04T02:50:24-5ee1b9f/aix-ppc64
2019-10-14T21:28:45-da9a3fd/android-amd64-emu
2019-09-26T02:58:31-c00fd9a/android-386-emu
2019-09-09T00:30:24-a7b1673/linux-ppc64-buildlet
2019-08-13T14:13:03-74dc4d7/dragonfly-amd64
2019-07-24T01:30:45-ca1201d/android-386-emu
2019-04-24T11:20:56-4829fb1/linux-arm64-packet
2019-04-03T14:48:56-b630fd6/android-386-emu
2019-03-26T08:36:53-a33f666/linux-amd64-race
2018-11-14T22:03:01-adae6a3/freebsd-arm-paulzhol

CC @bradfitz @tombergan @empijei @neild

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 16, 2021
@networkimprov
Copy link

cc @fraenkel

@fraenkel
Copy link
Contributor

The failure makes sense when the connection is closed. It is unclear who/what is closing the connection though.

@bcmills
Copy link
Contributor Author

bcmills commented Jan 5, 2022

The failure makes sense when the connection is closed. It is unclear who/what is closing the connection though.

Does that theory also match this write: broken pipe failure mode?

--- FAIL: TestTransportBodyReadError_Immediately (0.00s)
    transport_test.go:818: write tcp4 127.0.0.1:50091->127.0.0.1:50092: write: broken pipe
FAIL
FAIL	golang.org/x/net/http2	10.802s

greplogs --dashboard -md -l -e '(?m)FAIL: TestTransportBodyReadError_.*\n( .+)*write: broken pipe' --since=2021-01-01

2022-01-05T18:14:20-5b0dc2d-ab70d7c/darwin-amd64-nocgo

@bcmills
Copy link
Contributor Author

bcmills commented Jan 5, 2022

@bcmills
Copy link
Contributor Author

bcmills commented Jan 5, 2022

The past month or so includes failures on linux/amd64 and darwin/amd64, which are both first-class ports, and x/net/http2 is vendored into net/http.

This is not a new failure mode, but it is occurring frequently enough to be a problem. Marking as release-blocker, but for Go 1.19 (since it isn't new and could take significant time to debug).

@heschi
Copy link
Contributor

heschi commented Mar 16, 2022

ping @neild

@bcmills
Copy link
Contributor Author

bcmills commented Apr 22, 2022

greplogs --dashboard -md -l -e '(?m)FAIL: TestTransportBodyReadError_.*\n( .+\n)* .+(?:client: missing connection|write: broken pipe)' --since=2022-03-30

2022-04-21T19:06:52-1850ba1-aac1d3a/linux-amd64-race
2022-04-18T20:11:49-a630d4f-3df9df8/linux-amd64-race
2022-04-07T20:39:32-749bd19-6f6942e/android-amd64-emu
2022-03-31T22:21:57-de3da57-fb2a9d2/dragonfly-amd64
2022-03-30T19:14:04-de3da57-1ac1658/android-amd64-emu

@bcmills
Copy link
Contributor Author

bcmills commented May 12, 2022

greplogs -l -e '(?m)FAIL: TestTransportBodyReadError_.*\n( .+\n)* .+(?:client: missing connection|write: broken pipe)' --since=2022-04-22
2022-05-11T13:30:43-2871e0c-636c5f0/android-amd64-emu

@bcmills
Copy link
Contributor Author

bcmills commented May 18, 2022

greplogs -l -e '(?m)FAIL: TestTransportBodyReadError_.*\n( .+\n)* .+(?:client: missing connection|write: broken pipe)' --since=2022-05-12
2022-05-16T09:35:17-9564170-2a6e138/linux-ppc64-buildlet
2022-05-16T09:31:42-9564170-3caf67d/dragonfly-amd64

@heschi
Copy link
Contributor

heschi commented Jun 6, 2022

@neild is this OK after beta 1?

@neild neild added the okay-after-beta1 Used by release team to mark a release-blocker issue as okay to resolve either before or after beta1 label Jun 7, 2022
@gopherbot
Copy link

Change https://go.dev/cl/410934 mentions this issue: http2: add newly dialed conns to the pool before signaling completion

@gopherbot gopherbot removed the okay-after-beta1 Used by release team to mark a release-blocker issue as okay to resolve either before or after beta1 label Jun 10, 2022
@rsc rsc unassigned neild Jun 23, 2022
WeiminShang added a commit to WeiminShang/net that referenced this issue Nov 16, 2022
In dialCall.dial, close the done channel to mark dial completion after
adding the new connection to the clientConnPool.

Fixes a race condition in TestTransportBodyReadError, where the client
side of the test could observe the clientConnPool as unexpectedly
containing no conns, because the new conn had not yet been added to
the pool.

Fixes golang/go#44304.

Change-Id: I121200f9aa664fae29d0532e7fa2da47de3fe6a8
Reviewed-on: https://go-review.googlesource.com/c/net/+/410934
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
Development

No branches or pull requests

6 participants