database/sql: deflake TestPendingConnsAfterErr and fix races, panics

TestPendingConnsAfterErr only cared that things didn't deadlock, so 5
seconds is a sufficient timer. We don't need 100 milliseconds.

I was able to reproduce with a tiny (5 nanosecond) timeout value,
instead of 100 milliseconds. In the process of testing with -race and
a high -count= value, I noticed several data races and panics
(sendings on a closed channel) which are also fixed in this change.

Fixes #15684

Change-Id: Ib4605fcc0f296e658cb948352ed642b801cb578c
Reviewed-on: https://go-review.googlesource.com/24550
Reviewed-by: Marko Tiikkaja <marko@joh.to>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>

diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go
index e7482a8e2f2ac61ec7003d4a3e42b8b72d48a59c..09de1c34e826bdb7468dcd26c5d0393e3864f299 100644 (file)
--- a/src/database/sql/sql.go
+++ b/src/database/sql/sql.go
@@ -718,6 +718,9 @@ func (db *DB) maybeOpenNewConnections() {
        for numRequests > 0 {
                db.numOpen++ // optimistically
                numRequests--
+               if db.closed {
+                       return
+               }
                db.openerCh <- struct{}{}
        }
 }
@@ -915,6 +918,9 @@ func (db *DB) putConn(dc *driverConn, err error) {
 // If a connRequest was fulfilled or the *driverConn was placed in the
 // freeConn list, then true is returned, otherwise false is returned.
 func (db *DB) putConnDBLocked(dc *driverConn, err error) bool {
+       if db.closed {
+               return false
+       }
        if db.maxOpen > 0 && db.numOpen > db.maxOpen {
                return false
        }

diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go
index 6f11303c14232ca94d88747334bd613115df4c31..08df0c7666a357ae173a5d3b5918d3bcb1c9862a 100644 (file)
--- a/src/database/sql/sql_test.go
+++ b/src/database/sql/sql_test.go
@@ -144,7 +144,7 @@ func closeDB(t testing.TB, db *DB) {
        count := db.numOpen
        db.mu.Unlock()
        if count != 0 {
-               t.Fatalf("%d connections still open after closing DB", db.numOpen)
+               t.Fatalf("%d connections still open after closing DB", count)
        }
 }
 
@@ -1239,7 +1239,7 @@ func TestPendingConnsAfterErr(t *testing.T) {
        time.Sleep(10 * time.Millisecond) // make extra sure all workers are blocked
        close(unblock)                    // let all workers proceed
 
-       const timeout = 100 * time.Millisecond
+       const timeout = 5 * time.Second
        to := time.NewTimer(timeout)
        defer to.Stop()
 
@@ -1615,6 +1615,8 @@ func TestManyErrBadConn(t *testing.T) {
                        }
                }()
 
+               db.mu.Lock()
+               defer db.mu.Unlock()
                if db.numOpen != nconn {
                        t.Fatalf("unexpected numOpen %d (was expecting %d)", db.numOpen, nconn)
                } else if len(db.freeConn) != nconn {