Commit 4aaac4bb authored by Tom Bergan's avatar Tom Bergan Committed by Brad Fitzpatrick

http2: fix flake in TestServer_Push_StateTransitions

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: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
parent b03f0d9a
...@@ -431,7 +431,9 @@ func TestServer_Push_StateTransitions(t *testing.T) { ...@@ -431,7 +431,9 @@ func TestServer_Push_StateTransitions(t *testing.T) {
const body = "foo" const body = "foo"
startedPromise := make(chan bool) startedPromise := make(chan bool)
gotPromise := make(chan bool)
finishedPush := make(chan bool) finishedPush := make(chan bool)
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) { st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
switch r.URL.RequestURI() { switch r.URL.RequestURI() {
case "/": case "/":
...@@ -442,6 +444,8 @@ func TestServer_Push_StateTransitions(t *testing.T) { ...@@ -442,6 +444,8 @@ func TestServer_Push_StateTransitions(t *testing.T) {
// Don't finish this request until the push finishes so we don't // Don't finish this request until the push finishes so we don't
// nondeterministically interleave output frames with the push. // nondeterministically interleave output frames with the push.
<-finishedPush <-finishedPush
case "/pushed":
<-gotPromise
} }
w.Header().Set("Content-Type", "text/html") w.Header().Set("Content-Type", "text/html")
w.Header().Set("Content-Length", strconv.Itoa(len(body))) w.Header().Set("Content-Length", strconv.Itoa(len(body)))
...@@ -459,10 +463,11 @@ func TestServer_Push_StateTransitions(t *testing.T) { ...@@ -459,10 +463,11 @@ func TestServer_Push_StateTransitions(t *testing.T) {
} }
getSlash(st) getSlash(st)
<-startedPromise <-startedPromise
st.wantPushPromise()
if got, want := st.streamState(2), stateHalfClosedRemote; got != want { if got, want := st.streamState(2), stateHalfClosedRemote; got != want {
t.Fatalf("streamState(2)=%v, want %v", got, want) t.Fatalf("streamState(2)=%v, want %v", got, want)
} }
st.wantPushPromise() close(gotPromise)
st.wantHeaders() st.wantHeaders()
if df := st.wantData(); !df.StreamEnded() { if df := st.wantData(); !df.StreamEnded() {
t.Fatal("expected END_STREAM flag on DATA") t.Fatal("expected END_STREAM flag on DATA")
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment