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>

diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go
index c968852adeefeb321b14bc2993a459a82a8ff760..99bfd62491fa3beb3d4721b024ba93dc5b5181df 100644 (file)
--- a/src/database/sql/sql_test.go
+++ b/src/database/sql/sql_test.go
@@ -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)