Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow checking for functions anywhere in the stack #80

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 21 additions & 12 deletions internal/stack/stacks.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@

// Stack represents a single Goroutine's stack.
type Stack struct {
id int
state string
firstFunction string
fullStack *bytes.Buffer
id int
state string
functions []string
fullStack *bytes.Buffer
}

// ID returns the goroutine ID.
Expand All @@ -57,13 +57,21 @@

// FirstFunction returns the name of the first function on the stack.
func (s Stack) FirstFunction() string {
return s.firstFunction
if len(s.functions) > 0 {
return s.functions[0]
}
return ""

Check warning on line 63 in internal/stack/stacks.go

View check run for this annotation

Codecov / codecov/patch

internal/stack/stacks.go#L63

Added line #L63 was not covered by tests
}

// AllFunctions returns the names of all functions on the stack.
func (s Stack) AllFunctions() []string {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you update the tests in stacks_test.go to verify the behaviour of AllFunctions as well?

return s.functions
}

func (s Stack) String() string {
return fmt.Sprintf(
"Goroutine %v in state %v, with %v on top of the stack:\n%s",
s.id, s.state, s.firstFunction, s.Full())
s.id, s.state, s.FirstFunction(), s.Full())
}

func getStacks(all bool) []Stack {
Expand All @@ -82,7 +90,6 @@
}

// If we see the goroutine header, start a new stack.
isFirstLine := false
if strings.HasPrefix(line, "goroutine ") {
// flush any previous stack
if curStack != nil {
Expand All @@ -94,11 +101,10 @@
state: goState,
fullStack: &bytes.Buffer{},
}
isFirstLine = true
}
curStack.fullStack.WriteString(line)
if !isFirstLine && curStack.firstFunction == "" {
curStack.firstFunction = parseFirstFunc(line)
if f := parseFunc(line); f != "" {
curStack.functions = append(curStack.functions, f)
}
Comment on lines +106 to 108
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like it's trying to parse every line, should we instead parse only the lines known to be functions?

I think the format is:

<function>
<tab><file:line>
<function>
<tab><file:line>
...

}

Expand Down Expand Up @@ -127,12 +133,15 @@
}
}

func parseFirstFunc(line string) string {
func parseFunc(line string) string {
line = strings.TrimSpace(line)
if idx := strings.LastIndex(line, "("); idx > 0 {
return line[:idx]
}
panic(fmt.Sprintf("function calls missing parents: %q", line))
if idx := strings.LastIndex(line, "created by"); idx >= 0 {
return strings.TrimPrefix(line, "created by ")
}
Comment on lines +141 to +143
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it's not part of the stack, may be better to skip parsing these till it's required

return ""
}

// parseGoStackHeader parses a stack header that looks like:
Expand Down
14 changes: 14 additions & 0 deletions options.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,20 @@ func Cleanup(cleanupFunc func(exitCode int)) Option {
})
}

// IgnoreAnyFunction ignores any goroutines where the specified function
// is anywhere in the stack. The function name should be fully qualified,
// e.g., go.uber.org/goleak.IgnoreTopFunction
func IgnoreAnyFunction(f string) Option {
return addFilter(func(s stack.Stack) bool {
for _, f2 := range s.AllFunctions() {
if f == f2 {
return true
}
}
return false
})
}

// IgnoreCurrent records all current goroutines when the option is created, and ignores
// them in any future Find/Verify calls.
func IgnoreCurrent() Option {
Expand Down
4 changes: 4 additions & 0 deletions options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ func TestOptionsFilters(t *testing.T) {
// If we add an extra filter to ignore blockTill, it shouldn't match.
opts = buildOpts(IgnoreTopFunction("go.uber.org/goleak.(*blockedG).run"))
require.Zero(t, countUnfiltered(), "blockedG should be filtered out. running: %v", stack.All())

// If we add an extra filter to ignore startBlockedG (second item in the stack), it shouldn't match.
opts = buildOpts(IgnoreAnyFunction("go.uber.org/goleak.startBlockedG"))
require.Zero(t, countUnfiltered(), "startBlockedG should be filtered out. running: %v", stack.All())
}

func TestOptionsRetry(t *testing.T) {
Expand Down