From c076bdc893fbd4fb94a7d35476198d4fbd05caec Mon Sep 17 00:00:00 2001 From: Dmitry Titov Date: Tue, 17 Dec 2024 15:13:30 +0700 Subject: [PATCH] fix logger race --- AUTHORS | 1 + driver_test.go | 2 +- dsn.go | 4 +- dsn_test.go | 125 ++++++++++++++++++++++++++----------------------- errors.go | 18 ++++++- errors_test.go | 6 +-- 6 files changed, 89 insertions(+), 67 deletions(-) diff --git a/AUTHORS b/AUTHORS index 361c6b647..a1c0e7786 100644 --- a/AUTHORS +++ b/AUTHORS @@ -35,6 +35,7 @@ Daniël van Eeden Dave Protasowski Dirkjan Bussink DisposaBoy +Dmitry Titov Egor Smolyakov Erwan Martin Evan Elias diff --git a/driver_test.go b/driver_test.go index 24d73c34f..b30ae129b 100644 --- a/driver_test.go +++ b/driver_test.go @@ -2141,7 +2141,7 @@ func TestInsertRetrieveEscapedData(t *testing.T) { func TestUnixSocketAuthFail(t *testing.T) { runTests(t, dsn, func(dbt *DBTest) { // Save the current logger so we can restore it. - oldLogger := defaultLogger + oldLogger := getLogger() // Set a new logger so we can capture its output. buffer := bytes.NewBuffer(make([]byte, 0, 64)) diff --git a/dsn.go b/dsn.go index f391a8fc9..c3eaefb7e 100644 --- a/dsn.go +++ b/dsn.go @@ -89,7 +89,7 @@ func NewConfig() *Config { cfg := &Config{ Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, - Logger: defaultLogger, + Logger: getLogger(), AllowNativePasswords: true, CheckConnLiveness: true, } @@ -203,7 +203,7 @@ func (cfg *Config) normalize() error { } if cfg.Logger == nil { - cfg.Logger = defaultLogger + cfg.Logger = getLogger() } return nil diff --git a/dsn_test.go b/dsn_test.go index 863d14824..b415e7960 100644 --- a/dsn_test.go +++ b/dsn_test.go @@ -17,67 +17,74 @@ import ( "time" ) -var testDSNs = []struct { +var testDSNs []struct { in string out *Config -}{{ - "username:password@protocol(address)/dbname?param=value", - &Config{User: "username", Passwd: "password", Net: "protocol", Addr: "address", DBName: "dbname", Params: map[string]string{"param": "value"}, Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: defaultLogger, AllowNativePasswords: true, CheckConnLiveness: true}, -}, { - "username:password@protocol(address)/dbname?param=value&columnsWithAlias=true", - &Config{User: "username", Passwd: "password", Net: "protocol", Addr: "address", DBName: "dbname", Params: map[string]string{"param": "value"}, Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: defaultLogger, AllowNativePasswords: true, CheckConnLiveness: true, ColumnsWithAlias: true}, -}, { - "username:password@protocol(address)/dbname?param=value&columnsWithAlias=true&multiStatements=true", - &Config{User: "username", Passwd: "password", Net: "protocol", Addr: "address", DBName: "dbname", Params: map[string]string{"param": "value"}, Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: defaultLogger, AllowNativePasswords: true, CheckConnLiveness: true, ColumnsWithAlias: true, MultiStatements: true}, -}, { - "user@unix(/path/to/socket)/dbname?charset=utf8", - &Config{User: "user", Net: "unix", Addr: "/path/to/socket", DBName: "dbname", charsets: []string{"utf8"}, Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: defaultLogger, AllowNativePasswords: true, CheckConnLiveness: true}, -}, { - "user:password@tcp(localhost:5555)/dbname?charset=utf8&tls=true", - &Config{User: "user", Passwd: "password", Net: "tcp", Addr: "localhost:5555", DBName: "dbname", charsets: []string{"utf8"}, Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: defaultLogger, AllowNativePasswords: true, CheckConnLiveness: true, TLSConfig: "true"}, -}, { - "user:password@tcp(localhost:5555)/dbname?charset=utf8mb4,utf8&tls=skip-verify", - &Config{User: "user", Passwd: "password", Net: "tcp", Addr: "localhost:5555", DBName: "dbname", charsets: []string{"utf8mb4", "utf8"}, Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: defaultLogger, AllowNativePasswords: true, CheckConnLiveness: true, TLSConfig: "skip-verify"}, -}, { - "user:password@/dbname?loc=UTC&timeout=30s&readTimeout=1s&writeTimeout=1s&allowAllFiles=1&clientFoundRows=true&allowOldPasswords=TRUE&collation=utf8mb4_unicode_ci&maxAllowedPacket=16777216&tls=false&allowCleartextPasswords=true&parseTime=true&rejectReadOnly=true", - &Config{User: "user", Passwd: "password", Net: "tcp", Addr: "127.0.0.1:3306", DBName: "dbname", Collation: "utf8mb4_unicode_ci", Loc: time.UTC, TLSConfig: "false", AllowCleartextPasswords: true, AllowNativePasswords: true, Timeout: 30 * time.Second, ReadTimeout: time.Second, WriteTimeout: time.Second, Logger: defaultLogger, AllowAllFiles: true, AllowOldPasswords: true, CheckConnLiveness: true, ClientFoundRows: true, MaxAllowedPacket: 16777216, ParseTime: true, RejectReadOnly: true}, -}, { - "user:password@/dbname?allowNativePasswords=false&checkConnLiveness=false&maxAllowedPacket=0&allowFallbackToPlaintext=true", - &Config{User: "user", Passwd: "password", Net: "tcp", Addr: "127.0.0.1:3306", DBName: "dbname", Loc: time.UTC, MaxAllowedPacket: 0, Logger: defaultLogger, AllowFallbackToPlaintext: true, AllowNativePasswords: false, CheckConnLiveness: false}, -}, { - "user:p@ss(word)@tcp([de:ad:be:ef::ca:fe]:80)/dbname?loc=Local", - &Config{User: "user", Passwd: "p@ss(word)", Net: "tcp", Addr: "[de:ad:be:ef::ca:fe]:80", DBName: "dbname", Loc: time.Local, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: defaultLogger, AllowNativePasswords: true, CheckConnLiveness: true}, -}, { - "/dbname", - &Config{Net: "tcp", Addr: "127.0.0.1:3306", DBName: "dbname", Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: defaultLogger, AllowNativePasswords: true, CheckConnLiveness: true}, -}, { - "/dbname%2Fwithslash", - &Config{Net: "tcp", Addr: "127.0.0.1:3306", DBName: "dbname/withslash", Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: defaultLogger, AllowNativePasswords: true, CheckConnLiveness: true}, -}, { - "@/", - &Config{Net: "tcp", Addr: "127.0.0.1:3306", Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: defaultLogger, AllowNativePasswords: true, CheckConnLiveness: true}, -}, { - "/", - &Config{Net: "tcp", Addr: "127.0.0.1:3306", Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: defaultLogger, AllowNativePasswords: true, CheckConnLiveness: true}, -}, { - "", - &Config{Net: "tcp", Addr: "127.0.0.1:3306", Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: defaultLogger, AllowNativePasswords: true, CheckConnLiveness: true}, -}, { - "user:p@/ssword@/", - &Config{User: "user", Passwd: "p@/ssword", Net: "tcp", Addr: "127.0.0.1:3306", Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: defaultLogger, AllowNativePasswords: true, CheckConnLiveness: true}, -}, { - "unix/?arg=%2Fsome%2Fpath.ext", - &Config{Net: "unix", Addr: "/tmp/mysql.sock", Params: map[string]string{"arg": "/some/path.ext"}, Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: defaultLogger, AllowNativePasswords: true, CheckConnLiveness: true}, -}, { - "tcp(127.0.0.1)/dbname", - &Config{Net: "tcp", Addr: "127.0.0.1:3306", DBName: "dbname", Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: defaultLogger, AllowNativePasswords: true, CheckConnLiveness: true}, -}, { - "tcp(de:ad:be:ef::ca:fe)/dbname", - &Config{Net: "tcp", Addr: "[de:ad:be:ef::ca:fe]:3306", DBName: "dbname", Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: defaultLogger, AllowNativePasswords: true, CheckConnLiveness: true}, -}, { - "user:password@/dbname?loc=UTC&timeout=30s&parseTime=true&timeTruncate=1h", - &Config{User: "user", Passwd: "password", Net: "tcp", Addr: "127.0.0.1:3306", DBName: "dbname", Loc: time.UTC, Timeout: 30 * time.Second, ParseTime: true, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: defaultLogger, AllowNativePasswords: true, CheckConnLiveness: true, timeTruncate: time.Hour}, -}, +} + +func init() { + testDSNs = []struct { + in string + out *Config + }{{ + "username:password@protocol(address)/dbname?param=value", + &Config{User: "username", Passwd: "password", Net: "protocol", Addr: "address", DBName: "dbname", Params: map[string]string{"param": "value"}, Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: getLogger(), AllowNativePasswords: true, CheckConnLiveness: true}, + }, { + "username:password@protocol(address)/dbname?param=value&columnsWithAlias=true", + &Config{User: "username", Passwd: "password", Net: "protocol", Addr: "address", DBName: "dbname", Params: map[string]string{"param": "value"}, Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: getLogger(), AllowNativePasswords: true, CheckConnLiveness: true, ColumnsWithAlias: true}, + }, { + "username:password@protocol(address)/dbname?param=value&columnsWithAlias=true&multiStatements=true", + &Config{User: "username", Passwd: "password", Net: "protocol", Addr: "address", DBName: "dbname", Params: map[string]string{"param": "value"}, Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: getLogger(), AllowNativePasswords: true, CheckConnLiveness: true, ColumnsWithAlias: true, MultiStatements: true}, + }, { + "user@unix(/path/to/socket)/dbname?charset=utf8", + &Config{User: "user", Net: "unix", Addr: "/path/to/socket", DBName: "dbname", charsets: []string{"utf8"}, Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: getLogger(), AllowNativePasswords: true, CheckConnLiveness: true}, + }, { + "user:password@tcp(localhost:5555)/dbname?charset=utf8&tls=true", + &Config{User: "user", Passwd: "password", Net: "tcp", Addr: "localhost:5555", DBName: "dbname", charsets: []string{"utf8"}, Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: getLogger(), AllowNativePasswords: true, CheckConnLiveness: true, TLSConfig: "true"}, + }, { + "user:password@tcp(localhost:5555)/dbname?charset=utf8mb4,utf8&tls=skip-verify", + &Config{User: "user", Passwd: "password", Net: "tcp", Addr: "localhost:5555", DBName: "dbname", charsets: []string{"utf8mb4", "utf8"}, Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: getLogger(), AllowNativePasswords: true, CheckConnLiveness: true, TLSConfig: "skip-verify"}, + }, { + "user:password@/dbname?loc=UTC&timeout=30s&readTimeout=1s&writeTimeout=1s&allowAllFiles=1&clientFoundRows=true&allowOldPasswords=TRUE&collation=utf8mb4_unicode_ci&maxAllowedPacket=16777216&tls=false&allowCleartextPasswords=true&parseTime=true&rejectReadOnly=true", + &Config{User: "user", Passwd: "password", Net: "tcp", Addr: "127.0.0.1:3306", DBName: "dbname", Collation: "utf8mb4_unicode_ci", Loc: time.UTC, TLSConfig: "false", AllowCleartextPasswords: true, AllowNativePasswords: true, Timeout: 30 * time.Second, ReadTimeout: time.Second, WriteTimeout: time.Second, Logger: getLogger(), AllowAllFiles: true, AllowOldPasswords: true, CheckConnLiveness: true, ClientFoundRows: true, MaxAllowedPacket: 16777216, ParseTime: true, RejectReadOnly: true}, + }, { + "user:password@/dbname?allowNativePasswords=false&checkConnLiveness=false&maxAllowedPacket=0&allowFallbackToPlaintext=true", + &Config{User: "user", Passwd: "password", Net: "tcp", Addr: "127.0.0.1:3306", DBName: "dbname", Loc: time.UTC, MaxAllowedPacket: 0, Logger: getLogger(), AllowFallbackToPlaintext: true, AllowNativePasswords: false, CheckConnLiveness: false}, + }, { + "user:p@ss(word)@tcp([de:ad:be:ef::ca:fe]:80)/dbname?loc=Local", + &Config{User: "user", Passwd: "p@ss(word)", Net: "tcp", Addr: "[de:ad:be:ef::ca:fe]:80", DBName: "dbname", Loc: time.Local, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: getLogger(), AllowNativePasswords: true, CheckConnLiveness: true}, + }, { + "/dbname", + &Config{Net: "tcp", Addr: "127.0.0.1:3306", DBName: "dbname", Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: getLogger(), AllowNativePasswords: true, CheckConnLiveness: true}, + }, { + "/dbname%2Fwithslash", + &Config{Net: "tcp", Addr: "127.0.0.1:3306", DBName: "dbname/withslash", Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: getLogger(), AllowNativePasswords: true, CheckConnLiveness: true}, + }, { + "@/", + &Config{Net: "tcp", Addr: "127.0.0.1:3306", Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: getLogger(), AllowNativePasswords: true, CheckConnLiveness: true}, + }, { + "/", + &Config{Net: "tcp", Addr: "127.0.0.1:3306", Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: getLogger(), AllowNativePasswords: true, CheckConnLiveness: true}, + }, { + "", + &Config{Net: "tcp", Addr: "127.0.0.1:3306", Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: getLogger(), AllowNativePasswords: true, CheckConnLiveness: true}, + }, { + "user:p@/ssword@/", + &Config{User: "user", Passwd: "p@/ssword", Net: "tcp", Addr: "127.0.0.1:3306", Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: getLogger(), AllowNativePasswords: true, CheckConnLiveness: true}, + }, { + "unix/?arg=%2Fsome%2Fpath.ext", + &Config{Net: "unix", Addr: "/tmp/mysql.sock", Params: map[string]string{"arg": "/some/path.ext"}, Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: getLogger(), AllowNativePasswords: true, CheckConnLiveness: true}, + }, { + "tcp(127.0.0.1)/dbname", + &Config{Net: "tcp", Addr: "127.0.0.1:3306", DBName: "dbname", Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: getLogger(), AllowNativePasswords: true, CheckConnLiveness: true}, + }, { + "tcp(de:ad:be:ef::ca:fe)/dbname", + &Config{Net: "tcp", Addr: "[de:ad:be:ef::ca:fe]:3306", DBName: "dbname", Loc: time.UTC, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: getLogger(), AllowNativePasswords: true, CheckConnLiveness: true}, + }, { + "user:password@/dbname?loc=UTC&timeout=30s&parseTime=true&timeTruncate=1h", + &Config{User: "user", Passwd: "password", Net: "tcp", Addr: "127.0.0.1:3306", DBName: "dbname", Loc: time.UTC, Timeout: 30 * time.Second, ParseTime: true, MaxAllowedPacket: defaultMaxAllowedPacket, Logger: getLogger(), AllowNativePasswords: true, CheckConnLiveness: true, timeTruncate: time.Hour}, + }, + } } func TestDSNParser(t *testing.T) { diff --git a/errors.go b/errors.go index 584617b11..b4466cdaf 100644 --- a/errors.go +++ b/errors.go @@ -13,6 +13,7 @@ import ( "fmt" "log" "os" + "sync/atomic" ) // Various errors the driver might return. Can change between driver versions. @@ -37,7 +38,11 @@ var ( errBadConnNoWrite = errors.New("bad connection") ) -var defaultLogger = Logger(log.New(os.Stderr, "[mysql] ", log.Ldate|log.Ltime)) +var defaultLogger atomic.Pointer[Logger] + +func init() { + SetLogger(Logger(log.New(os.Stderr, "[mysql] ", log.Ldate|log.Ltime))) +} // Logger is used to log critical error messages. type Logger interface { @@ -56,10 +61,19 @@ func SetLogger(logger Logger) error { if logger == nil { return errors.New("logger is nil") } - defaultLogger = logger + defaultLogger.Store(&logger) return nil } +func getLogger() Logger { + l := defaultLogger.Load() + if l == nil { + return nil + } + + return *l +} + // MySQLError is an error type which represents a single MySQL error type MySQLError struct { Number uint16 diff --git a/errors_test.go b/errors_test.go index 53d634454..69ce5967d 100644 --- a/errors_test.go +++ b/errors_test.go @@ -16,9 +16,9 @@ import ( ) func TestErrorsSetLogger(t *testing.T) { - previous := defaultLogger + previous := getLogger() defer func() { - defaultLogger = previous + SetLogger(previous) }() // set up logger @@ -28,7 +28,7 @@ func TestErrorsSetLogger(t *testing.T) { // print SetLogger(logger) - defaultLogger.Print("test") + getLogger().Print("test") // check result if actual := buffer.String(); actual != expected {