database/sql: ensure all driver interfaces are called under single lock

Russ pointed out in a previous CL golang.org/cl/65731 that not only
was the locking incomplete, previous changes did not correctly
lock driver calls in other sections. After inspecting
driverConn, driverStmt, driverResult, Tx, and Rows structs
where driver interfaces are stored, I discovered a few more places
that failed to lock driver calls. The largest of these
was the parameter type converter "driverArgs".

driverArgs was typically called right before another call to the
driver in a locked region, so I made the entire driverArgs expect
a locked driver mutex and combined the region. This should not
be a problem because the connection is pulled out of the connection
pool either way so there shouldn't be contention.

Fixes #21117

Change-Id: I88d46f74dca25fb11a30f0bf8e79785a73133d23
Reviewed-on: https://go-review.googlesource.com/71433
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

1 files changed, 2 insertions, 13 deletions

diff --git a/src/database/sql/convert.go b/src/database/sql/convert.go
index c349a96edf..b44bed559d 100644
--- a/src/database/sql/convert.go
+++ b/src/database/sql/convert.go
@@ -12,7 +12,6 @@ import (
 	"fmt"
 	"reflect"
 	"strconv"
-	"sync"
 	"time"
 	"unicode"
 	"unicode/utf8"
@@ -38,17 +37,10 @@ func validateNamedValueName(name string) error {
 	return fmt.Errorf("name %q does not begin with a letter", name)
 }
 
-func driverNumInput(ds *driverStmt) int {
-	ds.Lock()
-	defer ds.Unlock() // in case NumInput panics
-	return ds.si.NumInput()
-}
-
 // ccChecker wraps the driver.ColumnConverter and allows it to be used
 // as if it were a NamedValueChecker. If the driver ColumnConverter
 // is not present then the NamedValueChecker will return driver.ErrSkip.
 type ccChecker struct {
-	sync.Locker
 	cci  driver.ColumnConverter
 	want int
 }
@@ -88,9 +80,7 @@ func (c ccChecker) CheckNamedValue(nv *driver.NamedValue) error {
 	// same error.
 	var err error
 	arg := nv.Value
-	c.Lock()
 	nv.Value, err = c.cci.ColumnConverter(index).ConvertValue(arg)
-	c.Unlock()
 	if err != nil {
 		return err
 	}
@@ -112,7 +102,7 @@ func defaultCheckNamedValue(nv *driver.NamedValue) (err error) {
 // Stmt.Query into driver Values.
 //
 // The statement ds may be nil, if no statement is available.
-func driverArgs(ci driver.Conn, ds *driverStmt, args []interface{}) ([]driver.NamedValue, error) {
+func driverArgsConnLocked(ci driver.Conn, ds *driverStmt, args []interface{}) ([]driver.NamedValue, error) {
 	nvargs := make([]driver.NamedValue, len(args))
 
 	// -1 means the driver doesn't know how to count the number of
@@ -124,8 +114,7 @@ func driverArgs(ci driver.Conn, ds *driverStmt, args []interface{}) ([]driver.Na
 	var cc ccChecker
 	if ds != nil {
 		si = ds.si
-		want = driverNumInput(ds)
-		cc.Locker = ds.Locker
+		want = ds.si.NumInput()
 		cc.want = want
 	}