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 23, 2021
1 parent 9ad2462 commit f930164
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 2 deletions.
14 changes: 12 additions & 2 deletions klog.go
Expand Up @@ -917,7 +917,15 @@ 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) {
var isLocked = true
l.mu.Lock()
defer func() {
if isLocked {
// Unlock before returning in case that it wasn't done already.
l.mu.Unlock()
}
}()

if l.traceLocation.isSet() {
if l.traceLocation.match(file, line) {
buf.Write(stacks(false))
Expand Down Expand Up @@ -980,6 +988,7 @@ func (l *loggingT) output(s severity, log *logr.Logger, buf *buffer, depth int,
// If we got here via Exit rather than Fatal, print no stacks.
if atomic.LoadUint32(&fatalNoStacks) > 0 {
l.mu.Unlock()
isLocked = false
timeoutFlush(10 * time.Second)
os.Exit(1)
}
Expand All @@ -997,11 +1006,12 @@ func (l *loggingT) output(s severity, log *logr.Logger, buf *buffer, depth int,
}
}
l.mu.Unlock()
isLocked = false
timeoutFlush(10 * time.Second)
os.Exit(255) // C++ uses -1, which is silly because it's anded with 255 anyway.
os.Exit(255) // C++ uses -1, which is silly because it's anded(&) with 255 anyway.
}
l.putBuffer(buf)
l.mu.Unlock()

if stats := severityStats[s]; stats != nil {
atomic.AddInt64(&stats.lines, 1)
atomic.AddInt64(&stats.bytes, int64(len(data)))
Expand Down
32 changes: 32 additions & 0 deletions klog_test.go
Expand Up @@ -1844,3 +1844,35 @@ func TestKObjs(t *testing.T) {
})
}
}

// Benchmark test for lock with/without defer
type structWithLock struct {
m sync.Mutex
n int64
}

func BenchmarkWithoutDeferUnLock(b *testing.B) {
s := structWithLock{}
for i := 0; i < b.N; i++ {
s.addWithoutDefer()
}
}

func BenchmarkWithDeferUnLock(b *testing.B) {
s := structWithLock{}
for i := 0; i < b.N; i++ {
s.addWithDefer()
}
}

func (s *structWithLock) addWithoutDefer() {
s.m.Lock()
s.n++
s.m.Unlock()
}

func (s *structWithLock) addWithDefer() {
s.m.Lock()
defer s.m.Unlock()
s.n++
}

0 comments on commit f930164

Please sign in to comment.