Merge pull request #133 from o1egl/fix_race

Fix data race in  query factory initialisation

database.go

@@ -3,6 +3,7 @@ package goqu
 import (
 	"context"
 	"database/sql"
+	"sync"
 
 	"github.com/doug-martin/goqu/v8/exec"
 )
@@ -28,6 +29,7 @@ type (
 		dialect string
 		Db      SQLDatabase
 		qf      exec.QueryFactory
+		qfOnce  sync.Once
 	}
 )
 
@@ -324,9 +326,9 @@ func (d *Database) QueryRowContext(ctx context.Context, query string, args ...in
 }
 
 func (d *Database) queryFactory() exec.QueryFactory {
-	if d.qf == nil {
+	d.qfOnce.Do(func() {
 		d.qf = exec.NewQueryFactory(d)
-	}
+	})
 	return d.qf
 }
 
@@ -443,6 +445,7 @@ type (
 		dialect string
 		Tx      SQLTx
 		qf      exec.QueryFactory
+		qfOnce  sync.Once
 	}
 )
 
@@ -545,9 +548,9 @@ func (td *TxDatabase) QueryRowContext(ctx context.Context, query string, args ..
 }
 
 func (td *TxDatabase) queryFactory() exec.QueryFactory {
-	if td.qf == nil {
+	td.qfOnce.Do(func() {
 		td.qf = exec.NewQueryFactory(td)
-	}
+	})
 	return td.qf
 }
 

database_test.go

@@ -3,8 +3,11 @@ package goqu
 import (
 	"context"
 	"fmt"
+	"sync"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+
 	"github.com/DATA-DOG/go-sqlmock"
 	"github.com/doug-martin/goqu/v8/internal/errors"
 	"github.com/stretchr/testify/suite"
@@ -313,6 +316,40 @@ func (ds *databaseSuite) TestWithTx() {
 	}
 }
 
+func (ds *databaseSuite) TestDataRace() {
+	t := ds.T()
+	mDb, mock, err := sqlmock.New()
+	assert.NoError(t, err)
+	db := newDatabase("mock", mDb)
+
+	const concurrency = 10
+
+	for i := 0; i < concurrency; i++ {
+		mock.ExpectQuery(`SELECT "address", "name" FROM "items"`).
+			WithArgs().
+			WillReturnRows(sqlmock.NewRows([]string{"address", "name"}).
+				FromCSVString("111 Test Addr,Test1\n211 Test Addr,Test2"))
+	}
+
+	wg := sync.WaitGroup{}
+	for i := 0; i < concurrency; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+
+			sql := db.From("items").Limit(1)
+			var item testActionItem
+			found, err := sql.ScanStruct(&item)
+			assert.NoError(t, err)
+			assert.True(t, found)
+			assert.Equal(t, item.Address, "111 Test Addr")
+			assert.Equal(t, item.Name, "Test1")
+		}()
+	}
+
+	wg.Wait()
+}
+
 func TestDatabaseSuite(t *testing.T) {
 	suite.Run(t, new(databaseSuite))
 }
@@ -623,6 +660,45 @@ func (tds *txdatabaseSuite) TestWrap() {
 	}), "goqu: tx error")
 }
 
+func (tds *txdatabaseSuite) TestDataRace() {
+	t := tds.T()
+	mDb, mock, err := sqlmock.New()
+	assert.NoError(t, err)
+	mock.ExpectBegin()
+	db := newDatabase("mock", mDb)
+	tx, err := db.Begin()
+	assert.NoError(t, err)
+
+	const concurrency = 10
+
+	for i := 0; i < concurrency; i++ {
+		mock.ExpectQuery(`SELECT "address", "name" FROM "items"`).
+			WithArgs().
+			WillReturnRows(sqlmock.NewRows([]string{"address", "name"}).
+				FromCSVString("111 Test Addr,Test1\n211 Test Addr,Test2"))
+	}
+
+	wg := sync.WaitGroup{}
+	for i := 0; i < concurrency; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+
+			sql := tx.From("items").Limit(1)
+			var item testActionItem
+			found, err := sql.ScanStruct(&item)
+			assert.NoError(t, err)
+			assert.True(t, found)
+			assert.Equal(t, item.Address, "111 Test Addr")
+			assert.Equal(t, item.Name, "Test1")
+		}()
+	}
+
+	wg.Wait()
+	mock.ExpectCommit()
+	assert.NoError(t, tx.Commit())
+}
+
 func TestTxDatabaseSuite(t *testing.T) {
 	suite.Run(t, new(txdatabaseSuite))
 }

go.test.sh

@@ -3,4 +3,4 @@
 set -e
 echo "" > coverage.txt
 
-go test -coverprofile=coverage.txt -coverpkg=./... ./...
+go test -race -coverprofile=coverage.txt -coverpkg=./... ./...