Stowage/go
2
0
Fork 0
mirror of https://github.com/golang/go.git synced 2025-12-08 06:10:04 +00:00
Code Issues Projects Releases Packages Wiki Activity

database/sql: avoid deadlock waiting for connections

Browse source 
Previously with db.maxOpen > 0, db.maxOpen+n failed connection attempts
started concurrently could result in a deadlock. DB.conn and
DB.openNewConnection did not trigger the DB.connectionOpener go routine
after a failed connection attempt. This omission could leave go routines
waiting for DB.connectionOpener forever.

In addition the logic to track the state of the pool was inconsistent.
db.numOpen was sometimes incremented optimistically and sometimes not.
This change harmonizes the logic and eliminates the db.pendingOpens
variable, making the logic easier to understand and maintain.

Fixes #10886

Change-Id: I983c4921a3dacfbd531c3d7f8d2da8a592e9922a
Reviewed-on: https://go-review.googlesource.com/14547
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This commit is contained in:
Chris Hines 2015-09-14 03:44:56 -04:00 • committed by Brad Fitzpatrick
parent 19aa4209ae
commit 6de40099c8
3 changed files with 97 additions and 8 deletions
Download patch file Download diff file Expand all files Collapse all files

24
src/database/sql/sql.go
View file

@ -229,8 +229,7 @@ type DB struct {
mu sync.Mutex // protects following fields
freeConn []*driverConn
connRequests []chan connRequest
numOpen int
pendingOpens int
numOpen int // number of opened and pending open connections
// Used to signal the need for new connections
// a goroutine running connectionOpener() reads on this chan and
// maybeOpenNewConnections sends on the chan (one send per needed connection)
@ -615,15 +614,15 @@ func (db *DB) Stats() DBStats {
// If there are connRequests and the connection limit hasn't been reached,
// then tell the connectionOpener to open new connections.
func (db *DB) maybeOpenNewConnections() {
numRequests := len(db.connRequests) - db.pendingOpens
numRequests := len(db.connRequests)
if db.maxOpen > 0 {
numCanOpen := db.maxOpen - (db.numOpen + db.pendingOpens)
numCanOpen := db.maxOpen - db.numOpen
if numRequests > numCanOpen {
numRequests = numCanOpen
}
}
for numRequests > 0 {
db.pendingOpens++
db.numOpen++ // optimistically
numRequests--
db.openerCh <- struct{}{}
}
@ -638,6 +637,9 @@ func (db *DB) connectionOpener() {
// Open one new connection
func (db *DB) openNewConnection() {
// maybeOpenNewConnctions has already executed db.numOpen++ before it sent
// on db.openerCh. This function must execute db.numOpen-- if the
// connection fails or is closed before returning.
ci, err := db.driver.Open(db.dsn)
db.mu.Lock()
defer db.mu.Unlock()
@ -645,11 +647,13 @@ func (db *DB) openNewConnection() {
if err == nil {
ci.Close()
}
db.numOpen--
return
}
db.pendingOpens--
if err != nil {
db.numOpen--
db.putConnDBLocked(nil, err)
db.maybeOpenNewConnections()
return
}
dc := &driverConn{
@ -658,8 +662,8 @@ func (db *DB) openNewConnection() {
}
if db.putConnDBLocked(dc, err) {
db.addDepLocked(dc, dc)
db.numOpen++
} else {
db.numOpen--
ci.Close()
}
}
@ -701,7 +705,10 @@ func (db *DB) conn(strategy connReuseStrategy) (*driverConn, error) {
req := make(chan connRequest, 1)
db.connRequests = append(db.connRequests, req)
db.mu.Unlock()
ret := <-req
ret, ok := <-req
if !ok {
return nil, errDBClosed
}
return ret.conn, ret.err
}
@ -711,6 +718,7 @@ func (db *DB) conn(strategy connReuseStrategy) (*driverConn, error) {
if err != nil {
db.mu.Lock()
db.numOpen-- // correct for earlier optimism
db.maybeOpenNewConnections()
db.mu.Unlock()
return nil, err
}