From 1226362f1ebdf96d195349b263082ce129958136 Mon Sep 17 00:00:00 2001 From: 1911860538 Date: Tue, 9 Dec 2025 21:40:00 +0800 Subject: [PATCH] perf(recovery): optimize line reading in stack function --- recovery.go | 54 +++++++++++++++++++++++++++++----------- recovery_test.go | 65 +++++++++++++++++++++++++++++++++++++----------- 2 files changed, 89 insertions(+), 30 deletions(-) diff --git a/recovery.go b/recovery.go index e79e118a..6d4b4b2b 100644 --- a/recovery.go +++ b/recovery.go @@ -5,7 +5,9 @@ package gin import ( + "bufio" "bytes" + "cmp" "errors" "fmt" "io" @@ -21,9 +23,10 @@ import ( "github.com/gin-gonic/gin/internal/bytesconv" ) -const dunno = "???" - -var dunnoBytes = []byte(dunno) +const ( + dunno = "???" + stackSkip = 3 +) // RecoveryFunc defines the function passable to CustomRecovery. type RecoveryFunc func(c *Context, err any) @@ -72,7 +75,6 @@ func CustomRecoveryWithWriter(out io.Writer, handle RecoveryFunc) HandlerFunc { brokenPipe = true } if logger != nil { - const stackSkip = 3 if brokenPipe { logger.Printf("%s\n%s%s", err, secureRequestDump(c.Request), reset) } else if IsDebugging() { @@ -120,8 +122,11 @@ func stack(skip int) []byte { buf := new(bytes.Buffer) // the returned data // As we loop, we open files and read them. These variables record the currently // loaded file. - var lines [][]byte - var lastFile string + var ( + nLine string + lastFile string + err error + ) for i := skip; ; i++ { // Skip the expected number of frames pc, file, line, ok := runtime.Caller(i) if !ok { @@ -130,25 +135,44 @@ func stack(skip int) []byte { // Print this much at least. If we can't find the source, it won't show. fmt.Fprintf(buf, "%s:%d (0x%x)\n", file, line, pc) if file != lastFile { - data, err := os.ReadFile(file) + nLine, err = readNthLine(file, line-1) if err != nil { continue } - lines = bytes.Split(data, []byte{'\n'}) lastFile = file } - fmt.Fprintf(buf, "\t%s: %s\n", function(pc), source(lines, line)) + fmt.Fprintf(buf, "\t%s: %s\n", function(pc), cmp.Or(nLine, dunno)) } return buf.Bytes() } -// source returns a space-trimmed slice of the n'th line. -func source(lines [][]byte, n int) []byte { - n-- // in stack trace, lines are 1-indexed but our array is 0-indexed - if n < 0 || n >= len(lines) { - return dunnoBytes +// readNthLine reads the nth line from the file. +// It returns the trimmed content of the line if found, +// or an empty string if the line doesn't exist. +// If there's an error opening the file, it returns the error. +func readNthLine(file string, n int) (string, error) { + if n < 0 { + return "", nil } - return bytes.TrimSpace(lines[n]) + + f, err := os.Open(file) + if err != nil { + return "", err + } + defer f.Close() + + scanner := bufio.NewScanner(f) + for i := 0; i < n; i++ { + if !scanner.Scan() { + return "", nil + } + } + + if scanner.Scan() { + return strings.TrimSpace(scanner.Text()), nil + } + + return "", nil } // function returns, if possible, the name of the function containing the PC. diff --git a/recovery_test.go b/recovery_test.go index 073f4858..912ab601 100644 --- a/recovery_test.go +++ b/recovery_test.go @@ -88,21 +88,6 @@ func TestPanicWithAbort(t *testing.T) { assert.Equal(t, http.StatusBadRequest, w.Code) } -func TestSource(t *testing.T) { - bs := source(nil, 0) - assert.Equal(t, dunnoBytes, bs) - - in := [][]byte{ - []byte("Hello world."), - []byte("Hi, gin.."), - } - bs = source(in, 10) - assert.Equal(t, dunnoBytes, bs) - - bs = source(in, 1) - assert.Equal(t, []byte("Hello world."), bs) -} - func TestFunction(t *testing.T) { bs := function(1) assert.Equal(t, dunno, bs) @@ -331,3 +316,53 @@ func TestSecureRequestDump(t *testing.T) { }) } } + +// TestReadNthLine tests the readNthLine function with various scenarios. +func TestReadNthLine(t *testing.T) { + // Create a temporary test file + testContent := "line 0 \n line 1 \nline 2 \nline 3 \nline 4" + tempFile, err := os.CreateTemp("", "testfile*.txt") + if err != nil { + t.Fatal(err) + } + defer os.Remove(tempFile.Name()) + + // Write test content to the temporary file + if _, err := tempFile.WriteString(testContent); err != nil { + t.Fatal(err) + } + if err := tempFile.Close(); err != nil { + t.Fatal(err) + } + + // Test cases + tests := []struct { + name string + lineNum int + fileName string + want string + wantErr bool + }{ + {name: "Read first line", lineNum: 0, fileName: tempFile.Name(), want: "line 0", wantErr: false}, + {name: "Read middle line", lineNum: 2, fileName: tempFile.Name(), want: "line 2", wantErr: false}, + {name: "Read last line", lineNum: 4, fileName: tempFile.Name(), want: "line 4", wantErr: false}, + {name: "Line number exceeds file length", lineNum: 10, fileName: tempFile.Name(), want: "", wantErr: false}, + {name: "Negative line number", lineNum: -1, fileName: tempFile.Name(), want: "", wantErr: false}, + {name: "Non-existent file", lineNum: 1, fileName: "/non/existent/file.txt", want: "", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := readNthLine(tt.fileName, tt.lineNum) + assert.Equal(t, tt.wantErr, err != nil) + assert.Equal(t, tt.want, got) + }) + } +} + +func BenchmarkStack(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + _ = stack(stackSkip) + } +}