Skip to content

Commit fb0dc84

Browse files
committed
Fix the TestConcurrent test to pass Go's race detection.
Running the race detection against the previous version of this test reveals race conditions surrounding the "max" variable. Subsequent work in this patch also revealed race conditions surrounding canStop. Additionally, it is not valid to call *Testing.FailNow in goroutines started by the tests. This should retain the meaning of the original test while cleaning up the race conditions and guaranteeing the *Testing.Fatal call occurs in the correct goroutine.
1 parent 12cf283 commit fb0dc84

File tree

2 files changed

+51
-15
lines changed

2 files changed

+51
-15
lines changed

AUTHORS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,5 @@ Xiuming Chen <cc at cxm.cc>
2727

2828
# Organizations
2929

30+
Barracuda Networks, Inc.
3031
Google Inc.

driver_test.go

Lines changed: 50 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import (
1919
"net/url"
2020
"os"
2121
"strings"
22+
"sync"
23+
"sync/atomic"
2224
"testing"
2325
"time"
2426
)
@@ -1220,40 +1222,73 @@ func TestConcurrent(t *testing.T) {
12201222
dbt.Fatalf("%s", err.Error())
12211223
}
12221224
dbt.Logf("Testing up to %d concurrent connections \r\n", max)
1223-
canStop := false
1224-
c := make(chan struct{}, max)
1225+
1226+
canStopVal := false
1227+
var canStopM sync.Mutex
1228+
canStop := func() bool {
1229+
canStopM.Lock()
1230+
defer canStopM.Unlock()
1231+
return canStopVal
1232+
}
1233+
setCanStop := func() {
1234+
canStopM.Lock()
1235+
defer canStopM.Unlock()
1236+
canStopVal = true
1237+
}
1238+
1239+
var succeeded int32
1240+
1241+
var wg sync.WaitGroup
1242+
wg.Add(max)
1243+
1244+
var fatalError string
1245+
var once sync.Once
1246+
fatal := func(s string, vals ...interface{}) {
1247+
once.Do(func() {
1248+
fatalError = fmt.Sprintf(s, vals...)
1249+
})
1250+
}
1251+
12251252
for i := 0; i < max; i++ {
12261253
go func(id int) {
1254+
defer wg.Done()
1255+
12271256
tx, err := dbt.db.Begin()
12281257
if err != nil {
1229-
canStop = true
1258+
setCanStop()
12301259
if err.Error() == "Error 1040: Too many connections" {
1231-
max--
12321260
return
12331261
} else {
1234-
dbt.Fatalf("Error on Con %d: %s", id, err.Error())
1262+
fatal("Error on Con %d: %s", id, err.Error())
12351263
}
12361264
}
1237-
c <- struct{}{}
1238-
for !canStop {
1265+
1266+
atomic.AddInt32(&succeeded, 1)
1267+
1268+
var hasSelected bool
1269+
for !canStop() || !hasSelected {
12391270
_, err = tx.Exec("SELECT 1")
12401271
if err != nil {
1241-
canStop = true
1242-
dbt.Fatalf("Error on Con %d: %s", id, err.Error())
1272+
setCanStop()
1273+
fatal("Error on Con %d: %s", id, err.Error())
12431274
}
1275+
hasSelected = true
12441276
}
12451277
err = tx.Commit()
12461278
if err != nil {
1247-
canStop = true
1248-
dbt.Fatalf("Error on Con %d: %s", id, err.Error())
1279+
fatal("Error on Con %d: %s", id, err.Error())
12491280
}
12501281
}(i)
12511282
}
1252-
for i := 0; i < max; i++ {
1253-
<-c
1283+
1284+
setCanStop()
1285+
1286+
wg.Wait()
1287+
1288+
if fatalError != "" {
1289+
dbt.Fatal(fatalError)
12541290
}
1255-
canStop = true
12561291

1257-
dbt.Logf("Reached %d concurrent connections \r\n", max)
1292+
dbt.Logf("Reached %d concurrent connections \r\n", succeeded)
12581293
})
12591294
}

0 commit comments

Comments
 (0)