Skip to content

Commit

Permalink
pkg/util/log: clean up some unsafe lock usage in bufferedSink
Browse files Browse the repository at this point in the history
There are multiple undeferred lock usages in the bufferedSink
beyond just the one that surrounds the call to `appendMsg`.

This small patch cleans these usages up to use `defer` instead.

Release note: none
  • Loading branch information
abarganier committed Aug 23, 2023
1 parent 8bb6d19 commit 96acea6
Showing 1 changed file with 11 additions and 7 deletions.
18 changes: 11 additions & 7 deletions pkg/util/log/buffered_sink.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,8 @@ func (bs *bufferedSink) output(b []byte, opts sinkOutputOptions) error {
if bs.mu.timer == nil && bs.maxStaleness > 0 {
bs.mu.timer = time.AfterFunc(bs.maxStaleness, func() {
bs.mu.Lock()
defer bs.mu.Unlock()
bs.flushAsyncLocked()
bs.mu.Unlock()
})
}
}
Expand Down Expand Up @@ -330,9 +330,11 @@ func (bs *bufferedSink) runFlusher(stopC <-chan struct{}) {
// We'll return after flushing everything.
done = true
}
bs.mu.Lock()
msg, errC := buf.flush(bs.format.prefix, bs.format.suffix, bs.format.delimiter)
bs.mu.Unlock()
msg, errC := func() (*buffer, chan<- error) {
bs.mu.Lock()
defer bs.mu.Unlock()
return buf.flush(bs.format.prefix, bs.format.suffix, bs.format.delimiter)
}()
if msg == nil {
// Nothing to flush.
// NOTE: This can happen in the done case, or if we get two flushC signals
Expand All @@ -350,9 +352,11 @@ func (bs *bufferedSink) runFlusher(stopC <-chan struct{}) {
} else if err != nil {
Ops.Errorf(context.Background(), "logging error from %T: %v", bs.child, err)
if bs.crashOnAsyncFlushFailure {
logging.mu.Lock()
f := logging.mu.exitOverride.f
logging.mu.Unlock()
f := func() func(exit.Code, error) {
logging.mu.Lock()
defer logging.mu.Unlock()
return logging.mu.exitOverride.f
}()
code := bs.exitCode()
if f != nil {
f(code, err)
Expand Down

0 comments on commit 96acea6

Please sign in to comment.