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 <iant@golang.org>
This commit is contained in:
Daniel Theophanes 2017-09-23 15:30:46 -07:00
parent 350b74bc4b
commit 897080d5cb
3 changed files with 20 additions and 1 deletions

View file

@ -2491,6 +2491,12 @@ func (rs *Rows) nextLocked() (doClose, ok bool) {
if rs.lastcols == nil {
rs.lastcols = make([]driver.Value, len(rs.rowsi.Columns()))
}
// Lock the driver connection before calling the driver interface
// rowsi to prevent a Tx from rolling back the connection at the same time.
rs.dc.Lock()
defer rs.dc.Unlock()
rs.lasterr = rs.rowsi.Next(rs.lastcols)
if rs.lasterr != nil {
// Close the connection if there is a driver error.
@ -2540,6 +2546,12 @@ func (rs *Rows) NextResultSet() bool {
doClose = true
return false
}
// Lock the driver connection before calling the driver interface
// rowsi to prevent a Tx from rolling back the connection at the same time.
rs.dc.Lock()
defer rs.dc.Unlock()
rs.lasterr = nextResultSet.NextResultSet()
if rs.lasterr != nil {
doClose = true