auth: Fix "race" - auth unit tests leaking goroutines

- We were leaking goroutines in auth-test
  - The go-routines were depending / modifying global test environment
variables (simpleTokenTTLDefault) leading to races

Removed the leaked go-routines, and expanded 'auth' package to
be covered we leaked go-routines detection.

auth/main_test.go

@@ -0,0 +1,15 @@
+// Copyright 2013 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package auth
+
+import (
+	"testing"
+
+	"go.etcd.io/etcd/v3/pkg/testutil"
+)
+
+func TestMain(m *testing.M) {
+	testutil.MustTestMainWithLeakDetection(m)
+}

auth/simple_token.go

@@ -36,6 +36,7 @@ const (
 )
 
 // var for testing purposes
+// TODO: Remove this mutable global state - as it's race-prone.
 var (
 	simpleTokenTTLDefault    = 300 * time.Second
 	simpleTokenTTLResolution = 1 * time.Second

auth/simple_token_test.go

@@ -50,6 +50,7 @@ func TestSimpleTokenDisabled(t *testing.T) {
 func TestSimpleTokenAssign(t *testing.T) {
 	tp := newTokenProviderSimple(zap.NewExample(), dummyIndexWaiter, simpleTokenTTLDefault)
 	tp.enable()
+	defer tp.disable()
 	ctx := context.WithValue(context.WithValue(context.TODO(), AuthenticateParamIndex{}, uint64(1)), AuthenticateParamSimpleTokenPrefix{}, "dummy")
 	token, err := tp.assign(ctx, "user1", 0)
 	if err != nil {

auth/store_test.go

@@ -64,10 +64,10 @@ func TestNewAuthStoreRevision(t *testing.T) {
 
 	// no changes to commit
 	b2 := backend.NewDefaultBackend(tPath)
+	defer b2.Close()
 	as = NewAuthStore(zap.NewExample(), b2, nil, tp, bcrypt.MinCost)
+	defer as.Close()
 	new := as.Revision()
-	as.Close()
-	b2.Close()
 
 	if old != new {
 		t.Fatalf("expected revision %d, got %d", old, new)
@@ -77,6 +77,7 @@ func TestNewAuthStoreRevision(t *testing.T) {
 // TestNewAuthStoreBryptCost ensures that NewAuthStore uses default when given bcrypt-cost is invalid
 func TestNewAuthStoreBcryptCost(t *testing.T) {
 	b, tPath := backend.NewDefaultTmpBackend()
+	defer b.Close()
 	defer os.Remove(tPath)
 
 	tp, err := NewTokenProvider(zap.NewExample(), tokenTypeSimple, dummyIndexWaiter, simpleTokenTTLDefault)
@@ -87,13 +88,11 @@ func TestNewAuthStoreBcryptCost(t *testing.T) {
 	invalidCosts := [2]int{bcrypt.MinCost - 1, bcrypt.MaxCost + 1}
 	for _, invalidCost := range invalidCosts {
 		as := NewAuthStore(zap.NewExample(), b, nil, tp, invalidCost)
+		defer as.Close()
 		if as.BcryptCost() != bcrypt.DefaultCost {
 			t.Fatalf("expected DefaultCost when bcryptcost is invalid")
 		}
-		as.Close()
 	}
-
-	b.Close()
 }
 
 func encodePassword(s string) string {
@@ -175,6 +174,7 @@ func TestUserAdd(t *testing.T) {
 
 func TestRecover(t *testing.T) {
 	as, tearDown := setupAuthStore(t)
+	defer as.Close()
 	defer tearDown(t)
 
 	as.enabled = false
@@ -654,6 +654,7 @@ func TestIsAuthEnabled(t *testing.T) {
 // TestAuthRevisionRace ensures that access to authStore.revision is thread-safe.
 func TestAuthInfoFromCtxRace(t *testing.T) {
 	b, tPath := backend.NewDefaultTmpBackend()
+	defer b.Close()
 	defer os.Remove(tPath)
 
 	tp, err := NewTokenProvider(zap.NewExample(), tokenTypeSimple, dummyIndexWaiter, simpleTokenTTLDefault)
@@ -709,7 +710,8 @@ func TestIsAdminPermitted(t *testing.T) {
 }
 
 func TestRecoverFromSnapshot(t *testing.T) {
-	as, _ := setupAuthStore(t)
+	as, teardown := setupAuthStore(t)
+	defer teardown(t)
 
 	ua := &pb.AuthUserAddRequest{Name: "foo", Options: &authpb.UserAddOptions{NoPassword: false}}
 	_, err := as.UserAdd(ua) // add an existing user
@@ -733,9 +735,7 @@ func TestRecoverFromSnapshot(t *testing.T) {
 		t.Fatal(err)
 	}
 	as2 := NewAuthStore(zap.NewExample(), as.be, nil, tp, bcrypt.MinCost)
-	defer func(a *authStore) {
+	defer as2.Close()
-		a.Close()
-	}(as2)
 
 	if !as2.IsAuthEnabled() {
 		t.Fatal("recovering authStore from existing backend failed")
@@ -808,13 +808,16 @@ func TestHammerSimpleAuthenticate(t *testing.T) {
 // TestRolesOrder tests authpb.User.Roles is sorted
 func TestRolesOrder(t *testing.T) {
 	b, tPath := backend.NewDefaultTmpBackend()
+	defer b.Close()
 	defer os.Remove(tPath)
 
 	tp, err := NewTokenProvider(zap.NewExample(), tokenTypeSimple, dummyIndexWaiter, simpleTokenTTLDefault)
+	defer tp.disable()
 	if err != nil {
 		t.Fatal(err)
 	}
 	as := NewAuthStore(zap.NewExample(), b, nil, tp, bcrypt.MinCost)
+	defer as.Close()
 	err = enableAuthAndCreateRoot(as)
 	if err != nil {
 		t.Fatal(err)
@@ -863,6 +866,7 @@ func TestAuthInfoFromCtxWithRootJWT(t *testing.T) {
 // testAuthInfoFromCtxWithRoot ensures "WithRoot" properly embeds token in the context.
 func testAuthInfoFromCtxWithRoot(t *testing.T, opts string) {
 	b, tPath := backend.NewDefaultTmpBackend()
+	defer b.Close()
 	defer os.Remove(tPath)
 
 	tp, err := NewTokenProvider(zap.NewExample(), opts, dummyIndexWaiter, simpleTokenTTLDefault)

client/integration/main_test.go

@@ -5,16 +5,11 @@
 package integration
 
 import (
-	"os"
 	"testing"
 
 	"go.etcd.io/etcd/v3/pkg/testutil"
 )
 
 func TestMain(m *testing.M) {
-	v := m.Run()
+	testutil.MustTestMainWithLeakDetection(m)
-	if v == 0 && testutil.CheckLeakedGoroutine() {
-		os.Exit(1)
-	}
-	os.Exit(v)
 }

clientv3/integration/main_test.go

@@ -5,16 +5,11 @@
 package integration
 
 import (
-	"os"
 	"testing"
 
 	"go.etcd.io/etcd/v3/pkg/testutil"
 )
 
 func TestMain(m *testing.M) {
-	v := m.Run()
+	testutil.MustTestMainWithLeakDetection(m)
-	if v == 0 && testutil.CheckLeakedGoroutine() {
-		os.Exit(1)
-	}
-	os.Exit(v)
 }

integration/main_test.go

@@ -5,16 +5,11 @@
 package integration
 
 import (
-	"os"
 	"testing"
 
 	"go.etcd.io/etcd/v3/pkg/testutil"
 )
 
 func TestMain(m *testing.M) {
-	v := m.Run()
+	testutil.MustTestMainWithLeakDetection(m)
-	if v == 0 && testutil.CheckLeakedGoroutine() {
-		os.Exit(1)
-	}
-	os.Exit(v)
 }

pkg/testutil/leak.go

@@ -24,11 +24,7 @@ running(leaking) after all tests.
 	import "go.etcd.io/etcd/v3/pkg/testutil"
 
 	func TestMain(m *testing.M) {
-		v := m.Run()
+		testutil.MustTestMainWithLeakDetection(m)
-		if v == 0 && testutil.CheckLeakedGoroutine() {
-			os.Exit(1)
-		}
-		os.Exit(v)
 	}
 
 	func TestSample(t *testing.T) {
@@ -38,10 +34,6 @@ running(leaking) after all tests.
 
 */
 func CheckLeakedGoroutine() bool {
-	if testing.Short() {
-		// not counting goroutines for leakage in -short mode
-		return false
-	}
 	gs := interestingGoroutines()
 	if len(gs) == 0 {
 		return false
@@ -66,9 +58,6 @@ func CheckLeakedGoroutine() bool {
 // Waits for go-routines shutdown for 'd'.
 func CheckAfterTest(d time.Duration) error {
 	http.DefaultTransport.(*http.Transport).CloseIdleConnections()
-	if testing.Short() {
-		return nil
-	}
 	var bad string
 	badSubstring := map[string]string{
 		").writeLoop(": "a Transport",
@@ -140,3 +129,19 @@ func interestingGoroutines() (gs []string) {
 	sort.Strings(gs)
 	return gs
 }
+
+// MustTestMainWithLeakDetection expands standard m.Run with leaked
+// goroutines detection.
+func MustTestMainWithLeakDetection(m *testing.M) {
+	v := m.Run()
+
+	http.DefaultTransport.(*http.Transport).CloseIdleConnections()
+
+	// Let the other goroutines finalize.
+	runtime.Gosched()
+
+	if v == 0 && CheckLeakedGoroutine() {
+		os.Exit(1)
+	}
+	os.Exit(v)
+}