From 897080d5cbb1793f8ad3ef5fb7c6fafba2e97d42 Mon Sep 17 00:00:00 2001 From: Daniel Theophanes Date: Sat, 23 Sep 2017 15:30:46 -0700 Subject: database/sql: prevent race in driver by locking dc in Next Database drivers should be called from a single goroutine to ease driver's design. If a driver chooses to handle context cancels internally it may do so. The sql package violated this agreement when calling Next or NextResultSet. It was possible for a concurrent rollback triggered from a context cancel to call a Tx.Rollback (which takes a driver connection lock) while a Rows.Next is in progress (which does not tack the driver connection lock). The current internal design of the sql package is each call takes roughly two locks: a closemu lock which prevents an disposing of internal resources (assigning nil or removing from lists) and a driver connection lock that prevents calling driver code from multiple goroutines. Fixes #21117 Change-Id: Ie340dc752a503089c27f57ffd43e191534829360 Reviewed-on: https://go-review.googlesource.com/65731 Reviewed-by: Ian Lance Taylor --- src/database/sql/sql_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'src/database/sql/sql_test.go') diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go index 046d95aff4..760159a9ac 100644 --- a/src/database/sql/sql_test.go +++ b/src/database/sql/sql_test.go @@ -3127,6 +3127,9 @@ func TestIssue6081(t *testing.T) { // In the test, a context is canceled while the query is in process so // the internal rollback will run concurrently with the explicitly called // Tx.Rollback. +// +// The addition of calling rows.Next also tests +// Issue 21117. func TestIssue18429(t *testing.T) { db := newTestDB(t, "people") defer closeDB(t, db) @@ -3137,7 +3140,7 @@ func TestIssue18429(t *testing.T) { const milliWait = 30 - for i := 0; i < 100; i++ { + for i := 0; i < 1000; i++ { sem <- true wg.Add(1) go func() { @@ -3159,6 +3162,9 @@ func TestIssue18429(t *testing.T) { // reported. rows, _ := tx.QueryContext(ctx, "WAIT|"+qwait+"|SELECT|people|name|") if rows != nil { + // Call Next to test Issue 21117 and check for races. + for rows.Next() { + } rows.Close() } // This call will race with the context cancel rollback to complete -- cgit v1.3