Fix data race (#8204)

* 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
release/v1.15
Mura Li 2019-09-17 17:39:37 +08:00 committed by Lauris BH
parent a629904b30
commit eec997d30a
3 changed files with 57 additions and 20 deletions

View File

@ -10,6 +10,7 @@ import (
"os" "os"
"runtime" "runtime"
"strings" "strings"
"sync"
"testing" "testing"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
@ -25,11 +26,21 @@ type TestLogger struct {
var writerCloser = &testLoggerWriterCloser{} var writerCloser = &testLoggerWriterCloser{}
type testLoggerWriterCloser struct { type testLoggerWriterCloser struct {
sync.RWMutex
t testing.TB 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) { 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' { if len(p) > 0 && p[len(p)-1] == '\n' {
p = p[:len(p)-1] 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
} }
return len(p), nil return len(p), nil
@ -77,7 +88,7 @@ func PrintCurrentTest(t testing.TB, skip ...int) {
} else { } else {
fmt.Fprintf(os.Stdout, "=== %s (%s:%d)\n", t.Name(), strings.TrimPrefix(filename, prefix), line) 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 // Printf takes a format and args and prints the string to os.Stdout

View File

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

View File

@ -143,7 +143,7 @@ func TestNewNamedLogger(t *testing.T) {
level := INFO level := INFO
err := NewNamedLogger("test", 0, "console", "console", fmt.Sprintf(`{"level":"%s"}`, level.String())) err := NewNamedLogger("test", 0, "console", "console", fmt.Sprintf(`{"level":"%s"}`, level.String()))
assert.NoError(t, err) assert.NoError(t, err)
logger := NamedLoggers["test"] logger, _ := NamedLoggers.Load("test")
assert.Equal(t, level, logger.GetLevel()) assert.Equal(t, level, logger.GetLevel())
written, closed := baseConsoleTest(t, logger) written, closed := baseConsoleTest(t, logger)