Skip to content

Commit

Permalink
Fix test race: Atomically access minConnecTimout in testing environme…
Browse files Browse the repository at this point in the history
…nt. (grpc#1897)
  • Loading branch information
MakMukhi authored Mar 7, 2018
1 parent 3ae2a61 commit 207e276
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 6 deletions.
14 changes: 11 additions & 3 deletions clientconn.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ import (
"google.golang.org/grpc/transport"
)

const (
// minimum time to give a connection to complete
minConnectTimeout = 20 * time.Second
)

var (
// ErrClientConnClosing indicates that the operation is illegal because
// the ClientConn is closing.
Expand All @@ -60,8 +65,11 @@ var (
errConnUnavailable = errors.New("grpc: the connection is unavailable")
// errBalancerClosed indicates that the balancer is closed.
errBalancerClosed = errors.New("grpc: balancer is closed")
// minimum time to give a connection to complete
minConnectTimeout = 20 * time.Second
// We use an accessor so that minConnectTimeout can be
// atomically read and updated while testing.
getMinConnectTimeout = func() time.Duration {
return minConnectTimeout
}
)

// The following errors are returned from Dial and DialContext
Expand Down Expand Up @@ -1055,7 +1063,7 @@ func (ac *addrConn) resetTransport() error {
// connection.
backoffFor := ac.dopts.bs.backoff(connectRetryNum) // time.Duration.
// This will be the duration that dial gets to finish.
dialDuration := minConnectTimeout
dialDuration := getMinConnectTimeout()
if backoffFor > dialDuration {
// Give dial more time as we keep failing to connect.
dialDuration = backoffFor
Expand Down
17 changes: 14 additions & 3 deletions clientconn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,16 @@ import (
"google.golang.org/grpc/testdata"
)

var (
mutableMinConnectTimeout = time.Second * 20
)

func init() {
getMinConnectTimeout = func() time.Duration {
return time.Duration(atomic.LoadInt64((*int64)(&mutableMinConnectTimeout)))
}
}

func assertState(wantState connectivity.State, cc *ClientConn) (connectivity.State, bool) {
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
defer cancel()
Expand Down Expand Up @@ -169,13 +179,14 @@ func TestDialWaitsForServerSettings(t *testing.T) {
}

func TestCloseConnectionWhenServerPrefaceNotReceived(t *testing.T) {
mctBkp := minConnectTimeout
mctBkp := getMinConnectTimeout()
// Call this only after transportMonitor goroutine has ended.
defer func() {
minConnectTimeout = mctBkp
atomic.StoreInt64((*int64)(&mutableMinConnectTimeout), int64(mctBkp))

}()
defer leakcheck.Check(t)
minConnectTimeout = time.Millisecond * 500
atomic.StoreInt64((*int64)(&mutableMinConnectTimeout), int64(time.Millisecond)*500)
lis, err := net.Listen("tcp", "localhost:0")
if err != nil {
t.Fatalf("Error while listening. Err: %v", err)
Expand Down

0 comments on commit 207e276

Please sign in to comment.