From 931fe39400006e6c272d13f2a4d46de2a6736761 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Tue, 10 Dec 2019 10:02:27 -0500 Subject: [PATCH] net/http: await state traces earlier in TestServerConnState This approach attempts to ensure that the log for each connection is complete before the next sequence of states begins. Updates #32329 Change-Id: I25150d3ceab6568af56a40d2b14b5f544dc87f61 Reviewed-on: https://go-review.googlesource.com/c/go/+/210717 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/net/http/serve_test.go | 145 +++++++++++++++++++------------------ 1 file changed, 76 insertions(+), 69 deletions(-) diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go index 63ae0fe6d8d..29b937993e0 100644 --- a/src/net/http/serve_test.go +++ b/src/net/http/serve_test.go @@ -34,7 +34,6 @@ import ( "regexp" "runtime" "runtime/debug" - "sort" "strconv" "strings" "sync" @@ -4116,14 +4115,49 @@ func TestServerConnState(t *testing.T) { panic("intentional panic") }, } + + // A stateLog is a log of states over the lifetime of a connection. + type stateLog struct { + active net.Conn // The connection for which the log is recorded; set to the first connection seen in StateNew. + got []ConnState + want []ConnState + complete chan<- struct{} // If non-nil, closed when either 'got' is equal to 'want', or 'got' is no longer a prefix of 'want'. + } + activeLog := make(chan *stateLog, 1) + + // wantLog invokes doRequests, then waits for the resulting connection to + // either pass through the sequence of states in want or enter a state outside + // of that sequence. + wantLog := func(doRequests func(), want ...ConnState) { + t.Helper() + complete := make(chan struct{}) + activeLog <- &stateLog{want: want, complete: complete} + + doRequests() + + timer := time.NewTimer(5 * time.Second) + select { + case <-timer.C: + t.Errorf("Timed out waiting for connection to change state.") + case <-complete: + timer.Stop() + } + sl := <-activeLog + if !reflect.DeepEqual(sl.got, sl.want) { + t.Errorf("Request(s) produced unexpected state sequence.\nGot: %v\nWant: %v", sl.got, sl.want) + } + // Don't return sl to activeLog: we don't expect any further states after + // this point, and want to keep the ConnState callback blocked until the + // next call to wantLog. + } + ts := httptest.NewUnstartedServer(HandlerFunc(func(w ResponseWriter, r *Request) { handler[r.URL.Path](w, r) })) - defer ts.Close() - - var mu sync.Mutex // guard stateLog and connID - var stateLog = map[int][]ConnState{} - var connID = map[net.Conn]int{} + defer func() { + activeLog <- &stateLog{} // If the test failed, allow any remaining ConnState callbacks to complete. + ts.Close() + }() ts.Config.ErrorLog = log.New(ioutil.Discard, "", 0) ts.Config.ConnState = func(c net.Conn, state ConnState) { @@ -4131,20 +4165,27 @@ func TestServerConnState(t *testing.T) { t.Errorf("nil conn seen in state %s", state) return } - mu.Lock() - defer mu.Unlock() - id, ok := connID[c] - if !ok { - id = len(connID) + 1 - connID[c] = id + sl := <-activeLog + if sl.active == nil && state == StateNew { + sl.active = c + } else if sl.active != c { + t.Errorf("unexpected conn in state %s", state) + activeLog <- sl + return } - stateLog[id] = append(stateLog[id], state) + sl.got = append(sl.got, state) + if sl.complete != nil && (len(sl.got) >= len(sl.want) || !reflect.DeepEqual(sl.got, sl.want[:len(sl.got)])) { + close(sl.complete) + sl.complete = nil + } + activeLog <- sl } - ts.Start() + ts.Start() c := ts.Client() mustGet := func(url string, headers ...string) { + t.Helper() req, err := NewRequest("GET", url, nil) if err != nil { t.Fatal(err) @@ -4165,26 +4206,33 @@ func TestServerConnState(t *testing.T) { } } - mustGet(ts.URL + "/") - mustGet(ts.URL + "/close") + wantLog(func() { + mustGet(ts.URL + "/") + mustGet(ts.URL + "/close") + }, StateNew, StateActive, StateIdle, StateActive, StateClosed) - mustGet(ts.URL + "/") - mustGet(ts.URL+"/", "Connection", "close") + wantLog(func() { + mustGet(ts.URL + "/") + mustGet(ts.URL+"/", "Connection", "close") + }, StateNew, StateActive, StateIdle, StateActive, StateClosed) - mustGet(ts.URL + "/hijack") - mustGet(ts.URL + "/hijack-panic") + wantLog(func() { + mustGet(ts.URL + "/hijack") + }, StateNew, StateActive, StateHijacked) - // New->Closed - { + wantLog(func() { + mustGet(ts.URL + "/hijack-panic") + }, StateNew, StateActive, StateHijacked) + + wantLog(func() { c, err := net.Dial("tcp", ts.Listener.Addr().String()) if err != nil { t.Fatal(err) } c.Close() - } + }, StateNew, StateClosed) - // New->Active->Closed - { + wantLog(func() { c, err := net.Dial("tcp", ts.Listener.Addr().String()) if err != nil { t.Fatal(err) @@ -4194,10 +4242,9 @@ func TestServerConnState(t *testing.T) { } c.Read(make([]byte, 1)) // block until server hangs up on us c.Close() - } + }, StateNew, StateActive, StateClosed) - // New->Idle->Closed - { + wantLog(func() { c, err := net.Dial("tcp", ts.Listener.Addr().String()) if err != nil { t.Fatal(err) @@ -4213,47 +4260,7 @@ func TestServerConnState(t *testing.T) { t.Fatal(err) } c.Close() - } - - want := map[int][]ConnState{ - 1: {StateNew, StateActive, StateIdle, StateActive, StateClosed}, - 2: {StateNew, StateActive, StateIdle, StateActive, StateClosed}, - 3: {StateNew, StateActive, StateHijacked}, - 4: {StateNew, StateActive, StateHijacked}, - 5: {StateNew, StateClosed}, - 6: {StateNew, StateActive, StateClosed}, - 7: {StateNew, StateActive, StateIdle, StateClosed}, - } - logString := func(m map[int][]ConnState) string { - var b bytes.Buffer - var keys []int - for id := range m { - keys = append(keys, id) - } - sort.Ints(keys) - for _, id := range keys { - fmt.Fprintf(&b, "Conn %d: ", id) - for _, s := range m[id] { - fmt.Fprintf(&b, "%s ", s) - } - b.WriteString("\n") - } - return b.String() - } - - for i := 0; i < 5; i++ { - time.Sleep(time.Duration(i) * 50 * time.Millisecond) - mu.Lock() - match := reflect.DeepEqual(stateLog, want) - mu.Unlock() - if match { - return - } - } - - mu.Lock() - t.Errorf("Unexpected events.\nGot log:\n%s\n Want:\n%s\n", logString(stateLog), logString(want)) - mu.Unlock() + }, StateNew, StateActive, StateIdle, StateClosed) } func TestServerKeepAlivesEnabled(t *testing.T) {