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.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'src/database/sql/sql.go') diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go index 17910904f6..9a3957b267 100644 --- a/src/database/sql/sql.go +++ b/src/database/sql/sql.go @@ -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 -- cgit v1.3