From 05de11fd7bceb7dac41cb4d410d2a18b5cadd51a Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Tue, 8 Feb 2022 15:30:00 +0100 Subject: [PATCH] SetLogger/ClearLogger/SetFilter: document as not thread-safe It has never been claimed that these functions are thread-safe and in practice they weren't because read access was not protected by the same mutex as the write access. These functions were meant to be called during program initialization, therefore the documentation is lacking, not the implementation. Making them thread-safe would imply adding mutex locking to the implementation of V, which most likely would have a noticeable effect. --- klog.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/klog.go b/klog.go index 1f201d06d..7cf60f9ee 100644 --- a/klog.go +++ b/klog.go @@ -965,15 +965,18 @@ func (rb *redirectBuffer) Write(bytes []byte) (n int, err error) { // // To remove a backing logr implemention, use ClearLogger. Setting an // empty logger with SetLogger(logr.Logger{}) does not work. +// +// Modifying the logger is not thread-safe and should be done while no other +// goroutines invoke log calls, usually during program initialization. func SetLogger(logr logr.Logger) { - logging.mu.Lock() - defer logging.mu.Unlock() - logging.logr = &logr } // ClearLogger removes a backing logr implementation if one was set earlier // with SetLogger. +// +// Modifying the logger is not thread-safe and should be done while no other +// goroutines invoke log calls, usually during program initialization. func ClearLogger() { logging.mu.Lock() defer logging.mu.Unlock() @@ -1774,10 +1777,11 @@ type LogFilter interface { FilterS(msg string, keysAndValues []interface{}) (string, []interface{}) } +// SetLogFilter installs a filter that is used for all log calls. +// +// Modifying the filter is not thread-safe and should be done while no other +// goroutines invoke log calls, usually during program initialization. func SetLogFilter(filter LogFilter) { - logging.mu.Lock() - defer logging.mu.Unlock() - logging.filter = filter }