Skip to content

Commit

Permalink
util: fix recover log doesn't contains stack (#49868)
Browse files Browse the repository at this point in the history
close #49867
D3Hunter authored Dec 28, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 3b41717 commit b56a09b
Showing 4 changed files with 31 additions and 6 deletions.
1 change: 1 addition & 0 deletions pkg/util/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -82,6 +82,7 @@ go_test(
"//pkg/testkit/testsetup",
"//pkg/types",
"//pkg/util/fastrand",
"//pkg/util/logutil",
"//pkg/util/memory",
"@com_github_pingcap_errors//:errors",
"@com_github_stretchr_testify//assert",
2 changes: 1 addition & 1 deletion pkg/util/cpuprofile/cpuprofile.go
Original file line number Diff line number Diff line change
@@ -211,7 +211,7 @@ func (p *parallelCPUProfiler) sendToConsumers() {
defer func() {
p.Unlock()
if r := recover(); r != nil {
logutil.BgLogger().Error("parallel cpu profiler panic", zap.Any("recover", r))
logutil.BgLogger().Error("parallel cpu profiler panic", zap.Any("recover", r), zap.Stack("stack"))
}
}()

3 changes: 1 addition & 2 deletions pkg/util/wait_group_wrapper.go
Original file line number Diff line number Diff line change
@@ -229,8 +229,7 @@ func (g *ErrorGroupWithRecover) Go(fn func() error) {
g.Group.Go(func() (err error) {
defer func() {
if r := recover(); r != nil {
// stack is automatically printed
logutil.BgLogger().Error("panic in error group", zap.Any("recover", r))
logutil.BgLogger().Error("panic in error group", zap.Any("recover", r), zap.Stack("stack"))
err = GetRecoverError(r)
}
}()
31 changes: 28 additions & 3 deletions pkg/util/wait_group_wrapper_test.go
Original file line number Diff line number Diff line change
@@ -16,9 +16,12 @@ package util

import (
"fmt"
"os"
"path"
"testing"
"time"

"github.com/pingcap/tidb/pkg/util/logutil"
"github.com/stretchr/testify/require"
"go.uber.org/atomic"
)
@@ -91,11 +94,33 @@ func TestWaitGroupWrapperCheck(t *testing.T) {
require.False(t, wg.check())
}

func middleF() {
var a int
_ = 10 / a
}

func TestNewErrorGroupWithRecover(t *testing.T) {
bak := os.Stdout
t.Cleanup(func() {
os.Stdout = bak
})
tempDir := t.TempDir()
fileName := path.Join(tempDir, "test.log")
file, err := os.OpenFile(fileName, os.O_CREATE|os.O_WRONLY, 0644)
require.NoError(t, err)
os.Stdout = file
// InitLogger contains zap.AddStacktrace(zapcore.FatalLevel), so log level
// below fatal will not contain stack automatically.
require.NoError(t, logutil.InitLogger(&logutil.LogConfig{}))
eg := NewErrorGroupWithRecover()
eg.Go(func() error {
panic("test")
middleF()
return nil
})
err := eg.Wait()
require.Errorf(t, err, "test")
err = eg.Wait()
require.ErrorContains(t, err, "runtime error: integer divide by zero")
require.NoError(t, file.Close())
content, err := os.ReadFile(fileName)
require.NoError(t, err)
require.Contains(t, string(content), "pkg/util.middleF")
}

0 comments on commit b56a09b

Please sign in to comment.