1
0
mirror of https://github.com/golang/go synced 2024-11-19 03:14:42 -07:00

database: remove race in TestTxContextWait

This test contained a data race.
On line 437, db.BeginTx starts a goroutine that runs tx.awaitDone,
which reads tx.keepConnOnRollback.
On line 445, the test writes to tx.keepConnOnRollback.
tx.awaitDone waits on ctx, but because ctx is timeout-based,
there's no ordering guarantee between the write and the read.

The race detector never caught this before
because the context package implementation of Done
contained enough synchronization to make it safe.
That synchronization is not package of the context API or guarantees,
and the first several releases it was not present.
Another commit soon will remove that synchronization,
exposing the latent data race.

To fix the race, emulate a time-based context
using an explicit cancellation-based context.
This gives us enough control to avoid the race.

Change-Id: I103fe9b987b1d4c02e7a20ac3c22a682652128b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/288493
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
This commit is contained in:
Josh Bleecher Snyder 2021-02-01 08:35:36 -08:00
parent 26001d109e
commit e496120891

View File

@ -431,25 +431,24 @@ func TestTxContextWait(t *testing.T) {
db := newTestDB(t, "people")
defer closeDB(t, db)
ctx, cancel := context.WithTimeout(context.Background(), 15*time.Millisecond)
defer cancel()
ctx, cancel := context.WithCancel(context.Background())
tx, err := db.BeginTx(ctx, nil)
if err != nil {
// Guard against the context being canceled before BeginTx completes.
if err == context.DeadlineExceeded {
t.Skip("tx context canceled prior to first use")
}
t.Fatal(err)
}
tx.keepConnOnRollback = false
go func() {
time.Sleep(15 * time.Millisecond)
cancel()
}()
// This will trigger the *fakeConn.Prepare method which will take time
// performing the query. The ctxDriverPrepare func will check the context
// after this and close the rows and return an error.
_, err = tx.QueryContext(ctx, "WAIT|1s|SELECT|people|age,name|")
if err != context.DeadlineExceeded {
t.Fatalf("expected QueryContext to error with context deadline exceeded but returned %v", err)
if err != context.Canceled {
t.Fatalf("expected QueryContext to error with context canceled but returned %v", err)
}
waitForFree(t, db, 5*time.Second, 0)