diff --git a/.travis.yml b/.travis.yml index bc45a51..5a791be 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,5 @@ language: go go: - - 1.7 - 1.8 - 1.9 - "1.10" diff --git a/leaktest.go b/leaktest.go index 6b3f12a..219e930 100644 --- a/leaktest.go +++ b/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(") || diff --git a/leaktest_test.go b/leaktest_test.go index d6c70cd..6cecb0b 100644 --- a/leaktest_test.go +++ b/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, diff --git a/leaktest_utils_test.go b/leaktest_utils_test.go new file mode 100644 index 0000000..78dccbc --- /dev/null +++ b/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 +}