test: fix testleak in TestIntegration (pingcap#17667)

Signed-off-by: wjhuang2016 <[email protected]>

domain/db_test.go

@@ -20,20 +20,25 @@ import (
 	. "github.com/pingcap/check"
 	"github.com/pingcap/tidb/session"
 	"github.com/pingcap/tidb/store/mockstore"
+	"github.com/pingcap/tidb/util/testleak"
 )
 
 type dbTestSuite struct{}
 
 var _ = Suite(&dbTestSuite{})
 
 func (ts *dbTestSuite) TestIntegration(c *C) {
+	testleak.BeforeTest()
+	defer testleak.AfterTest(c)()
 	var err error
 	lease := 50 * time.Millisecond
 	store, err := mockstore.NewMockStore()
 	c.Assert(err, IsNil)
+	defer store.Close()
 	session.SetSchemaLease(lease)
-	_, err = session.BootstrapSession(store)
+	domain, err := session.BootstrapSession(store)
 	c.Assert(err, IsNil)
+	defer domain.Close()
 
 	// for NotifyUpdatePrivilege
 	createRoleSQL := `CREATE ROLE 'test'@'localhost';`

domain/domain_test.go

@@ -18,9 +18,6 @@ import (
 	"crypto/tls"
 	"math"
 	"net"
-	"runtime"
-	"sort"
-	"strings"
 	"testing"
 	"time"
 
@@ -40,12 +37,10 @@ import (
 	"github.com/pingcap/tidb/store/mockstore"
 	"github.com/pingcap/tidb/store/tikv/oracle"
 	"github.com/pingcap/tidb/util"
-	"github.com/pingcap/tidb/util/logutil"
 	"github.com/pingcap/tidb/util/mock"
 	"github.com/pingcap/tidb/util/testleak"
 	dto "github.com/prometheus/client_model/go"
 	"go.etcd.io/etcd/integration"
-	"go.uber.org/zap"
 )
 
 func TestT(t *testing.T) {
@@ -91,137 +86,12 @@ func unixSocketAvailable() bool {
 	return false
 }
 
-// For debug only, will be removed later.
-// TODO: remove these tests for investigating the leak problem.
-func interestingGoroutines() (gs []string) {
-	buf := make([]byte, 2<<20)
-	buf = buf[:runtime.Stack(buf, true)]
-	for _, g := range strings.Split(string(buf), "\n\n") {
-		sl := strings.SplitN(g, "\n", 2)
-		if len(sl) != 2 {
-			continue
-		}
-		stack := strings.TrimSpace(sl[1])
-		if stack == "" ||
-			strings.Contains(stack, "created by github.com/pingcap/tidb.init") ||
-			strings.Contains(stack, "testing.RunTests") ||
-			strings.Contains(stack, "check.(*resultTracker).start") ||
-			strings.Contains(stack, "check.(*suiteRunner).runFunc") ||
-			strings.Contains(stack, "check.(*suiteRunner).parallelRun") ||
-			strings.Contains(stack, "localstore.(*dbStore).scheduler") ||
-			strings.Contains(stack, "tikv.(*noGCHandler).Start") ||
-			strings.Contains(stack, "ddl.(*ddl).start") ||
-			strings.Contains(stack, "ddl.(*delRange).startEmulator") ||
-			strings.Contains(stack, "domain.NewDomain") ||
-			strings.Contains(stack, "testing.(*T).Run") ||
-			strings.Contains(stack, "domain.(*Domain).LoadPrivilegeLoop") ||
-			strings.Contains(stack, "domain.(*Domain).UpdateTableStatsLoop") ||
-			strings.Contains(stack, "testing.Main(") ||
-			strings.Contains(stack, "runtime.goexit") ||
-			strings.Contains(stack, "created by runtime.gc") ||
-			strings.Contains(stack, "interestingGoroutines") ||
-			strings.Contains(stack, "runtime.MHeap_Scavenger") ||
-			// these go routines are async terminated, so they may still alive after test end, thus cause
-			// false positive leak failures
-			strings.Contains(stack, "google.golang.org/grpc.(*addrConn).resetTransport") ||
-			strings.Contains(stack, "google.golang.org/grpc.(*ccBalancerWrapper).watcher") ||
-			strings.Contains(stack, "github.com/pingcap/goleveldb/leveldb/util.(*BufferPool).drain") ||
-			strings.Contains(stack, "github.com/pingcap/goleveldb/leveldb.(*DB).compactionError") ||
-			strings.Contains(stack, "github.com/pingcap/goleveldb/leveldb.(*DB).mpoolDrain") {
-			continue
-		}
-		gs = append(gs, stack)
-	}
-	sort.Strings(gs)
-	return
-}
-
-var beforeTestGoroutines = map[string]bool{}
-var testGoroutinesInited bool
-
-// BeforeTest gets the current goroutines.
-// It's used for check.Suite.SetUpSuite() function.
-// Now it's only used in the tidb_test.go.
-func BeforeTest() {
-	for _, g := range interestingGoroutines() {
-		beforeTestGoroutines[g] = true
-	}
-	testGoroutinesInited = true
-}
-
-const defaultCheckCnt = 50
-
-func checkLeakAfterTest(errorFunc func(cnt int, g string)) func() {
-	// After `BeforeTest`, `beforeTestGoroutines` may still be empty, in this case,
-	// we shouldn't init it again.
-	if !testGoroutinesInited && len(beforeTestGoroutines) == 0 {
-		for _, g := range interestingGoroutines() {
-			beforeTestGoroutines[g] = true
-		}
-	}
-
-	for str := range beforeTestGoroutines {
-		logutil.BgLogger().Info("checkLeakAfterTest", zap.String("L163", str))
-	}
-
-	cnt := defaultCheckCnt
-	return func() {
-		defer func() {
-			beforeTestGoroutines = map[string]bool{}
-			testGoroutinesInited = false
-		}()
-
-		var leaked []string
-		for i := 0; i < cnt; i++ {
-			leaked = leaked[:0]
-			for _, g := range interestingGoroutines() {
-				if !beforeTestGoroutines[g] {
-					leaked = append(leaked, g)
-					for str := range beforeTestGoroutines {
-						logutil.BgLogger().Info("checkLeakAfterTest", zap.String("L180", str), zap.Int("i", i))
-					}
-					for _, str := range interestingGoroutines() {
-						logutil.BgLogger().Info("checkLeakAfterTest", zap.String("L183", str), zap.Int("i", i))
-					}
-				}
-			}
-			// Bad stuff found, but goroutines might just still be
-			// shutting down, so give it some time.
-			if len(leaked) != 0 {
-				time.Sleep(50 * time.Millisecond)
-				continue
-			}
-
-			return
-		}
-		for _, g := range leaked {
-			errorFunc(cnt, g)
-		}
-	}
-}
-
-// AfterTestT is used after all the test cases is finished.
-func AfterTestT(t *testing.T) func() {
-	errorFunc := func(cnt int, g string) {
-		t.Errorf("Test %s check-count %d appears to have leaked: %v", t.Name(), cnt, g)
-	}
-	return checkLeakAfterTest(errorFunc)
-}
-
 func TestInfo(t *testing.T) {
 	if !unixSocketAvailable() {
 		return
 	}
-	for i, str := range interestingGoroutines() {
-		t.Logf("TestInfo: BeforeTest %d %s", i, str)
-	}
-	BeforeTest()
-	defer func() {
-		AfterTestT(t)()
-		for i, str := range interestingGoroutines() {
-			t.Logf("TestInfo: AfterTest %d %s", i, str)
-		}
-	}()
+	testleak.BeforeTest()
+	defer testleak.AfterTestT(t)()
 	ddlLease := 80 * time.Millisecond
 	s, err := mockstore.NewMockStore()
 	if err != nil {

expression/evaluator_test.go

@@ -38,7 +38,7 @@ var _ = Suite(&testVectorizeSuite2{})
 
 func TestT(t *testing.T) {
 	testleak.BeforeTest()
-	defer testleak.AfterTestT(t)
+	defer testleak.AfterTestT(t)()
 
 	CustomVerboseFlag = true
 	*CustomParallelSuiteFlag = true

util/testleak/leaktest.go

@@ -77,6 +77,18 @@ var testGoroutinesInited bool
 // BeforeTest gets the current goroutines.
 // It's used for check.Suite.SetUpSuite() function.
 // Now it's only used in the tidb_test.go.
+// Note: it's not accurate, consider the following function:
+// func loop() {
+//   for {
+//     select {
+//       case <-ticker.C:
+//         DoSomething()
+//     }
+//   }
+// }
+// If this loop step into DoSomething() during BeforeTest(), the stack for this goroutine will contain DoSomething().
+// Then if this loop jumps out of DoSomething during AfterTest(), the stack for this goroutine will not contain DoSomething().
+// Resulting in false-positive leak reports.
 func BeforeTest() {
 	for _, g := range interestingGoroutines() {
 		beforeTestGoroutines[g] = true