Skip to content

Commit

Permalink
fix logger unlock on panic
Browse files Browse the repository at this point in the history
  • Loading branch information
astraw99 committed Nov 19, 2021
1 parent 9ad2462 commit 13bbb7e
Showing 1 changed file with 10 additions and 1 deletion.
11 changes: 10 additions & 1 deletion klog.go
Expand Up @@ -918,6 +918,13 @@ func LogToStderr(stderr bool) {
// output writes the data to the log files and releases the buffer.
func (l *loggingT) output(s severity, log *logr.Logger, buf *buffer, depth int, file string, line int, alsoToStderr bool) {
l.mu.Lock()
defer func() {
if err := recover(); err != nil {
l.mu.Unlock() // Make sure to do `Unlock` if panic occurs below (eg: #L938 Error)
os.Stderr.Write([]byte("panic observed in loggingT.output")) // Make sure the message appears somewhere.
}
}()

if l.traceLocation.isSet() {
if l.traceLocation.match(file, line) {
buf.Write(stacks(false))
Expand All @@ -928,7 +935,7 @@ func (l *loggingT) output(s severity, log *logr.Logger, buf *buffer, depth int,
// TODO: set 'severity' and caller information as structured log info
// keysAndValues := []interface{}{"severity", severityName[s], "file", file, "line", line}
if s == errorLog {
l.logr.WithCallDepth(depth+3).Error(nil, string(data))
l.logr.WithCallDepth(depth+3).Error(nil, string(data)) // If panic here will be captured by defer
} else {
log.WithCallDepth(depth + 3).Info(string(data))
}
Expand All @@ -945,6 +952,7 @@ func (l *loggingT) output(s severity, log *logr.Logger, buf *buffer, depth int,
if l.file[infoLog] == nil {
if err := l.createFiles(infoLog); err != nil {
os.Stderr.Write(data) // Make sure the message appears somewhere.
l.mu.Unlock() // At this point `logExitFunc` is nil (checked in #L1068), so `Unlock` needed.
l.exit(err)
}
}
Expand All @@ -953,6 +961,7 @@ func (l *loggingT) output(s severity, log *logr.Logger, buf *buffer, depth int,
if l.file[s] == nil {
if err := l.createFiles(s); err != nil {
os.Stderr.Write(data) // Make sure the message appears somewhere.
l.mu.Unlock() // At this point `logExitFunc` is nil (checked in #L1068), so `Unlock` needed.
l.exit(err)
}
}
Expand Down

0 comments on commit 13bbb7e

Please sign in to comment.