diff --git a/src/pkg/database/sql/fakedb_test.go b/src/pkg/database/sql/fakedb_test.go index 39c02827897..2ed1364759b 100644 --- a/src/pkg/database/sql/fakedb_test.go +++ b/src/pkg/database/sql/fakedb_test.go @@ -38,6 +38,8 @@ type fakeDriver struct { mu sync.Mutex // guards 3 following fields openCount int // conn opens closeCount int // conn closes + waitCh chan struct{} + waitingCh chan struct{} dbs map[string]*fakeDB } @@ -146,6 +148,10 @@ func (d *fakeDriver) Open(dsn string) (driver.Conn, error) { if len(parts) >= 2 && parts[1] == "badConn" { conn.bad = true } + if d.waitCh != nil { + d.waitingCh <- struct{}{} + <-d.waitCh + } return conn, nil } diff --git a/src/pkg/database/sql/sql.go b/src/pkg/database/sql/sql.go index fe46ff3781f..3047735acc4 100644 --- a/src/pkg/database/sql/sql.go +++ b/src/pkg/database/sql/sql.go @@ -593,9 +593,12 @@ func (db *DB) openNewConnection() { db: db, ci: ci, } - db.addDepLocked(dc, dc) - db.numOpen++ - db.putConnDBLocked(dc, err) + if db.putConnDBLocked(dc, err) { + db.addDepLocked(dc, dc) + db.numOpen++ + } else { + ci.Close() + } } // connRequest represents one request for a new connection diff --git a/src/pkg/database/sql/sql_test.go b/src/pkg/database/sql/sql_test.go index 32605ce7613..093c0d64cac 100644 --- a/src/pkg/database/sql/sql_test.go +++ b/src/pkg/database/sql/sql_test.go @@ -1677,6 +1677,57 @@ func TestConcurrency(t *testing.T) { doConcurrentTest(t, new(concurrentRandomTest)) } +func TestConnectionLeak(t *testing.T) { + db := newTestDB(t, "people") + defer closeDB(t, db) + // Start by opening defaultMaxIdleConns + rows := make([]*Rows, defaultMaxIdleConns) + // We need to SetMaxOpenConns > MaxIdleConns, so the DB can open + // a new connection and we can fill the idle queue with the released + // connections. + db.SetMaxOpenConns(len(rows) + 1) + for ii := range rows { + r, err := db.Query("SELECT|people|name|") + if err != nil { + t.Fatal(err) + } + r.Next() + if err := r.Err(); err != nil { + t.Fatal(err) + } + rows[ii] = r + } + // Now we have defaultMaxIdleConns busy connections. Open + // a new one, but wait until the busy connections are released + // before returning control to DB. + drv := db.driver.(*fakeDriver) + drv.waitCh = make(chan struct{}, 1) + drv.waitingCh = make(chan struct{}, 1) + var wg sync.WaitGroup + wg.Add(1) + go func() { + r, err := db.Query("SELECT|people|name|") + if err != nil { + t.Fatal(err) + } + r.Close() + wg.Done() + }() + // Wait until the goroutine we've just created has started waiting. + <-drv.waitingCh + // Now close the busy connections. This provides a connection for + // the blocked goroutine and then fills up the idle queue. + for _, v := range rows { + v.Close() + } + // At this point we give the new connection to DB. This connection is + // now useless, since the idle queue is full and there are no pending + // requests. DB should deal with this situation without leaking the + // connection. + drv.waitCh <- struct{}{} + wg.Wait() +} + func BenchmarkConcurrentDBExec(b *testing.B) { b.ReportAllocs() ct := new(concurrentDBExecTest)