Skip to content

Commit

Permalink
gracefully handle elided frames (#120)
Browse files Browse the repository at this point in the history
When parsing stacks we frequently encounter elided frames. The actual
stack line looks something like `...23 frames elided...`, which is
passed to `parseFuncName`, which of course is not parsable as a function
name. As a result, whenever this happens we get a useless panic like the
following:

```
panic: Failed to parse stack trace: parse function: no function found: "...23 frames elided..."
goroutine 1 [running]:
go.uber.org/goleak/internal/stack.getStackBuffer(0x1)
	/home/runner/go/pkg/mod/go.uber.org/goleak@v1.3.0/internal/stack/stacks.go:240 +0x65
go.uber.org/goleak/internal/stack.getStacks(0x1)
	/home/runner/go/pkg/mod/go.uber.org/goleak@v1.3.0/internal/stack/stacks.go:84 +0x3b
go.uber.org/goleak/internal/stack.All(...)
	/home/runner/go/pkg/mod/go.uber.org/goleak@v1.3.0/internal/stack/stacks.go:229
go.uber.org/goleak.Find({0xc0003ae880, 0x4, 0x4})
	/home/runner/go/pkg/mod/go.uber.org/goleak@v1.3.0/leaks.go:65 +0x1ea
```

This PR gracefully handles elided frames to prevent the panic.
  • Loading branch information
drshriveer committed Apr 13, 2024
1 parent 31095c6 commit b62053b
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 2 deletions.
7 changes: 7 additions & 0 deletions internal/stack/stacks.go
Expand Up @@ -158,6 +158,13 @@ func (p *stackParser) parseStack(line string) (Stack, error) {
// Just skip it.
continue
}
if strings.HasPrefix(line, "...") && strings.HasSuffix(line, " frames elided...") {
// e.g. ...23 frames elided...
// This indicates frames were elided from the stack trace,
// attempting to parse them via parseFuncName will fail resulting in a panic
// and a relatively useless output. Gracefully handle this.
continue
}

funcName, creator, err := parseFuncName(line)
if err != nil {
Expand Down
31 changes: 29 additions & 2 deletions internal/stack/stacks_test.go
Expand Up @@ -21,6 +21,7 @@
package stack

import (
"bytes"
"os"
"path/filepath"
"runtime"
Expand Down Expand Up @@ -136,8 +137,8 @@ func TestCurrentCreatedBy(t *testing.T) {

func TestAllLargeStack(t *testing.T) {
const (
stackDepth = 100
numGoroutines = 100
stackDepth = 101
numGoroutines = 101
)

var started sync.WaitGroup
Expand All @@ -163,6 +164,15 @@ func TestAllLargeStack(t *testing.T) {
t.Fatalf("Expected larger stack buffer")
}

// Also test the stack parser here to ensure it handles elided frames gracefully.
// We want to check this here, so that if the format of the "elided frames" message changes, we catch it.
// At the time of writing this test, with a stack depth of 101, we get 2 elided frames:
// "...2 frames elided...".
assert.Contains(t, string(buf), "frames elided...")
stacks, err := newStackParser(bytes.NewReader(buf)).Parse()
require.NoError(t, err)
assert.Greater(t, len(stacks), numGoroutines, "expect more parsed stacks than goroutines")

// Start enough goroutines so we exceed the default buffer size.
close(done)
}
Expand Down Expand Up @@ -263,6 +273,23 @@ func TestParseStack(t *testing.T) {
"example.com/foo/bar.baz",
},
},
{
name: "elided frames",
give: joinLines(
"goroutine 1 [running]:",
"example.com/foo/bar.baz()",
" example.com/foo/bar.go:123",
"...3 frames elided...",
"created by example.com/foo/bar.qux",
" example.com/foo/bar.go:456",
),
id: 1,
state: "running",
firstFunc: "example.com/foo/bar.baz",
funcs: []string{
"example.com/foo/bar.baz",
},
},
}

for _, tt := range tests {
Expand Down

0 comments on commit b62053b

Please sign in to comment.