From aa0e221163f44dc8cfd6b2d96eb3c383432893ea Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Thu, 14 Dec 2023 12:09:48 +0200 Subject: [PATCH] Update uber-go/goleak v1.2.1 => v1.3.0 - better built-in ignoring --- go.mod | 2 +- go.sum | 4 +- vendor/go.uber.org/goleak/CHANGELOG.md | 17 +- vendor/go.uber.org/goleak/README.md | 4 +- .../go.uber.org/goleak/internal/stack/scan.go | 56 +++++ .../goleak/internal/stack/stacks.go | 231 ++++++++++++++---- vendor/go.uber.org/goleak/leaks.go | 6 + vendor/go.uber.org/goleak/options.go | 28 ++- vendor/go.uber.org/goleak/tracestack_new.go | 10 +- vendor/modules.txt | 4 +- 10 files changed, 299 insertions(+), 63 deletions(-) create mode 100644 vendor/go.uber.org/goleak/internal/stack/scan.go diff --git a/go.mod b/go.mod index b4ea4d89b23..911b3932e29 100644 --- a/go.mod +++ b/go.mod @@ -44,7 +44,7 @@ require ( go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.19.0 go.opentelemetry.io/otel/sdk v1.19.0 go.opentelemetry.io/otel/trace v1.19.0 - go.uber.org/goleak v1.2.1 + go.uber.org/goleak v1.3.0 golang.org/x/crypto v0.16.0 golang.org/x/crypto/x509roots/fallback v0.0.0-20231129163542-152cdb1503eb golang.org/x/net v0.19.0 diff --git a/go.sum b/go.sum index 1d13149679f..9baec8d8457 100644 --- a/go.sum +++ b/go.sum @@ -220,8 +220,8 @@ go.opentelemetry.io/proto/otlp v1.0.0 h1:T0TX0tmXU8a3CbNXzEKGeU5mIVOdf0oykP+u2lI go.opentelemetry.io/proto/otlp v1.0.0/go.mod h1:Sy6pihPLfYHkr3NkUbEhGHFhINUSI/v80hjKIs5JXpM= go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A= -go.uber.org/goleak v1.2.1 h1:NBol2c7O1ZokfZ0LEU9K6Whx/KnwvepVetCUhtKja4A= -go.uber.org/goleak v1.2.1/go.mod h1:qlT2yGI9QafXHhZZLxlSuNsMw3FFLxBr+tBRlmO1xH4= +go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= +go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= go.uber.org/zap v1.18.1/go.mod h1:xg/QME4nWcxGxrpdeYfq7UvYrLh66cuVKdrbD1XF/NI= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= diff --git a/vendor/go.uber.org/goleak/CHANGELOG.md b/vendor/go.uber.org/goleak/CHANGELOG.md index 530f0a573f6..5cd3f88a587 100644 --- a/vendor/go.uber.org/goleak/CHANGELOG.md +++ b/vendor/go.uber.org/goleak/CHANGELOG.md @@ -4,6 +4,21 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). +## [1.3.0] +### Fixed +- Built-in ignores now match function names more accurately. + They will no longer ignore stacks because of file names + that look similar to function names. (#112) +### Added +- Add an `IgnoreAnyFunction` option to ignore stack traces + that have the provided function anywhere in the stack. (#113) +- Ignore `testing.runFuzzing` and `testing.runFuzzTests` alongside + other already-ignored test functions (`testing.RunTests`, etc). (#105) +### Changed +- Miscellaneous CI-related fixes. (#103, #108, #114) + +[1.3.0]: https://github.com/uber-go/goleak/compare/v1.2.1...v1.3.0 + ## [1.2.1] ### Changed - Drop golang/x/lint dependency. @@ -56,4 +71,4 @@ Thanks to @denis-tingajkin for their contributions to this release. ## [0.10.0] - Initial release. -[0.10.0]: https://github.com/uber-go/goleak/compare/v0.10.0...HEAD \ No newline at end of file +[0.10.0]: https://github.com/uber-go/goleak/compare/v0.10.0...HEAD diff --git a/vendor/go.uber.org/goleak/README.md b/vendor/go.uber.org/goleak/README.md index a545b5e7792..de3d7d51df7 100644 --- a/vendor/go.uber.org/goleak/README.md +++ b/vendor/go.uber.org/goleak/README.md @@ -67,8 +67,8 @@ No breaking changes will be made to exported APIs before 2.0. [doc-img]: https://godoc.org/go.uber.org/goleak?status.svg [doc]: https://godoc.org/go.uber.org/goleak -[ci-img]: https://github.com/uber-go/goleak/actions/workflows/go.yml/badge.svg -[ci]: https://github.com/uber-go/goleak/actions/workflows/go.yml +[ci-img]: https://github.com/uber-go/goleak/actions/workflows/ci.yml/badge.svg +[ci]: https://github.com/uber-go/goleak/actions/workflows/ci.yml [cov-img]: https://codecov.io/gh/uber-go/goleak/branch/master/graph/badge.svg [cov]: https://codecov.io/gh/uber-go/goleak [release]: https://go.dev/doc/devel/release#policy diff --git a/vendor/go.uber.org/goleak/internal/stack/scan.go b/vendor/go.uber.org/goleak/internal/stack/scan.go new file mode 100644 index 00000000000..4b7ac8423e7 --- /dev/null +++ b/vendor/go.uber.org/goleak/internal/stack/scan.go @@ -0,0 +1,56 @@ +// Copyright (c) 2023 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package stack + +import ( + "bufio" + "io" +) + +// scanner provides a bufio.Scanner the ability to Unscan, +// which allows the current token to be read again +// after the next Scan. +type scanner struct { + *bufio.Scanner + + unscanned bool +} + +func newScanner(r io.Reader) *scanner { + return &scanner{Scanner: bufio.NewScanner(r)} +} + +func (s *scanner) Scan() bool { + if s.unscanned { + s.unscanned = false + return true + } + return s.Scanner.Scan() +} + +// Unscan stops the scanner from advancing its position +// for the next Scan. +// +// Bytes and Text will return the same token after next Scan +// that they do right now. +func (s *scanner) Unscan() { + s.unscanned = true +} diff --git a/vendor/go.uber.org/goleak/internal/stack/stacks.go b/vendor/go.uber.org/goleak/internal/stack/stacks.go index 94f82e4c0d5..241a9b84482 100644 --- a/vendor/go.uber.org/goleak/internal/stack/stacks.go +++ b/vendor/go.uber.org/goleak/internal/stack/stacks.go @@ -1,4 +1,4 @@ -// Copyright (c) 2017 Uber Technologies, Inc. +// Copyright (c) 2017-2023 Uber Technologies, Inc. // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to deal @@ -21,8 +21,8 @@ package stack import ( - "bufio" "bytes" + "errors" "fmt" "io" "runtime" @@ -34,10 +34,17 @@ const _defaultBufferSize = 64 * 1024 // 64 KiB // Stack represents a single Goroutine's stack. type Stack struct { - id int - state string + id int + state string // e.g. 'running', 'chan receive' + + // The first function on the stack. firstFunction string - fullStack *bytes.Buffer + + // A set of all functions in the stack, + allFunctions map[string]struct{} + + // Full, raw stack trace. + fullStack string } // ID returns the goroutine ID. @@ -52,7 +59,7 @@ func (s Stack) State() string { // Full returns the full stack trace for this goroutine. func (s Stack) Full() string { - return s.fullStack.String() + return s.fullStack } // FirstFunction returns the name of the first function on the stack. @@ -60,6 +67,13 @@ func (s Stack) FirstFunction() string { return s.firstFunction } +// HasFunction reports whether the stack has the given function +// anywhere in it. +func (s Stack) HasFunction(name string) bool { + _, ok := s.allFunctions[name] + return ok +} + func (s Stack) String() string { return fmt.Sprintf( "Goroutine %v in state %v, with %v on top of the stack:\n%s", @@ -67,45 +81,147 @@ func (s Stack) String() string { } func getStacks(all bool) []Stack { - var stacks []Stack + trace := getStackBuffer(all) + stacks, err := newStackParser(bytes.NewReader(trace)).Parse() + if err != nil { + // Well-formed stack traces should never fail to parse. + // If they do, it's a bug in this package. + // Panic so we can fix it. + panic(fmt.Sprintf("Failed to parse stack trace: %v\n%s", err, trace)) + } + return stacks +} + +type stackParser struct { + scan *scanner + stacks []Stack + errors []error +} + +func newStackParser(r io.Reader) *stackParser { + return &stackParser{ + scan: newScanner(r), + } +} + +func (p *stackParser) Parse() ([]Stack, error) { + for p.scan.Scan() { + line := p.scan.Text() + + // If we see the goroutine header, start a new stack. + if strings.HasPrefix(line, "goroutine ") { + stack, err := p.parseStack(line) + if err != nil { + p.errors = append(p.errors, err) + continue + } + p.stacks = append(p.stacks, stack) + } + } + + p.errors = append(p.errors, p.scan.Err()) + return p.stacks, errors.Join(p.errors...) +} + +// parseStack parses a single stack trace from the given scanner. +// line is the first line of the stack trace, which should look like: +// +// goroutine 123 [runnable]: +func (p *stackParser) parseStack(line string) (Stack, error) { + id, state, err := parseGoStackHeader(line) + if err != nil { + return Stack{}, fmt.Errorf("parse header: %w", err) + } - var curStack *Stack - stackReader := bufio.NewReader(bytes.NewReader(getStackBuffer(all))) - for { - line, err := stackReader.ReadString('\n') - if err == io.EOF { + // Read the rest of the stack trace. + var ( + firstFunction string + fullStack bytes.Buffer + ) + funcs := make(map[string]struct{}) + for p.scan.Scan() { + line := p.scan.Text() + if strings.HasPrefix(line, "goroutine ") { + // If we see the goroutine header, + // it's the end of this stack. + // Unscan so the next Scan sees the same line. + p.scan.Unscan() break } - if err != nil { - // We're reading using bytes.NewReader which should never fail. - panic("bufio.NewReader failed on a fixed string") + + fullStack.WriteString(line) + fullStack.WriteByte('\n') // scanner trims the newline + + if len(line) == 0 { + // Empty line usually marks the end of the stack + // but we don't want to have to rely on that. + // Just skip it. + continue } - // If we see the goroutine header, start a new stack. - isFirstLine := false - if strings.HasPrefix(line, "goroutine ") { - // flush any previous stack - if curStack != nil { - stacks = append(stacks, *curStack) + funcName, creator, err := parseFuncName(line) + if err != nil { + return Stack{}, fmt.Errorf("parse function: %w", err) + } + 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 } - id, goState := parseGoStackHeader(line) - curStack = &Stack{ - id: id, - state: goState, - fullStack: &bytes.Buffer{}, + + } + + // The function name followed by a line in the form: + // + // example.com/path/to/package/file.go:123 +0x123 + // + // We don't care about the position so we can skip this line. + if p.scan.Scan() { + // Be defensive: + // Skip the line only if it starts with a tab. + bs := p.scan.Bytes() + if len(bs) > 0 && bs[0] == '\t' { + fullStack.Write(bs) + fullStack.WriteByte('\n') + } else { + // Put it back and let the next iteration handle it + // if it doesn't start with a tab. + p.scan.Unscan() } - isFirstLine = true } - curStack.fullStack.WriteString(line) - if !isFirstLine && curStack.firstFunction == "" { - curStack.firstFunction = parseFirstFunc(line) + + 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 } } - if curStack != nil { - stacks = append(stacks, *curStack) - } - return stacks + return Stack{ + id: id, + state: state, + firstFunction: firstFunction, + allFunctions: funcs, + fullStack: fullStack.String(), + }, nil } // All returns the stacks for all running goroutines. @@ -127,29 +243,56 @@ func getStackBuffer(all bool) []byte { } } -func parseFirstFunc(line string) string { - line = strings.TrimSpace(line) - if idx := strings.LastIndex(line, "("); idx > 0 { - return line[:idx] +// Parses a single function from the given line. +// The line is in one of these formats: +// +// example.com/path/to/package.funcName(args...) +// 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 [...] +// +// 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 [...]". + idx := strings.Index(after, " in goroutine") + if idx >= 0 { + 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] } - panic(fmt.Sprintf("function calls missing parents: %q", line)) + + if name == "" { + return "", false, fmt.Errorf("no function found: %q", line) + } + + return name, creator, nil } // parseGoStackHeader parses a stack header that looks like: // goroutine 643 [runnable]:\n // And returns the goroutine ID, and the state. -func parseGoStackHeader(line string) (goroutineID int, state string) { - line = strings.TrimSuffix(line, ":\n") +func parseGoStackHeader(line string) (goroutineID int, state string, err error) { + // The scanner will have already trimmed the "\n", + // but we'll guard against it just in case. + // + // Trimming them separately makes them both optional. + line = strings.TrimSuffix(strings.TrimSuffix(line, ":"), "\n") parts := strings.SplitN(line, " ", 3) if len(parts) != 3 { - panic(fmt.Sprintf("unexpected stack header format: %q", line)) + return 0, "", fmt.Errorf("unexpected format: %q", line) } id, err := strconv.Atoi(parts[1]) if err != nil { - panic(fmt.Sprintf("failed to parse goroutine ID: %v in line %q", parts[1], line)) + return 0, "", fmt.Errorf("bad goroutine ID %q in line %q", parts[1], line) } state = strings.TrimSuffix(strings.TrimPrefix(parts[2], "["), "]") - return id, state + return id, state, nil } diff --git a/vendor/go.uber.org/goleak/leaks.go b/vendor/go.uber.org/goleak/leaks.go index ee122b7464e..cc206f1815b 100644 --- a/vendor/go.uber.org/goleak/leaks.go +++ b/vendor/go.uber.org/goleak/leaks.go @@ -82,6 +82,12 @@ type testHelper interface { // tests by doing: // // defer VerifyNone(t) +// +// VerifyNone is currently incompatible with t.Parallel because it cannot +// associate specific goroutines with specific tests. Thus, non-leaking +// goroutines from other tests running in parallel could fail this check. +// If you need to run tests in parallel, use [VerifyTestMain] instead, +// which will verify that no leaking goroutines exist after ALL tests finish. func VerifyNone(t TestingT, options ...Option) { opts := buildOpts(options...) var cleanup func(int) diff --git a/vendor/go.uber.org/goleak/options.go b/vendor/go.uber.org/goleak/options.go index d2d473b71eb..53fc0a1decc 100644 --- a/vendor/go.uber.org/goleak/options.go +++ b/vendor/go.uber.org/goleak/options.go @@ -1,4 +1,4 @@ -// Copyright (c) 2017 Uber Technologies, Inc. +// Copyright (c) 2017-2023 Uber Technologies, Inc. // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to deal @@ -67,6 +67,22 @@ func IgnoreTopFunction(f string) Option { }) } +// IgnoreAnyFunction ignores goroutines where the specified function +// is present anywhere in the stack. +// +// The function name must be fully qualified, e.g., +// +// go.uber.org/goleak.IgnoreAnyFunction +// +// For methods, the fully qualified form looks like: +// +// go.uber.org/goleak.(*MyType).MyMethod +func IgnoreAnyFunction(f string) Option { + return addFilter(func(s stack.Stack) bool { + return s.HasFunction(f) + }) +} + // Cleanup sets up a cleanup function that will be executed at the // end of the leak check. // When passed to [VerifyTestMain], the exit code passed to cleanupFunc @@ -151,8 +167,12 @@ func isTestStack(s stack.Stack) bool { // Since go1.7, a separate goroutine is started to wait for signals. // T.Parallel is for parallel tests, which are blocked until all serial // tests have run with T.Parallel at the top of the stack. + // testing.runFuzzTests is for fuzz testing, it's blocked until the test + // function with all seed corpus have run. + // testing.runFuzzing is for fuzz testing, it's blocked until a failing + // input is found. switch s.FirstFunction() { - case "testing.RunTests", "testing.(*T).Run", "testing.(*T).Parallel": + case "testing.RunTests", "testing.(*T).Run", "testing.(*T).Parallel", "testing.runFuzzing", "testing.runFuzzTests": // In pre1.7 and post-1.7, background goroutines started by the testing // package are blocked waiting on a channel. return strings.HasPrefix(s.State(), "chan receive") @@ -163,7 +183,7 @@ func isTestStack(s stack.Stack) bool { func isSyscallStack(s stack.Stack) bool { // Typically runs in the background when code uses CGo: // https://github.com/golang/go/issues/16714 - return s.FirstFunction() == "runtime.goexit" && strings.HasPrefix(s.State(), "syscall") + return s.HasFunction("runtime.goexit") && strings.HasPrefix(s.State(), "syscall") } func isStdLibStack(s stack.Stack) bool { @@ -174,5 +194,5 @@ func isStdLibStack(s stack.Stack) bool { } // Using signal.Notify will start a runtime goroutine. - return strings.Contains(s.Full(), "runtime.ensureSigM") + return s.HasFunction("runtime.ensureSigM") } diff --git a/vendor/go.uber.org/goleak/tracestack_new.go b/vendor/go.uber.org/goleak/tracestack_new.go index d12ffd84043..4fc6cefcea9 100644 --- a/vendor/go.uber.org/goleak/tracestack_new.go +++ b/vendor/go.uber.org/goleak/tracestack_new.go @@ -1,4 +1,4 @@ -// Copyright (c) 2021 Uber Technologies, Inc. +// Copyright (c) 2021-2023 Uber Technologies, Inc. // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to deal @@ -23,12 +23,8 @@ package goleak -import ( - "strings" - - "go.uber.org/goleak/internal/stack" -) +import "go.uber.org/goleak/internal/stack" func isTraceStack(s stack.Stack) bool { - return strings.Contains(s.Full(), "runtime.ReadTrace") + return s.HasFunction("runtime.ReadTrace") } diff --git a/vendor/modules.txt b/vendor/modules.txt index b60efffca12..c5dce19ffbb 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -369,8 +369,8 @@ go.opentelemetry.io/proto/otlp/collector/trace/v1 go.opentelemetry.io/proto/otlp/common/v1 go.opentelemetry.io/proto/otlp/resource/v1 go.opentelemetry.io/proto/otlp/trace/v1 -# go.uber.org/goleak v1.2.1 -## explicit; go 1.18 +# go.uber.org/goleak v1.3.0 +## explicit; go 1.20 go.uber.org/goleak go.uber.org/goleak/internal/stack # golang.org/x/crypto v0.16.0