Skip to content

Commit

Permalink
stack: "created by" is not part of the stack
Browse files Browse the repository at this point in the history
The function tha created a goroutine
should not be considered part of its stack.

However, we can use that entry to mark the end of a stack trace.
  • Loading branch information
abhinav committed Oct 22, 2023
1 parent 32d0656 commit f46c0cd
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 28 deletions.
52 changes: 40 additions & 12 deletions internal/stack/stacks.go
Expand Up @@ -159,13 +159,21 @@ func (p *stackParser) parseStack(line string) (Stack, error) {
continue
}

funcName, err := parseFuncName(line)
funcName, creator, err := parseFuncName(line)
if err != nil {
return Stack{}, fmt.Errorf("parse function: %w", err)
}
funcs[funcName] = struct{}{}
if firstFunction == "" {
firstFunction = funcName
if !creator {
// A function is part of a goroutine's stack
// only if it's not a "created by" function.
//
// The creator function is part of a different stack.
// We don't care about it right now.
funcs[funcName] = struct{}{}
if firstFunction == "" {
firstFunction = funcName
}

}

// The function name followed by a line in the form:
Expand All @@ -180,12 +188,30 @@ func (p *stackParser) parseStack(line string) (Stack, error) {
if len(bs) > 0 && bs[0] == '\t' {
fullStack.Write(bs)
fullStack.WriteByte('\n')
continue
} else {
// Put it back and let the next iteration handle it
// if it doesn't start with a tab.
p.scan.Unscan()

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

View check run for this annotation

Codecov / codecov/patch

internal/stack/stacks.go#L192-L194

Added lines #L192 - L194 were not covered by tests
}
}

// Put it back and let the next iteration handle it
// if it doesn't start with a tab.
p.scan.Unscan()
if creator {
// The "created by" line is the last line of the stack.
// We can stop parsing now.
//
// Note that if tracebackancestors=N is set,
// there may be more a traceback of the creator function
// following the "created by" line,
// but it should not be considered part of this stack.
// e.g.,
//
// created by testing.(*T).Run in goroutine 1
// /usr/lib/go/src/testing/testing.go:1648 +0x3ad
// [originating from goroutine 1]:
// testing.(*T).Run(...)
// /usr/lib/go/src/testing/testing.go:1649 +0x3ad
//
break
}
}

Expand Down Expand Up @@ -224,8 +250,9 @@ func getStackBuffer(all bool) []byte {
// example.com/path/to/package.(*typeName).funcName(args...)
// created by example.com/path/to/package.funcName
// created by example.com/path/to/package.funcName in goroutine [...]
func parseFuncName(line string) (string, error) {
var name string
//
// Also reports whether the line was a "created by" line.
func parseFuncName(line string) (name string, creator bool, err error) {
if after, ok := strings.CutPrefix(line, "created by "); ok {
// The function name is the part after "created by "
// and before " in goroutine [...]".
Expand All @@ -234,16 +261,17 @@ func parseFuncName(line string) (string, error) {
after = after[:idx]
}
name = after
creator = true
} else if idx := strings.LastIndexByte(line, '('); idx >= 0 {
// The function name is the part before the last '('.
name = line[:idx]
}

if name == "" {
return "", fmt.Errorf("no function found: %q", line)
return "", false, fmt.Errorf("no function found: %q", line)
}

return name, nil
return name, creator, nil
}

// parseGoStackHeader parses a stack header that looks like:
Expand Down
38 changes: 22 additions & 16 deletions internal/stack/stacks_test.go
Expand Up @@ -102,9 +102,6 @@ func TestCurrent(t *testing.T) {
assert.Contains(t, got.String(), "in state")
assert.Contains(t, got.String(), "on top of the stack")

assert.True(t, got.HasFunction("testing.(*T).Run"),
"missing in stack: %v\n%s", "testing.(*T).Run", all)

assert.Contains(t, all, "stack/stacks_test.go",
"file name missing in stack:\n%s", all)

Expand All @@ -124,10 +121,15 @@ func TestCurrentCreatedBy(t *testing.T) {
}()
<-done

// This test function will be at the bottom of the stack
// as the function that created the goroutine.
// The test function created the goroutine
// so it won't be part of the stack.
assert.False(t, stack.HasFunction("go.uber.org/goleak/internal/stack.TestCurrentCreatedBy"),
"TestCurrentCreatedBy should not be in stack:\n%s", stack.Full())

// However, the nested function should be.
assert.True(t,
stack.HasFunction("go.uber.org/goleak/internal/stack.TestCurrentCreatedBy"))
stack.HasFunction("go.uber.org/goleak/internal/stack.TestCurrentCreatedBy.func1"),
"TestCurrentCreatedBy.func1 is not in stack:\n%s", stack.Full())
}

func TestAllLargeStack(t *testing.T) {
Expand Down Expand Up @@ -165,9 +167,10 @@ func TestAllLargeStack(t *testing.T) {

func TestParseFuncName(t *testing.T) {
tests := []struct {
name string
give string
want string
name string
give string
want string
creator bool
}{
{
name: "function",
Expand All @@ -180,22 +183,25 @@ func TestParseFuncName(t *testing.T) {
want: "example.com/foo/bar.(*baz).qux",
},
{
name: "created by", // Go 1.20
give: "created by example.com/foo/bar.baz",
want: "example.com/foo/bar.baz",
name: "created by", // Go 1.20
give: "created by example.com/foo/bar.baz",
want: "example.com/foo/bar.baz",
creator: true,
},
{
name: "created by/in goroutine", // Go 1.21
give: "created by example.com/foo/bar.baz in goroutine 123",
want: "example.com/foo/bar.baz",
name: "created by/in goroutine", // Go 1.21
give: "created by example.com/foo/bar.baz in goroutine 123",
want: "example.com/foo/bar.baz",
creator: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := parseFuncName(tt.give)
got, creator, err := parseFuncName(tt.give)
require.NoError(t, err)
assert.Equal(t, tt.want, got)
assert.Equal(t, tt.creator, creator)
})
}
}
Expand Down

0 comments on commit f46c0cd

Please sign in to comment.