From 272b3ca8fa06e0a7346e2a1689db1718bdf105db Mon Sep 17 00:00:00 2001 From: Alina Sudakov Date: Mon, 22 May 2023 11:48:21 +0300 Subject: [PATCH] Fix race conditions in logging package functions The logging package contains two functions, SetLogOptions and SetLogFile, that could experience race conditions when multiple goroutines access and modify the logger struct concurrently. To address these issues, a copy of the logger struct is now created in each function to eliminate data races. In addition, the test-go.sh script is updated to include the '-race' flag, enabling race detection during testing. This change helps prevent future race conditions by activating the Go race detector. Signed-off-by: Alina Sudakov --- hack/test-go.sh | 2 +- pkg/logging/logging.go | 33 ++++++++++++++++++++++----------- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/hack/test-go.sh b/hack/test-go.sh index 27bd0d6ef..e5cbbc3cf 100755 --- a/hack/test-go.sh +++ b/hack/test-go.sh @@ -19,5 +19,5 @@ if [ "$GO111MODULE" == "off" ]; then bash -c "umask 0; cd ${GOPATH}/src/${REPO_PATH}; PATH=${GOROOT}/bin:$(pwd)/bin:${PATH} go test -v -covermode=count -coverprofile=coverage.out ./..." else # test with go modules - bash -c "umask 0; go test -v -covermode=count -coverprofile=coverage.out ./..." + bash -c "umask 0; go test -v -race -covermode=atomic -coverprofile=coverage.out ./..." fi diff --git a/pkg/logging/logging.go b/pkg/logging/logging.go index 4cf001e8d..f621807f7 100644 --- a/pkg/logging/logging.go +++ b/pkg/logging/logging.go @@ -57,24 +57,29 @@ type LogOptions struct { // SetLogOptions set the LoggingOptions of NetConf func SetLogOptions(options *LogOptions) { // give some default value - logger.MaxSize = 100 - logger.MaxAge = 5 - logger.MaxBackups = 5 - logger.Compress = true + updatedLogger := lumberjack.Logger{ + Filename: logger.Filename, + MaxAge: 5, + MaxBackups: 5, + Compress: true, + MaxSize: 100, + LocalTime: logger.LocalTime, + } if options != nil { if options.MaxAge != nil { - logger.MaxAge = *options.MaxAge + updatedLogger.MaxAge = *options.MaxAge } if options.MaxSize != nil { - logger.MaxSize = *options.MaxSize + updatedLogger.MaxSize = *options.MaxSize } if options.MaxBackups != nil { - logger.MaxBackups = *options.MaxBackups + updatedLogger.MaxBackups = *options.MaxBackups } if options.Compress != nil { - logger.Compress = *options.Compress + updatedLogger.Compress = *options.Compress } } + logger = &updatedLogger loggingW = logger } @@ -174,10 +179,16 @@ func SetLogFile(filename string) { if filename == "" { return } - - logger.Filename = filename + updatedLogger := lumberjack.Logger{ + Filename: filename, + MaxAge: logger.MaxAge, + MaxBackups: logger.MaxBackups, + Compress: logger.Compress, + MaxSize: logger.MaxSize, + LocalTime: logger.LocalTime, + } + logger = &updatedLogger loggingW = logger - } func init() {