Merge pull request #29 from lawrencegripper/lg/ignorehttpkeepalive

Ignore HTTPKeepAlive in leaktest

.travis.yml

@@ -1,6 +1,5 @@
 language: go
 go:
- - 1.7
  - 1.8
  - 1.9
  - "1.10"

leaktest.go

@@ -41,6 +41,9 @@ func interestingGoroutine(g string) (*goroutine, error) {
 	}
 
 	if stack == "" ||
+		// Ignore HTTP keep alives
+		strings.Contains(stack, ").readLoop(") ||
+		strings.Contains(stack, ").writeLoop(") ||
 		// Below are the stacks ignored by the upstream leaktest code.
 		strings.Contains(stack, "testing.Main(") ||
 		strings.Contains(stack, "testing.(*T).Run(") ||

leaktest_test.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net/http"
+	"net/http/httptest"
 	"sync"
 	"testing"
 	"time"
@@ -19,60 +21,116 @@ func (tr *testReporter) Errorf(format string, args ...interface{}) {
 	tr.msg = fmt.Sprintf(format, args...)
 }
 
-var leakyFuncs = []func(){
-	// Infinite for loop
-	func() {
-		for {
-			time.Sleep(time.Second)
-		}
-	},
-	// Select on a channel not referenced by other goroutines.
-	func() {
-		c := make(chan struct{})
-		<-c
-	},
-	// Blocked select on channels not referenced by other goroutines.
-	func() {
-		c := make(chan struct{})
-		c2 := make(chan struct{})
-		select {
-		case <-c:
-		case c2 <- struct{}{}:
-		}
-	},
-	// Blocking wait on sync.Mutex that isn't referenced by other goroutines.
-	func() {
-		var mu sync.Mutex
-		mu.Lock()
-		mu.Lock()
-	},
-	// Blocking wait on sync.RWMutex that isn't referenced by other goroutines.
-	func() {
-		var mu sync.RWMutex
-		mu.RLock()
-		mu.Lock()
-	},
-	func() {
-		var mu sync.Mutex
-		mu.Lock()
-		c := sync.NewCond(&mu)
-		c.Wait()
-	},
-}
+// Client for the TestServer
+var testServer *httptest.Server
 
 func TestCheck(t *testing.T) {
+	leakyFuncs := []struct {
+		f          func()
+		name       string
+		expectLeak bool
+	}{
+		{
+			name:       "Infinite for loop",
+			expectLeak: true,
+			f: func() {
+				for {
+					time.Sleep(time.Second)
+				}
+			},
+		},
+		{
+			name:       "Select on a channel not referenced by other goroutines.",
+			expectLeak: true,
+			f: func() {
+				c := make(chan struct{})
+				<-c
+			},
+		},
+		{
+			name:       "Blocked select on channels not referenced by other goroutines.",
+			expectLeak: true,
+			f: func() {
+				c := make(chan struct{})
+				c2 := make(chan struct{})
+				select {
+				case <-c:
+				case c2 <- struct{}{}:
+				}
+			},
+		},
+		{
+			name:       "Blocking wait on sync.Mutex that isn't referenced by other goroutines.",
+			expectLeak: true,
+			f: func() {
+				var mu sync.Mutex
+				mu.Lock()
+				mu.Lock()
+			},
+		},
+		{
+			name:       "Blocking wait on sync.RWMutex that isn't referenced by other goroutines.",
+			expectLeak: true,
+			f: func() {
+				var mu sync.RWMutex
+				mu.RLock()
+				mu.Lock()
+			},
+		},
+		{
+			name:       "HTTP Client with KeepAlive Disabled.",
+			expectLeak: false,
+			f: func() {
+				tr := &http.Transport{
+					DisableKeepAlives: true,
+				}
+				client := &http.Client{Transport: tr}
+				_, err := client.Get(testServer.URL)
+				if err != nil {
+					t.Error(err)
+				}
+			},
+		},
+		{
+			name:       "HTTP Client with KeepAlive Enabled.",
+			expectLeak: true,
+			f: func() {
+				tr := &http.Transport{
+					DisableKeepAlives: false,
+				}
+				client := &http.Client{Transport: tr}
+				_, err := client.Get(testServer.URL)
+				if err != nil {
+					t.Error(err)
+				}
+			},
+		},
+	}
+
+	// Start our keep alive server for keep alive tests
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	testServer = startKeepAliveEnabledServer(ctx)
+
 	// this works because the running goroutine is left running at the
 	// start of the next test case - so the previous leaks don't affect the
 	// check for the next one
-	for i, fn := range leakyFuncs {
-		checker := &testReporter{}
-		snapshot := CheckTimeout(checker, time.Second)
-		go fn()
-
-		snapshot()
-		if !checker.failed {
-			t.Errorf("didn't catch sleeping goroutine, test #%d", i)
-		}
+	for _, leakyTestcase := range leakyFuncs {
+
+		t.Run(leakyTestcase.name, func(t *testing.T) {
+			checker := &testReporter{}
+			snapshot := CheckTimeout(checker, time.Second)
+			go leakyTestcase.f()
+
+			snapshot()
+
+			if !checker.failed && leakyTestcase.expectLeak {
+				t.Error("didn't catch sleeping goroutine")
+			}
+			if checker.failed && !leakyTestcase.expectLeak {
+				t.Error("got leak but didn't expect it")
+			}
+		})
 	}
 }
 
@@ -133,6 +191,10 @@ func TestInterestingGoroutine(t *testing.T) {
 			stack: "goroutine 123 [running]:",
 			err:   errors.New(`error parsing stack: "goroutine 123 [running]:"`),
 		},
+		{
+			stack: "goroutine 299 [IO wait]:\nnet/http.(*persistConn).readLoop(0xc420556240)",
+			err:   nil,
+		},
 		{
 			stack: "goroutine 123 [running]:\ntesting.RunTests",
 			err:   nil,

leaktest_utils_test.go

@@ -0,0 +1,30 @@
+package leaktest
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"time"
+)
+
+func index() http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	})
+}
+
+func startKeepAliveEnabledServer(ctx context.Context) *httptest.Server {
+	server := httptest.NewUnstartedServer(index())
+	server.Config.ReadTimeout = 5 * time.Second
+	server.Config.WriteTimeout = 10 * time.Second
+	server.Config.IdleTimeout = 15 * time.Second
+	server.Config.SetKeepAlivesEnabled(true)
+
+	server.Start()
+	go func() {
+		<-ctx.Done()
+		server.Close()
+	}()
+
+	return server
+}