Navigation Menu

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

Jump to bottom

x/net/http2: flaky TestServer_Push_StateTransitions #18559

Closed
bradfitz opened this issue Jan 7, 2017 · 1 comment
Closed

x/net/http2: flaky TestServer_Push_StateTransitions #18559

bradfitz opened this issue Jan 7, 2017 · 1 comment
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
Go1.9

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Jan 7, 2017

Saw this test failure on a trybot run:

https://storage.googleapis.com/go-build-log/8fa2344e/freebsd-amd64-gce101_341f01a5.log

--- FAIL: TestServer_Push_StateTransitions (0.01s)
	server_push_test.go:463: streamState(2)=Closed, want HalfClosedRemote
	server_test.go:229: Framer read log:
		2017-01-07 09:59:52.514728007 Framer 0xc42117c8f0: read SETTINGS len=18, settings: MAX_FRAME_SIZE=1048576, MAX_CONCURRENT_STREAMS=250, MAX_HEADER_LIST_SIZE=1048896
		2017-01-07 09:59:52.515457593 Framer 0xc42117c8f0: read SETTINGS flags=ACK len=0
	server_test.go:235: Framer write log:
		2017-01-07 09:59:52.51464267 Framer 0xc42117c8f0: wrote SETTINGS len=0
		2017-01-07 09:59:52.51474176 Framer 0xc42117c8f0: wrote SETTINGS flags=ACK len=0
		2017-01-07 09:59:52.515648389 Framer 0xc42117c8f0: wrote HEADERS flags=END_STREAM|END_HEADERS stream=1 len=16
FAIL
FAIL	golang.org/x/net/http2	7.252s

/cc @tombergan
@bradfitz bradfitz added 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. labels Jan 7, 2017
@bradfitz bradfitz added this to the Go1.9 milestone Jan 7, 2017
@gopherbot
Copy link

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

@golang golang locked and limited conversation to collaborators Jan 9, 2018
c3mb0 pushed a commit to c3mb0/net that referenced this issue Apr 2, 2018
@tombergan @bradfitz
http2: fix flake in TestServer_Push_StateTransitions
962c58c 
I believe there were two bugs, both fixed by this CL.

* Previously, we checked stateHalfClosedRemote before waiting for the
  PUSH_PROMISE. However, the pushed stream is not created until the promise
  is written, so the stream may not have started yet, which means we'd see
  stateIdle instead of stateHalfClosedRemote.

* The push reponse handler cannot write the response until after we
  check the pushed stream state. Otherwise, the response might finish
  just before we check the stream state and we'll see stateClosed instead
  of stateHalfClosedRemote.

Test passes with -count 1000.

Fixes golang/go#18559

Change-Id: I61f62635957e061fba905a41dcb15cd4e563031a
Reviewed-on: https://go-review.googlesource.com/34984
TryBot-Result: Gobot Gobot <gobot@golang.org>
Run-TryBot: Tom Bergan <tombergan@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
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
Milestone
Go1.9
Development

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