Skip to content

Commit 5b8e421

Browse files
rihter007rihter007
authored
Refactor goroutine leaks checker (#134)
1 parent 8e5666e commit 5b8e421

File tree

2 files changed

+53
-115
lines changed

2 files changed

+53
-115
lines changed

‎tests/common/goroutine_leak_check/goroutine_leak_check.go‎

Lines changed: 52 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -9,155 +9,94 @@ import (
99
"fmt"
1010
"os"
1111
"path"
12-
"path/filepath"
1312
"regexp"
1413
"runtime"
1514
"sort"
16-
"strconv"
1715
"strings"
1816
"testing"
1917
"time"
2018
)
2119

2220
var (
23-
goidRegex = regexp.MustCompile(`^\S+\s+(\d+)`)
2421
funcNameRegex = regexp.MustCompile(`^(\S+)\(\S+`)
25-
fileLineRegex = regexp.MustCompile(`^\s*(\S+):(\d+)`)
2622
)
2723

28-
type stackTraceEntry struct {
29-
GoID int
30-
File string
31-
Line int
32-
Func string
33-
Trace []string
34-
}
35-
36-
func (e *stackTraceEntry) String() string {
37-
return fmt.Sprintf("%d %s:%d %s\n %s",
38-
e.GoID, e.File, e.Line, e.Func, strings.Join(e.Trace, "\n "))
39-
}
40-
4124
// CheckLeakedGoRoutines is used to check for goroutine leaks at the end of a test.
4225
// It is not uncommon to leave a go routine that will never finish,
4326
// e.g. blocked on a channel that is unreachable will never be closed.
4427
// This function enumerates goroutines and reports any goroutines that are left running.
4528
func CheckLeakedGoRoutines(funcWhitelist ...string) error {
46-
_, err := checkLeakedGoRoutines(funcWhitelist...)
47-
return err
29+
var leaked []string
30+
for tryIdx := 0; tryIdx < 8; tryIdx++ {
31+
// We need to explicitly call GC to call full GC procedures for real.
32+
// Otherwise, for example there is high probability of not calling
33+
// Finalizers (which are used in xcontext, for example).
34+
runtime.GC()
35+
runtime.Gosched()
36+
37+
leaked = getLeakedGoroutines(funcWhitelist)
38+
if len(leaked) == 0 {
39+
return nil
40+
}
41+
time.Sleep(10 * time.Millisecond)
42+
}
43+
sort.Strings(leaked)
44+
return fmt.Errorf("leaked goroutines:\n %s\n", strings.Join(leaked, "\n "))
4845
}
4946

50-
func checkLeakedGoRoutines(funcWhitelist ...string) (string, error) {
51-
// Allow some time for dying routines to exit.
52-
time.Sleep(80 * time.Millisecond)
47+
func getLeakedGoroutines(funcWhitelist []string) []string {
5348
// Get goroutine stacks
5449
buf := make([]byte, 1000000)
5550
n := runtime.Stack(buf, true /* all */)
56-
// First goroutine is always the running one, we skip it.
57-
ph := 0
58-
var e *stackTraceEntry
5951
strBuf := string(buf[:n])
60-
var badEntries []string
6152

62-
addBadEntry := func() {
63-
if e == nil {
64-
return
65-
}
66-
found := false
67-
for _, wle := range funcWhitelist {
68-
if matched, _ := path.Match(wle, e.Func); matched {
69-
found = true
70-
break
71-
}
72-
}
73-
if !found {
74-
badEntries = append(badEntries, e.String())
75-
}
76-
}
53+
goroutinesStacks := strings.Split(strBuf, "\n\n")
7754

78-
for ln, line := range strings.Split(strBuf, "\n") {
79-
switch ph {
80-
case 0: // Look for an empty line
81-
if len(line) == 0 {
82-
addBadEntry()
83-
e = nil
84-
ph++
85-
} else {
86-
if e != nil {
87-
e.Trace = append(e.Trace, line)
88-
}
89-
}
90-
case 1: // Extract goroutine id
91-
if m := goidRegex.FindStringSubmatch(line); m != nil {
92-
if goid, err := strconv.Atoi(m[1]); err == nil {
93-
e = &stackTraceEntry{GoID: goid}
94-
ph++
95-
break
96-
}
97-
}
98-
panic(fmt.Sprintf("Cannot parse backtrace (goid) %d %q\n%s", ln+1, line, strBuf))
99-
case 2: // Extract function name.
100-
e.Trace = append(e.Trace, line)
101-
if m := funcNameRegex.FindStringSubmatch(line); m != nil {
102-
e.Func = m[1]
103-
ph++
104-
break
55+
var result []string
56+
// First goroutine is always the running one, we skip it.
57+
for _, goroutineStack := range goroutinesStacks[1:] {
58+
stack := strings.Split(goroutineStack, "\n")
59+
// first line is goroutine info, skip it
60+
var whitelisted bool
61+
var userGoroutine bool
62+
for idx, line := range stack[1:] {
63+
if idx%2 == 1 {
64+
// Lines go in the following order:
65+
// testing.tRunner(0xc000117040, 0x125ec08)
66+
// /usr/local/go/src/testing/testing.go:1259 +0x1db
67+
// created by testing.(*T).Run
68+
// /usr/local/go/src/testing/testing.go:1306 +0x673
69+
// ...
70+
// Should skip path
71+
continue
10572
}
106-
if e.Func != "" {
107-
// This means entire routine is in stdlib, ignore it.
108-
e = nil
109-
if line == "" {
110-
ph = 1
111-
} else {
112-
ph = 0
113-
}
114-
break
73+
m := funcNameRegex.FindStringSubmatch(line)
74+
if m == nil {
75+
continue
11576
}
116-
panic(fmt.Sprintf("Cannot parse backtrace (func) %d %q", ln, line))
117-
case 3: // Extract file name
118-
e.Trace = append(e.Trace, line)
119-
if m := fileLineRegex.FindStringSubmatch(line); m != nil {
120-
e.File = filepath.Base(m[1])
121-
if ln, err := strconv.Atoi(m[2]); err == nil {
122-
e.Line = ln
123-
}
124-
// We are looking for a non-stdlib function.
125-
if !strings.Contains(e.Func, "/") || !strings.Contains(path.Dir(e.Func), ".") {
126-
ph = 2
127-
} else {
128-
ph = 0
77+
funcName := m[1]
78+
79+
// stdlib functions do not contain '/'
80+
if strings.Contains(funcName, "/") {
81+
userGoroutine = true
82+
for _, wle := range funcWhitelist {
83+
if matched, _ := path.Match(wle, funcName); matched {
84+
whitelisted = true
85+
break
86+
}
12987
}
130-
break
13188
}
132-
panic(fmt.Sprintf("Cannot parse backtrace (file) %d %q", ln, line))
89+
}
90+
if userGoroutine && !whitelisted {
91+
result = append(result, goroutineStack)
13392
}
13493
}
135-
addBadEntry()
136-
137-
var err error
138-
if len(badEntries) > 0 {
139-
sort.Strings(badEntries)
140-
err = fmt.Errorf("leaked goroutines:\n %s\n", strings.Join(badEntries, "\n "))
141-
}
142-
return strBuf, err
94+
return result
14395
}
14496

14597
func LeakCheckingTestMain(m *testing.M, funcWhitelist ...string) {
14698
ret := m.Run()
14799
if ret == 0 {
148-
time.Sleep(20 * time.Millisecond) // Give stragglers some time to exit.
149-
150-
// We need to explicitly call GC to call full GC procedures for real.
151-
// Otherwise for example there is high probability of not calling
152-
// Finalizers (which are used in xcontext, for example).
153-
//
154-
// And we do it twice for better reliability (I already had
155-
// experience where calling `GC` one was not enough).
156-
runtime.GC()
157-
runtime.Gosched()
158-
runtime.GC()
159-
runtime.Gosched()
160-
161100
if err := CheckLeakedGoRoutines(funcWhitelist...); err != nil {
162101
fmt.Fprintf(os.Stderr, "%s", err)
163102
ret = 1

‎tests/common/goroutine_leak_check/goroutine_leak_check_test.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ func TestLeaks(t *testing.T) {
3030
go Func1(c)
3131
c.Wait() // Wait for the go routine to start.
3232
c.L.Unlock()
33-
dump, err := checkLeakedGoRoutines()
34-
require.Errorf(t, err, "===\n%s\n===", dump)
33+
err := CheckLeakedGoRoutines()
3534
require.Contains(t, err.Error(), "goroutine_leak_check_test.go")
3635
require.Contains(t, err.Error(), "goroutine_leak_check.Func1")
3736
c.Signal()

0 commit comments

Comments
 (0)