Fix data race (#8206)

* Fix data race

* Fix data race in modules/log

* Make the scope of lock finner-grained

* Use syc.Map

* Fix missing change in the test

* Do not export LoggerMap

diff --git a/integrations/testlogger.go b/integrations/testlogger.go
index 43a1471f66040ea5ad71db2a542f041545d00af8..91b94ac9f3ca57e4a09729741f7e0b6f5eb22abc 100644 (file)
--- a/integrations/testlogger.go
+++ b/integrations/testlogger.go
@@ -10,6 +10,7 @@ import (
        "os"
        "runtime"
        "strings"
+       "sync"
        "testing"
 
        "code.gitea.io/gitea/modules/log"
@@ -25,11 +26,21 @@ type TestLogger struct {
 var writerCloser = &testLoggerWriterCloser{}
 
 type testLoggerWriterCloser struct {
+       sync.RWMutex
        t testing.TB
 }
 
+func (w *testLoggerWriterCloser) setT(t *testing.TB) {
+       w.Lock()
+       w.t = *t
+       w.Unlock()
+}
+
 func (w *testLoggerWriterCloser) Write(p []byte) (int, error) {
-       if w.t != nil {
+       w.RLock()
+       t := w.t
+       w.RUnlock()
+       if t != nil {
                if len(p) > 0 && p[len(p)-1] == '\n' {
                        p = p[:len(p)-1]
                }
@@ -54,7 +65,7 @@ func (w *testLoggerWriterCloser) Write(p []byte) (int, error) {
                        }
                }()
 
-               w.t.Log(string(p))
+               t.Log(string(p))
                return len(p), nil
        }
        return len(p), nil
@@ -77,7 +88,7 @@ func PrintCurrentTest(t testing.TB, skip ...int) {
        } else {
                fmt.Fprintf(os.Stdout, "=== %s (%s:%d)\n", t.Name(), strings.TrimPrefix(filename, prefix), line)
        }
-       writerCloser.t = t
+       writerCloser.setT(&t)
 }
 
 // Printf takes a format and args and prints the string to os.Stdout

diff --git a/modules/log/log.go b/modules/log/log.go
index 0ca0f3adc5af1c0227e0060af3551a3a4e584d32..71e88491f1465e21dc57cd8c1920c46dae483855 100644 (file)
--- a/modules/log/log.go
+++ b/modules/log/log.go
@@ -8,13 +8,35 @@ import (
        "os"
        "runtime"
        "strings"
+       "sync"
 )
 
+type loggerMap struct {
+       sync.Map
+}
+
+func (m *loggerMap) Load(k string) (*Logger, bool) {
+       v, ok := m.Map.Load(k)
+       if !ok {
+               return nil, false
+       }
+       l, ok := v.(*Logger)
+       return l, ok
+}
+
+func (m *loggerMap) Store(k string, v *Logger) {
+       m.Map.Store(k, v)
+}
+
+func (m *loggerMap) Delete(k string) {
+       m.Map.Delete(k)
+}
+
 var (
        // DEFAULT is the name of the default logger
        DEFAULT = "default"
        // NamedLoggers map of named loggers
-       NamedLoggers = make(map[string]*Logger)
+       NamedLoggers loggerMap
        prefix       string
 )
 
@@ -25,16 +47,16 @@ func NewLogger(bufLen int64, name, provider, config string) *Logger {
                CriticalWithSkip(1, "Unable to create default logger: %v", err)
                panic(err)
        }
-       return NamedLoggers[DEFAULT]
+       l, _ := NamedLoggers.Load(DEFAULT)
+       return l
 }
 
 // NewNamedLogger creates a new named logger for a given configuration
 func NewNamedLogger(name string, bufLen int64, subname, provider, config string) error {
-       logger, ok := NamedLoggers[name]
+       logger, ok := NamedLoggers.Load(name)
        if !ok {
                logger = newLogger(name, bufLen)
-
-               NamedLoggers[name] = logger
+               NamedLoggers.Store(name, logger)
        }
 
        return logger.SetLogger(subname, provider, config)
@@ -42,16 +64,16 @@ func NewNamedLogger(name string, bufLen int64, subname, provider, config string)
 
 // DelNamedLogger closes and deletes the named logger
 func DelNamedLogger(name string) {
-       l, ok := NamedLoggers[name]
+       l, ok := NamedLoggers.Load(name)
        if ok {
-               delete(NamedLoggers, name)
+               NamedLoggers.Delete(name)
                l.Close()
        }
 }
 
 // DelLogger removes the named sublogger from the default logger
 func DelLogger(name string) error {
-       logger := NamedLoggers[DEFAULT]
+       logger, _ := NamedLoggers.Load(DEFAULT)
        found, err := logger.DelLogger(name)
        if !found {
                Trace("Log %s not found, no need to delete", name)
@@ -61,21 +83,24 @@ func DelLogger(name string) error {
 
 // GetLogger returns either a named logger or the default logger
 func GetLogger(name string) *Logger {
-       logger, ok := NamedLoggers[name]
+       logger, ok := NamedLoggers.Load(name)
        if ok {
                return logger
        }
-       return NamedLoggers[DEFAULT]
+       logger, _ = NamedLoggers.Load(DEFAULT)
+       return logger
 }
 
 // GetLevel returns the minimum logger level
 func GetLevel() Level {
-       return NamedLoggers[DEFAULT].GetLevel()
+       l, _ := NamedLoggers.Load(DEFAULT)
+       return l.GetLevel()
 }
 
 // GetStacktraceLevel returns the minimum logger level
 func GetStacktraceLevel() Level {
-       return NamedLoggers[DEFAULT].GetStacktraceLevel()
+       l, _ := NamedLoggers.Load(DEFAULT)
+       return l.GetStacktraceLevel()
 }
 
 // Trace records trace log
@@ -169,18 +194,18 @@ func IsFatal() bool {
 
 // Close closes all the loggers
 func Close() {
-       l, ok := NamedLoggers[DEFAULT]
+       l, ok := NamedLoggers.Load(DEFAULT)
        if !ok {
                return
        }
-       delete(NamedLoggers, DEFAULT)
+       NamedLoggers.Delete(DEFAULT)
        l.Close()
 }
 
 // Log a message with defined skip and at logging level
 // A skip of 0 refers to the caller of this command
 func Log(skip int, level Level, format string, v ...interface{}) {
-       l, ok := NamedLoggers[DEFAULT]
+       l, ok := NamedLoggers.Load(DEFAULT)
        if ok {
                l.Log(skip+1, level, format, v...)
        }
@@ -195,7 +220,8 @@ type LoggerAsWriter struct {
 // NewLoggerAsWriter creates a Writer representation of the logger with setable log level
 func NewLoggerAsWriter(level string, ourLoggers ...*Logger) *LoggerAsWriter {
        if len(ourLoggers) == 0 {
-               ourLoggers = []*Logger{NamedLoggers[DEFAULT]}
+               l, _ := NamedLoggers.Load(DEFAULT)
+               ourLoggers = []*Logger{l}
        }
        l := &LoggerAsWriter{
                ourLoggers: ourLoggers,

diff --git a/modules/log/log_test.go b/modules/log/log_test.go
index 9e3d7527b23dc0f15b32fb2a9f356d43a9660b59..0e7583081ceca49b49d397f70afb0d9eff7f3b3b 100644 (file)
--- a/modules/log/log_test.go
+++ b/modules/log/log_test.go
@@ -143,7 +143,7 @@ func TestNewNamedLogger(t *testing.T) {
        level := INFO
        err := NewNamedLogger("test", 0, "console", "console", fmt.Sprintf(`{"level":"%s"}`, level.String()))
        assert.NoError(t, err)
-       logger := NamedLoggers["test"]
+       logger, _ := NamedLoggers.Load("test")
        assert.Equal(t, level, logger.GetLevel())
 
        written, closed := baseConsoleTest(t, logger)