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: 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

Comments

@bradfitz
Copy link
Contributor

@bradfitz 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

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jan 9, 2017

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
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.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.