From a68115fc039d51cdbc35e891710fb86532f845b8 Mon Sep 17 00:00:00 2001 From: Brian Dunn and Jack Christensen Date: Tue, 16 Sep 2014 16:29:45 -0500 Subject: [PATCH] Fix data race with Rows and ConnPool In an effort to reduce memory allocations, Rows was stored on the Conn. This caused a race condition where Rows are closed and this returns the Conn to the Pool. The Pool could then give out the Conn again. Rows would then be reanimated and the original Rows could reclose it. --- conn.go | 1 - conn_pool_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ query.go | 3 +-- 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/conn.go b/conn.go index 6a9c0a01..0ca6e7ce 100644 --- a/conn.go +++ b/conn.go @@ -53,7 +53,6 @@ type Conn struct { alive bool causeOfDeath error logger Logger - rows Rows mr msgReader } diff --git a/conn_pool_test.go b/conn_pool_test.go index a5362e5c..80507af6 100644 --- a/conn_pool_test.go +++ b/conn_pool_test.go @@ -410,6 +410,50 @@ func TestConnPoolQuery(t *testing.T) { } } +func TestConnPoolQueryConcurrentLoad(t *testing.T) { + t.Parallel() + + pool := createConnPool(t, 10) + defer pool.Close() + + for i := 0; i < 100; i++ { + go func(i int) { + var rowCount int32 + + rows, err := pool.Query("select generate_series(1,$1)", 1000) + if err != nil { + fmt.Println(i, err) + t.Fatalf("pool.Query failed: %v", err) + } + defer rows.Close() + + for rows.Next() { + var n int32 + err = rows.Scan(&n) + if err != nil { + fmt.Println(i, err) + t.Fatalf("rows.Scan failed: %v", err) + } + if n != rowCount+1 { + fmt.Println(i, err) + t.Fatalf("Expected n to be %d, but it was %d", rowCount+1, n) + } + rowCount++ + } + + if rows.Err() != nil { + fmt.Println(i, err) + t.Fatalf("conn.Query failed: ", err) + } + + if rowCount != 1000 { + fmt.Println(i, err) + t.Error("Select called onDataRow wrong number of times") + } + }(i) + } +} + func TestConnPoolQueryRow(t *testing.T) { t.Parallel() diff --git a/query.go b/query.go index 1c2cc82b..d6ecbdba 100644 --- a/query.go +++ b/query.go @@ -354,8 +354,7 @@ func (rows *Rows) Values() ([]interface{}, error) { // be returned in an error state. So it is allowed to ignore the error returned // from Query and handle it in *Rows. func (c *Conn) Query(sql string, args ...interface{}) (*Rows, error) { - c.rows = Rows{conn: c, startTime: time.Now(), sql: sql, args: args, logger: c.logger} - rows := &c.rows + rows := &Rows{conn: c, startTime: time.Now(), sql: sql, args: args, logger: c.logger} ps, ok := c.preparedStatements[sql] if !ok {