From 93bb08568aa28d150a10c88eedc29560c727feaf Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Tue, 14 Jan 2025 23:23:43 +0800 Subject: [PATCH 1/7] ethclient/simulated: add test to check if simulated backend leaks goroutines --- ethclient/simulated/backend_test.go | 84 +++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/ethclient/simulated/backend_test.go b/ethclient/simulated/backend_test.go index 8efe93e24357..8a23ebb82907 100644 --- a/ethclient/simulated/backend_test.go +++ b/ethclient/simulated/backend_test.go @@ -20,8 +20,12 @@ import ( "context" "crypto/ecdsa" "crypto/sha256" + "fmt" "math/big" "math/rand" + "regexp" + "runtime" + "strings" "testing" "time" @@ -350,3 +354,83 @@ func TestAdjustTimeAfterFork(t *testing.T) { t.Errorf("failed to build block on fork") } } + +func createAndCloseSimBackend() { + genesisData := types.GenesisAlloc{} + simulatedBackend := NewBackend(genesisData) + defer simulatedBackend.Close() +} + +// sanitizeStackTrace removes any content inside parentheses (including the parentheses) from the input. it also +// removes occurances of the specific go-routine id. +func sanitizeStackTrace(input string) string { + re := regexp.MustCompile(`\(.*\)`) + sanitized := re.ReplaceAllString(input, "") + re2 := regexp.MustCompile(`goroutine [0-9*]*`) + sanitized = re2.ReplaceAllString(sanitized, "goroutine xxx") + return sanitized +} + +// collectGoroutineStacks collects the stack traces of all currently-running go-routines, stripping information specific +// to the current invocation (the parameter values in each function call, the specific go-routine id), and returning +// the stack traces as a map of stack trace text to the number of occurances of that stack-trace. +func collectGoroutineStacks() map[string]int { + buf := make([]byte, 1<<20) + stackLen := runtime.Stack(buf, true) + + // split all stack-traces by go-routine + stacks := strings.Split(string(buf[:stackLen]), "\n\n") + + res := make(map[string]int) + + for i := range stacks { + lines := strings.Split(stacks[i], "\n") + combinedLines := strings.Join(lines[1:], "\n") + + // filter out the callstack for this goroutine + if strings.Contains(combinedLines, "collectGoroutineStacks") { + continue + } + + // remove func call offset and variable values. + combinedLines = sanitizeStackTrace(combinedLines) + + res[combinedLines] = res[combinedLines] + 1 + } + + return res +} + +// leaks takes two sets of stack traces (returned as output from collectGoroutineStacks). If any stack traces exist +// after the second invocation which are not present in the first, they are considered "leaked" and returned in the +// output. +func leaks(firstGRs map[string]int, secondGRs map[string]int) []string { + var res []string + for key, _ := range secondGRs { + if _, ok := firstGRs[key]; !ok { + res = append(res, key) + } else if secondGRs[key] > firstGRs[key] { + res = append(res, key) + } + } + return res +} + +// TestCheckSimBackendGoroutineLeak checks whether creation of a simulated backend leaks go-routines. Any long-lived go-routines +// spawned by global variables are not considered leaked. +func TestCheckSimBackendGoroutineLeak(t *testing.T) { + createAndCloseSimBackend() + stacks1 := collectGoroutineStacks() + createAndCloseSimBackend() + stacks2 := collectGoroutineStacks() + + l := leaks(stacks1, stacks2) + if len(l) > 0 { + var leakedGRs string + for _, leak := range l { + leakedGRs = leakedGRs + fmt.Sprintf("%s\n\n", leak) + } + + t.Fatalf("leaked goroutines:\n%s", leakedGRs) + } +} From 21aefbf4ee7aa6b6823f0eb8342e179fdc09537f Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Mon, 20 Jan 2025 23:09:17 +0800 Subject: [PATCH 2/7] ignore false-alarm "leak" from leveldb --- ethclient/simulated/backend_test.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/ethclient/simulated/backend_test.go b/ethclient/simulated/backend_test.go index 8a23ebb82907..8f7370a0a6fb 100644 --- a/ethclient/simulated/backend_test.go +++ b/ethclient/simulated/backend_test.go @@ -426,11 +426,21 @@ func TestCheckSimBackendGoroutineLeak(t *testing.T) { l := leaks(stacks1, stacks2) if len(l) > 0 { + // ignore this "leak" from leveldb: After closing the db, LevelDB takes about a second to close a go-routine + // that it instantiates. + re, err := regexp.Compile("github\\.com/syndtr/goleveldb/leveldb\\.\n.*/leveldb/db_state\\.go:110 \\+0xe4\ncreated by github\\.com/syndtr/goleveldb/leveldb\\.openDB in goroutine xxx\n.*/leveldb/db\\.go:149 \\+0x3d0") + if err != nil { + panic(err) + } var leakedGRs string for _, leak := range l { + if re.MatchString(leak) { + continue + } leakedGRs = leakedGRs + fmt.Sprintf("%s\n\n", leak) } - - t.Fatalf("leaked goroutines:\n%s", leakedGRs) + if leakedGRs != "" { + t.Fatalf("leaked goroutines:\n%s", leakedGRs) + } } } From c7bdccae107bd1a2e6643c0a2b4185b05b46f5b1 Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Tue, 21 Jan 2025 16:33:50 +0800 Subject: [PATCH 3/7] strip trailing newline on go-routines dump --- ethclient/simulated/backend_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethclient/simulated/backend_test.go b/ethclient/simulated/backend_test.go index 8f7370a0a6fb..b328c59d77c7 100644 --- a/ethclient/simulated/backend_test.go +++ b/ethclient/simulated/backend_test.go @@ -380,7 +380,7 @@ func collectGoroutineStacks() map[string]int { // split all stack-traces by go-routine stacks := strings.Split(string(buf[:stackLen]), "\n\n") - + stacks[len(stacks)-1] = strings.TrimRight(stacks[len(stacks)-1], "\n") res := make(map[string]int) for i := range stacks { From 38d825d2801e429a6a986b8edc24e07d2678585c Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Tue, 21 Jan 2025 20:43:38 +0800 Subject: [PATCH 4/7] relax leveldb stack-trace regexp to match binaries compiled on all systems --- ethclient/simulated/backend_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethclient/simulated/backend_test.go b/ethclient/simulated/backend_test.go index b328c59d77c7..d6d5a46823e7 100644 --- a/ethclient/simulated/backend_test.go +++ b/ethclient/simulated/backend_test.go @@ -428,7 +428,7 @@ func TestCheckSimBackendGoroutineLeak(t *testing.T) { if len(l) > 0 { // ignore this "leak" from leveldb: After closing the db, LevelDB takes about a second to close a go-routine // that it instantiates. - re, err := regexp.Compile("github\\.com/syndtr/goleveldb/leveldb\\.\n.*/leveldb/db_state\\.go:110 \\+0xe4\ncreated by github\\.com/syndtr/goleveldb/leveldb\\.openDB in goroutine xxx\n.*/leveldb/db\\.go:149 \\+0x3d0") + re, err := regexp.Compile("github\\.com/syndtr/goleveldb/leveldb\\.\n.*/leveldb/db_state\\.go:110 \\+.*\ncreated by github\\.com/syndtr/goleveldb/leveldb\\.openDB in goroutine xxx\n.*/leveldb/db\\.go:149 \\+.*") if err != nil { panic(err) } From 4c429b45c36aad5fdf140c89356e577e9f205432 Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Wed, 22 Jan 2025 02:20:55 +0800 Subject: [PATCH 5/7] use goleak package --- ethclient/simulated/backend_test.go | 84 ++--------------------------- go.mod | 1 + go.sum | 2 + 3 files changed, 6 insertions(+), 81 deletions(-) diff --git a/ethclient/simulated/backend_test.go b/ethclient/simulated/backend_test.go index d6d5a46823e7..23b55461d2b6 100644 --- a/ethclient/simulated/backend_test.go +++ b/ethclient/simulated/backend_test.go @@ -20,12 +20,9 @@ import ( "context" "crypto/ecdsa" "crypto/sha256" - "fmt" + "go.uber.org/goleak" "math/big" "math/rand" - "regexp" - "runtime" - "strings" "testing" "time" @@ -361,86 +358,11 @@ func createAndCloseSimBackend() { defer simulatedBackend.Close() } -// sanitizeStackTrace removes any content inside parentheses (including the parentheses) from the input. it also -// removes occurances of the specific go-routine id. -func sanitizeStackTrace(input string) string { - re := regexp.MustCompile(`\(.*\)`) - sanitized := re.ReplaceAllString(input, "") - re2 := regexp.MustCompile(`goroutine [0-9*]*`) - sanitized = re2.ReplaceAllString(sanitized, "goroutine xxx") - return sanitized -} - -// collectGoroutineStacks collects the stack traces of all currently-running go-routines, stripping information specific -// to the current invocation (the parameter values in each function call, the specific go-routine id), and returning -// the stack traces as a map of stack trace text to the number of occurances of that stack-trace. -func collectGoroutineStacks() map[string]int { - buf := make([]byte, 1<<20) - stackLen := runtime.Stack(buf, true) - - // split all stack-traces by go-routine - stacks := strings.Split(string(buf[:stackLen]), "\n\n") - stacks[len(stacks)-1] = strings.TrimRight(stacks[len(stacks)-1], "\n") - res := make(map[string]int) - - for i := range stacks { - lines := strings.Split(stacks[i], "\n") - combinedLines := strings.Join(lines[1:], "\n") - - // filter out the callstack for this goroutine - if strings.Contains(combinedLines, "collectGoroutineStacks") { - continue - } - - // remove func call offset and variable values. - combinedLines = sanitizeStackTrace(combinedLines) - - res[combinedLines] = res[combinedLines] + 1 - } - - return res -} - -// leaks takes two sets of stack traces (returned as output from collectGoroutineStacks). If any stack traces exist -// after the second invocation which are not present in the first, they are considered "leaked" and returned in the -// output. -func leaks(firstGRs map[string]int, secondGRs map[string]int) []string { - var res []string - for key, _ := range secondGRs { - if _, ok := firstGRs[key]; !ok { - res = append(res, key) - } else if secondGRs[key] > firstGRs[key] { - res = append(res, key) - } - } - return res -} - // TestCheckSimBackendGoroutineLeak checks whether creation of a simulated backend leaks go-routines. Any long-lived go-routines // spawned by global variables are not considered leaked. func TestCheckSimBackendGoroutineLeak(t *testing.T) { createAndCloseSimBackend() - stacks1 := collectGoroutineStacks() + goleak.IgnoreCurrent() createAndCloseSimBackend() - stacks2 := collectGoroutineStacks() - - l := leaks(stacks1, stacks2) - if len(l) > 0 { - // ignore this "leak" from leveldb: After closing the db, LevelDB takes about a second to close a go-routine - // that it instantiates. - re, err := regexp.Compile("github\\.com/syndtr/goleveldb/leveldb\\.\n.*/leveldb/db_state\\.go:110 \\+.*\ncreated by github\\.com/syndtr/goleveldb/leveldb\\.openDB in goroutine xxx\n.*/leveldb/db\\.go:149 \\+.*") - if err != nil { - panic(err) - } - var leakedGRs string - for _, leak := range l { - if re.MatchString(leak) { - continue - } - leakedGRs = leakedGRs + fmt.Sprintf("%s\n\n", leak) - } - if leakedGRs != "" { - t.Fatalf("leaked goroutines:\n%s", leakedGRs) - } - } + goleak.VerifyNone(t) } diff --git a/go.mod b/go.mod index 35018fbe08b7..12c333fc4471 100644 --- a/go.mod +++ b/go.mod @@ -140,6 +140,7 @@ require ( github.com/tklauser/go-sysconf v0.3.12 // indirect github.com/tklauser/numcpus v0.6.1 // indirect github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 // indirect + go.uber.org/goleak v1.3.0 // indirect golang.org/x/mod v0.17.0 // indirect golang.org/x/net v0.25.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect diff --git a/go.sum b/go.sum index ded07f48675e..42ab425d6047 100644 --- a/go.sum +++ b/go.sum @@ -519,6 +519,8 @@ go.opencensus.io v0.22.3/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw= go.opencensus.io v0.22.4/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw= go.uber.org/automaxprocs v1.5.2 h1:2LxUOGiR3O6tw8ui5sZa2LAaHnsviZdVOUZw4fvbnME= go.uber.org/automaxprocs v1.5.2/go.mod h1:eRbA25aqJrxAbsLO0xy5jVwPt7FQnRgjW+efnwa1WM0= +go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= +go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= From a082038322a57cfa2f6c2c57ef58c16d9fd6c720 Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Wed, 22 Jan 2025 02:25:30 +0800 Subject: [PATCH 6/7] fix --- ethclient/simulated/backend_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ethclient/simulated/backend_test.go b/ethclient/simulated/backend_test.go index 23b55461d2b6..c3b65c2c6876 100644 --- a/ethclient/simulated/backend_test.go +++ b/ethclient/simulated/backend_test.go @@ -362,7 +362,9 @@ func createAndCloseSimBackend() { // spawned by global variables are not considered leaked. func TestCheckSimBackendGoroutineLeak(t *testing.T) { createAndCloseSimBackend() - goleak.IgnoreCurrent() + ignoreCur := goleak.IgnoreCurrent() + // ignore this leveldb function: this go-routine is guaranteed to be terminated 1 second after closing db handle + ignoreLdb := goleak.IgnoreAnyFunction("github.com/syndtr/goleveldb/leveldb.(*DB).mpoolDrain") createAndCloseSimBackend() - goleak.VerifyNone(t) + goleak.VerifyNone(t, ignoreCur, ignoreLdb) } From 5de3b7ca95780221f31e25cdb60c475a69c6a14c Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Thu, 23 Jan 2025 17:57:27 -0500 Subject: [PATCH 7/7] goimports --- ethclient/simulated/backend_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ethclient/simulated/backend_test.go b/ethclient/simulated/backend_test.go index c3b65c2c6876..378df3f3681c 100644 --- a/ethclient/simulated/backend_test.go +++ b/ethclient/simulated/backend_test.go @@ -20,12 +20,13 @@ import ( "context" "crypto/ecdsa" "crypto/sha256" - "go.uber.org/goleak" "math/big" "math/rand" "testing" "time" + "go.uber.org/goleak" + "github.com/ethereum/go-ethereum/crypto/kzg4844" "github.com/holiman/uint256"