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: delayed test failure in TestTransportPingWhenReading #41299

Closed
bcmills opened this issue Sep 9, 2020 · 3 comments
Closed

x/net/http2: delayed test failure in TestTransportPingWhenReading #41299

bcmills opened this issue Sep 9, 2020 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Sep 9, 2020

panic: Fail in goroutine after TestTransportPingWhenReading/two_pings_in_each_serverResponseInterval has completed

goroutine 14781 [running]:
testing.(*common).Fail(0xba2bb5e0)
	/workdir/go/src/testing/testing.go:680 +0x105
testing.(*common).Error(0xba2bb5e0, 0xb8bbcfcc, 0x1, 0x1)
	/workdir/go/src/testing/testing.go:780 +0x62
golang.org/x/net/http2.testTransportPingWhenReading.func2.1(0xb8c80498, 0xb894a5a0, 0xb8922da0, 0xba2bb5e0, 0x3b9aca00, 0x0)
	/workdir/gopath/src/golang.org/x/net/http2/transport_test.go:3514 +0x30b
created by golang.org/x/net/http2.testTransportPingWhenReading.func2
	/workdir/gopath/src/golang.org/x/net/http2/transport_test.go:3500 +0x33d
exitcode=2FAIL	golang.org/x/net/http2	26.736s

2020-09-04T19:48:48-62affa3/android-386-emu
2020-09-04T19:48:48-62affa3/linux-ppc64le-buildlet
2020-08-22T12:43:28-c890458/dragonfly-amd64-5_8
2020-08-22T12:43:28-c890458/freebsd-arm-paulzhol
2020-08-22T12:43:28-c890458/freebsd-arm64-dmgk
2020-08-22T12:43:28-c890458/linux-arm
2020-08-22T12:43:28-c890458/linux-ppc64le-buildlet
2020-08-13T13:45:08-3edf25e/darwin-amd64-10_12
2020-08-13T13:45:08-3edf25e/darwin-amd64-nocgo
2020-08-13T13:45:08-3edf25e/freebsd-arm64-dmgk
2020-08-13T13:45:08-3edf25e/linux-ppc64-buildlet
2020-08-13T13:45:08-3edf25e/linux-ppc64le-buildlet
2020-07-07T03:43:11-ab34263/freebsd-arm64-dmgk
2020-06-25T00:16:55-4c52546/linux-ppc64-buildlet
2020-06-25T00:16:55-4c52546/linux-s390x-ibm
2020-06-25T00:16:55-4c52546/plan9-386-0intro
2020-06-25T00:16:55-4c52546/plan9-amd64-9front
2020-06-02T11:40:24-627f964/dragonfly-amd64
2020-06-02T11:40:24-627f964/freebsd-arm64-dmgk
2020-06-02T11:40:24-627f964/linux-mips-rtrk
2020-06-02T11:40:24-627f964/linux-s390x-ibm
2020-06-02T11:40:24-627f964/plan9-386-0intro
2020-06-02T11:40:24-627f964/plan9-amd64-9front
2020-06-02T11:40:24-627f964/solaris-amd64-oraclerel
2020-05-28T22:51:25-3c3fba1/freebsd-arm64-dmgk
2020-05-28T22:51:25-3c3fba1/linux-ppc64le-buildlet
2020-05-20T18:23:14-0ba52f6/android-386-emu
2020-05-20T18:23:14-0ba52f6/darwin-amd64-10_12
2020-05-20T18:23:14-0ba52f6/darwin-amd64-10_15
2020-05-20T18:23:14-0ba52f6/freebsd-arm64-dmgk
2020-05-20T18:23:14-0ba52f6/linux-ppc64-buildlet
2020-05-20T18:23:14-0ba52f6/linux-ppc64le-buildlet
2020-05-20T18:23:14-0ba52f6/plan9-386-0intro

It looks like the test server starts a goroutine but does not wait for it to complete before the server function exits.

CC @caesarxuchao @bradfitz @fraenkel @neild

@bcmills bcmills added Testing An issue that has been verified to require only test changes, not just a test failure. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 9, 2020
@bcmills bcmills added this to the Backlog milestone Sep 9, 2020
@bcmills bcmills changed the title x/net/http/http2: delayed test failure in TestTransportPingWhenReading x/net/http2: delayed test failure in TestTransportPingWhenReading Sep 9, 2020
@gopherbot
Copy link

Change https://golang.org/cl/254278 mentions this issue: net/http2: wait for test goroutines to complete

@bcmills bcmills reopened this Nov 4, 2020
@gopherbot
Copy link

Change https://golang.org/cl/267760 mentions this issue: net/http2: wait for both client and server funcs

@golang golang locked and limited conversation to collaborators Nov 9, 2021
dteh pushed a commit to dteh/fhttp that referenced this issue Jun 22, 2022
Don't assume goroutines are gone immediately. Any error that occurs will
cause a panic after the test completes.

Fixes golang/go#41299

Change-Id: I00d3e246fd0b265a33a5dc1a536a33866df30d75
Reviewed-on: https://go-review.googlesource.com/c/net/+/254278
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Chao Xu <xuchao@google.com>
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
dteh pushed a commit to dteh/fhttp that referenced this issue Jun 22, 2022
When executing the clienttester.run(), it would exit when both
the client and server completed successfully or if either side failed.
If one side fails, the other side may not have finished. If an error was
reported, the goroutine was caught executing after the test completed.

The expectation is always that both sides complete successfully. In the
case where one side fails, we still need to wait for the other side to
complete.

Close both client and server connections on error to expedite the
completion.

Fixes golang/go#41299

Change-Id: If47fbe61de42495bb2b1327bd5b03d6c295670dc
Reviewed-on: https://go-review.googlesource.com/c/net/+/267760
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
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. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

2 participants