From 6de40099c88048c95df40c873e89b0e31f70ac24 Mon Sep 17 00:00:00 2001 From: Chris Hines Date: Mon, 14 Sep 2015 03:44:56 -0400 Subject: 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 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/database/sql/sql_test.go | 61 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) (limited to 'src/database/sql/sql_test.go') diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go index e1063bbc6b..d835bc160a 100644 --- a/src/database/sql/sql_test.go +++ b/src/database/sql/sql_test.go @@ -1159,6 +1159,67 @@ func TestMaxOpenConnsOnBusy(t *testing.T) { } } +// Issue 10886: tests that all connection attempts return when more than +// DB.maxOpen connections are in flight and the first DB.maxOpen fail. +func TestPendingConnsAfterErr(t *testing.T) { + const ( + maxOpen = 2 + tryOpen = maxOpen*2 + 2 + ) + + db := newTestDB(t, "people") + defer closeDB(t, db) + defer func() { + for k, v := range db.lastPut { + t.Logf("%p: %v", k, v) + } + }() + + db.SetMaxOpenConns(maxOpen) + db.SetMaxIdleConns(0) + + errOffline := errors.New("db offline") + defer func() { setHookOpenErr(nil) }() + + errs := make(chan error, tryOpen) + + unblock := make(chan struct{}) + setHookOpenErr(func() error { + <-unblock // block until all connections are in flight + return errOffline + }) + + var opening sync.WaitGroup + opening.Add(tryOpen) + for i := 0; i < tryOpen; i++ { + go func() { + opening.Done() // signal one connection is in flight + _, err := db.Exec("INSERT|people|name=Julia,age=19") + errs <- err + }() + } + + opening.Wait() // wait for all workers to begin running + time.Sleep(10 * time.Millisecond) // make extra sure all workers are blocked + close(unblock) // let all workers proceed + + const timeout = 100 * time.Millisecond + to := time.NewTimer(timeout) + defer to.Stop() + + // check that all connections fail without deadlock + for i := 0; i < tryOpen; i++ { + select { + case err := <-errs: + if got, want := err, errOffline; got != want { + t.Errorf("unexpected err: got %v, want %v", got, want) + } + case <-to.C: + t.Fatalf("orphaned connection request(s), still waiting after %v", timeout) + } + } +} + func TestSingleOpenConn(t *testing.T) { db := newTestDB(t, "people") defer closeDB(t, db) -- cgit v1.3