From 00727feed6efa6f5cbce471f7ed3b2c174d817e0 Mon Sep 17 00:00:00 2001 From: ICHINOSE Shogo Date: Fri, 22 Mar 2024 23:18:57 +0900 Subject: [PATCH 1/4] Add test for Issue1567 --- driver_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/driver_test.go b/driver_test.go index 6b52650c2..11040d072 100644 --- a/driver_test.go +++ b/driver_test.go @@ -20,6 +20,7 @@ import ( "io" "log" "math" + mrand "math/rand" "net" "net/url" "os" @@ -3577,3 +3578,31 @@ func runCallCommand(dbt *DBTest, query, name string) { } } } + +func TestIssue1567(t *testing.T) { + // enable TLS. + runTests(t, dsn+"&tls=skip-verify", func(dbt *DBTest) { + // disable connection pooling. + // data race happens when new connection is created. + dbt.db.SetMaxIdleConns(0) + + // estimate round trip time. + start := time.Now() + if err := dbt.db.PingContext(context.Background()); err != nil { + t.Fatal(err) + } + rtt := time.Since(start) + + count := 1000 + if testing.Short() { + count = 10 + } + + for i := 0; i < count; i++ { + timeout := time.Duration(mrand.Int63n(int64(rtt))) + ctx, cancel := context.WithTimeout(context.Background(), timeout) + dbt.db.PingContext(ctx) + cancel() + } + }) +} From 93f1a648e893b1611e52aa7c41bb2c16bc8a13e2 Mon Sep 17 00:00:00 2001 From: ICHINOSE Shogo Date: Fri, 22 Mar 2024 23:29:48 +0900 Subject: [PATCH 2/4] fix Data race between mysqlConn.cleanup() and writeHandshakeResponsePacket close #1567 --- connection.go | 6 +----- connector.go | 2 +- packets.go | 1 - 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/connection.go b/connection.go index f3656f0e6..722352a7e 100644 --- a/connection.go +++ b/connection.go @@ -153,11 +153,7 @@ func (mc *mysqlConn) cleanup() { // Makes cleanup idempotent close(mc.closech) - nc := mc.netConn - if nc == nil { - return - } - if err := nc.Close(); err != nil { + if err := mc.rawConn.Close(); err != nil { mc.log(err) } // This function can be called from multiple goroutines. diff --git a/connector.go b/connector.go index a0ee62839..b67077596 100644 --- a/connector.go +++ b/connector.go @@ -102,10 +102,10 @@ func (c *connector) Connect(ctx context.Context) (driver.Conn, error) { nd := net.Dialer{Timeout: mc.cfg.Timeout} mc.netConn, err = nd.DialContext(ctx, mc.cfg.Net, mc.cfg.Addr) } - if err != nil { return nil, err } + mc.rawConn = mc.netConn // Enable TCP Keepalives on TCP connections if tc, ok := mc.netConn.(*net.TCPConn); ok { diff --git a/packets.go b/packets.go index d727f00fe..90a34728b 100644 --- a/packets.go +++ b/packets.go @@ -351,7 +351,6 @@ func (mc *mysqlConn) writeHandshakeResponsePacket(authResp []byte, plugin string if err := tlsConn.Handshake(); err != nil { return err } - mc.rawConn = mc.netConn mc.netConn = tlsConn mc.buf.nc = tlsConn } From d05ec232fe25697cf8915bc479ceccd9b7f9204b Mon Sep 17 00:00:00 2001 From: ICHINOSE Shogo Date: Fri, 22 Mar 2024 23:33:26 +0900 Subject: [PATCH 3/4] Fix connection cleanup to handle nil rawConn --- connection.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/connection.go b/connection.go index 722352a7e..7b8abeb00 100644 --- a/connection.go +++ b/connection.go @@ -153,7 +153,11 @@ func (mc *mysqlConn) cleanup() { // Makes cleanup idempotent close(mc.closech) - if err := mc.rawConn.Close(); err != nil { + conn := mc.rawConn + if conn == nil { + return + } + if err := conn.Close(); err != nil { mc.log(err) } // This function can be called from multiple goroutines. From 73abfe6778385648a635b07dd93dd6703219d787 Mon Sep 17 00:00:00 2001 From: ICHINOSE Shogo Date: Fri, 22 Mar 2024 23:41:17 +0900 Subject: [PATCH 4/4] In some environments, rtt may become 0, so set it to at least 1ms. --- driver_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/driver_test.go b/driver_test.go index 11040d072..4fd196d4b 100644 --- a/driver_test.go +++ b/driver_test.go @@ -3592,6 +3592,10 @@ func TestIssue1567(t *testing.T) { t.Fatal(err) } rtt := time.Since(start) + if rtt <= 0 { + // In some environments, rtt may become 0, so set it to at least 1ms. + rtt = time.Millisecond + } count := 1000 if testing.Short() {