database/sql: avoid deadlock waiting for connections

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>

1 files changed, 20 insertions, 0 deletions

diff --git a/src/database/sql/fakedb_test.go b/src/database/sql/fakedb_test.go
index 112f280ec5..f1e8f6cb6e 100644
--- a/src/database/sql/fakedb_test.go
+++ b/src/database/sql/fakedb_test.go
@@ -153,12 +153,32 @@ func TestDrivers(t *testing.T) {
 	}
 }
 
+// hook to simulate connection failures
+var hookOpenErr struct {
+	sync.Mutex
+	fn func() error
+}
+
+func setHookOpenErr(fn func() error) {
+	hookOpenErr.Lock()
+	defer hookOpenErr.Unlock()
+	hookOpenErr.fn = fn
+}
+
 // Supports dsn forms:
 //    <dbname>
 //    <dbname>;<opts>  (only currently supported option is `badConn`,
 //                      which causes driver.ErrBadConn to be returned on
 //                      every other conn.Begin())
 func (d *fakeDriver) Open(dsn string) (driver.Conn, error) {
+	hookOpenErr.Lock()
+	fn := hookOpenErr.fn
+	hookOpenErr.Unlock()
+	if fn != nil {
+		if err := fn(); err != nil {
+			return nil, err
+		}
+	}
 	parts := strings.Split(dsn, ";")
 	if len(parts) < 1 {
 		return nil, errors.New("fakedb: no database name")