From 8a5961dd6a6a518aabbdce8eff8e595abebb5c26 Mon Sep 17 00:00:00 2001 From: Matthew Sykes Date: Thu, 20 Sep 2018 21:16:43 -0400 Subject: [PATCH] [FAB-12084] address simple lostcancel vet issues Also enable lostcancel checks on vet for all but the two remaining issues. Change-Id: I9cd94c28638e7c7db826ac0aee6e13953bdd8be3 Signed-off-by: Matthew Sykes --- common/crypto/tlsgen/ca_test.go | 4 +-- core/chaincode/accesscontrol/access_test.go | 4 +-- core/comm/connection.go | 10 +++++--- core/comm/connection_test.go | 4 +-- core/comm/server_test.go | 28 ++++++++------------- core/deliverservice/client.go | 1 + core/deliverservice/deliveryclient.go | 4 +-- core/peer/pkg_test.go | 10 +++----- gossip/comm/comm_test.go | 8 +++--- gossip/comm/crypto_test.go | 4 +-- scripts/golinter.sh | 10 ++++++++ 11 files changed, 44 insertions(+), 43 deletions(-) diff --git a/common/crypto/tlsgen/ca_test.go b/common/crypto/tlsgen/ca_test.go index db16ce786fe..a65478faac8 100644 --- a/common/crypto/tlsgen/ca_test.go +++ b/common/crypto/tlsgen/ca_test.go @@ -66,8 +66,8 @@ func TestTLSCA(t *testing.T) { } tlsCfg.RootCAs.AppendCertsFromPEM(ca.CertBytes()) tlsOpts := grpc.WithTransportCredentials(credentials.NewTLS(tlsCfg)) - ctx := context.Background() - ctx, _ = context.WithTimeout(ctx, time.Second) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() conn, err := grpc.DialContext(ctx, fmt.Sprintf("127.0.0.1:%d", randomPort), tlsOpts, grpc.WithBlock()) if err != nil { return err diff --git a/core/chaincode/accesscontrol/access_test.go b/core/chaincode/accesscontrol/access_test.go index d0a20156409..60a464562b9 100644 --- a/core/chaincode/accesscontrol/access_test.go +++ b/core/chaincode/accesscontrol/access_test.go @@ -112,8 +112,8 @@ func newClient(t *testing.T, port int, cert *tls.Certificate, peerCACert []byte) tlsCfg.Certificates = []tls.Certificate{*cert} } tlsOpts := grpc.WithTransportCredentials(credentials.NewTLS(tlsCfg)) - ctx := context.Background() - ctx, _ = context.WithTimeout(ctx, time.Second) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() conn, err := grpc.DialContext(ctx, fmt.Sprintf("localhost:%d", port), tlsOpts, grpc.WithBlock()) if err != nil { return nil, err diff --git a/core/comm/connection.go b/core/comm/connection.go index d75c8026bed..99b39cc7854 100644 --- a/core/comm/connection.go +++ b/core/comm/connection.go @@ -205,10 +205,12 @@ func NewClientConnectionWithAddress(peerAddress string, block bool, tslEnabled b if block { opts = append(opts, grpc.WithBlock()) } - opts = append(opts, grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(MaxRecvMsgSize), - grpc.MaxCallSendMsgSize(MaxSendMsgSize))) - ctx := context.Background() - ctx, _ = context.WithTimeout(ctx, defaultTimeout) + opts = append(opts, grpc.WithDefaultCallOptions( + grpc.MaxCallRecvMsgSize(MaxRecvMsgSize), + grpc.MaxCallSendMsgSize(MaxSendMsgSize), + )) + ctx, cancel := context.WithTimeout(context.Background(), defaultTimeout) + defer cancel() conn, err := grpc.DialContext(ctx, peerAddress, opts...) if err != nil { return nil, err diff --git a/core/comm/connection_test.go b/core/comm/connection_test.go index 51bd82989e4..6c9e7b48f58 100644 --- a/core/comm/connection_test.go +++ b/core/comm/connection_test.go @@ -354,8 +354,8 @@ func testInvoke( creds, err := cs.GetDeliverServiceCredentials(channelID) assert.NoError(t, err) endpoint := fmt.Sprintf("localhost:%d", s.port) - ctx := context.Background() - ctx, _ = context.WithTimeout(ctx, 1*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() conn, err := grpc.DialContext(ctx, endpoint, grpc.WithTransportCredentials(creds), grpc.WithBlock()) if shouldSucceed { assert.NoError(t, err) diff --git a/core/comm/server_test.go b/core/comm/server_test.go index 0905ddb3f5d..7fbc4ccaaf9 100644 --- a/core/comm/server_test.go +++ b/core/comm/server_test.go @@ -130,8 +130,8 @@ func (esss *emptyServiceServer) EmptyStream(stream testpb.EmptyService_EmptyStre // invoke the EmptyCall RPC func invokeEmptyCall(address string, dialOptions []grpc.DialOption) (*testpb.Empty, error) { - ctx := context.Background() - ctx, _ = context.WithTimeout(ctx, timeout) + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() //create GRPC client conn clientConn, err := grpc.DialContext(ctx, address, dialOptions...) if err != nil { @@ -142,12 +142,8 @@ func invokeEmptyCall(address string, dialOptions []grpc.DialOption) (*testpb.Emp //create GRPC client client := testpb.NewEmptyServiceClient(clientConn) - callCtx := context.Background() - callCtx, cancel := context.WithTimeout(callCtx, timeout) - defer cancel() - //invoke service - empty, err := client.EmptyCall(callCtx, new(testpb.Empty)) + empty, err := client.EmptyCall(context.Background(), new(testpb.Empty)) if err != nil { return nil, err } @@ -157,8 +153,8 @@ func invokeEmptyCall(address string, dialOptions []grpc.DialOption) (*testpb.Emp // invoke the EmptyStream RPC func invokeEmptyStream(address string, dialOptions []grpc.DialOption) (*testpb.Empty, error) { - ctx := context.Background() - ctx, _ = context.WithTimeout(ctx, timeout) + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() //create GRPC client conn clientConn, err := grpc.DialContext(ctx, address, dialOptions...) if err != nil { @@ -632,8 +628,7 @@ func TestNewGRPCServerFromListener(t *testing.T) { _, err = invokeEmptyCall(testAddress, dialOptions) if err != nil { - t.Fatalf("GRPC client failed to invoke the EmptyCall service on %s: %v", - testAddress, err) + t.Fatalf("GRPC client failed to invoke the EmptyCall service on %s: %v", testAddress, err) } else { t.Log("GRPC client successfully invoked the EmptyCall service: " + testAddress) } @@ -1543,18 +1538,15 @@ func TestKeepaliveClientResponse(t *testing.T) { defer srv.Stop() // test that connection does not close with response to ping - connectCtx, cancel := context.WithDeadline( - context.Background(), - time.Now().Add(1*time.Second)) + connectCtx, cancel := context.WithTimeout(context.Background(), time.Second) clientTransport, err := transport.NewClientTransport( connectCtx, context.Background(), transport.TargetInfo{Addr: testAddress}, transport.ConnectOptions{}, - func() {}) - if err != nil { - cancel() - } + func() {}, + ) + cancel() assert.NoError(t, err, "Unexpected error creating client transport") defer clientTransport.Close() // sleep past keepalive timeout diff --git a/core/deliverservice/client.go b/core/deliverservice/client.go index cb27670412f..bd9e0d53d68 100644 --- a/core/deliverservice/client.go +++ b/core/deliverservice/client.go @@ -172,6 +172,7 @@ func (bc *broadcastClient) connect() error { if err != nil { logger.Error("Connection to ", endpoint, "established but was unable to create gRPC stream:", err) conn.Close() + cf() return err } err = bc.afterConnect(conn, abc, cf, endpoint) diff --git a/core/deliverservice/deliveryclient.go b/core/deliverservice/deliveryclient.go index b92c40c8874..74d2d2ca17b 100644 --- a/core/deliverservice/deliveryclient.go +++ b/core/deliverservice/deliveryclient.go @@ -259,8 +259,8 @@ func DefaultConnectionFactory(channelID string) func(endpoint string) (*grpc.Cli } else { dialOpts = append(dialOpts, grpc.WithInsecure()) } - ctx := context.Background() - ctx, _ = context.WithTimeout(ctx, getConnectionTimeout()) + ctx, cancel := context.WithTimeout(context.Background(), getConnectionTimeout()) + defer cancel() return grpc.DialContext(ctx, endpoint, dialOpts...) } } diff --git a/core/peer/pkg_test.go b/core/peer/pkg_test.go index 3d9f73a5390..c7e81e19c50 100644 --- a/core/peer/pkg_test.go +++ b/core/peer/pkg_test.go @@ -61,8 +61,8 @@ func createCertPool(rootCAs [][]byte) (*x509.CertPool, error) { func invokeEmptyCall(address string, dialOptions []grpc.DialOption) (*testpb.Empty, error) { //add DialOptions dialOptions = append(dialOptions, grpc.WithBlock()) - ctx := context.Background() - ctx, _ = context.WithTimeout(ctx, timeout) + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() //create GRPC client conn clientConn, err := grpc.DialContext(ctx, address, dialOptions...) if err != nil { @@ -73,12 +73,8 @@ func invokeEmptyCall(address string, dialOptions []grpc.DialOption) (*testpb.Emp //create GRPC client client := testpb.NewTestServiceClient(clientConn) - callCtx := context.Background() - callCtx, cancel := context.WithTimeout(callCtx, timeout) - defer cancel() - //invoke service - empty, err := client.EmptyCall(callCtx, new(testpb.Empty)) + empty, err := client.EmptyCall(context.Background(), new(testpb.Empty)) if err != nil { return nil, err } diff --git a/gossip/comm/comm_test.go b/gossip/comm/comm_test.go index 92c7421c8fb..c6c6a79e129 100644 --- a/gossip/comm/comm_test.go +++ b/gossip/comm/comm_test.go @@ -143,8 +143,8 @@ func handshaker(endpoint string, comm Comm, t *testing.T, connMutator msgMutator secureOpts = grpc.WithInsecure() } acceptChan := comm.Accept(acceptAll) - ctx := context.Background() - ctx, _ = context.WithTimeout(ctx, time.Second) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() conn, err := grpc.DialContext(ctx, "localhost:9611", secureOpts, grpc.WithBlock()) assert.NoError(t, err, "%v", err) if err != nil { @@ -530,8 +530,8 @@ func TestCloseConn(t *testing.T) { } ta := credentials.NewTLS(tlsCfg) - ctx := context.Background() - ctx, _ = context.WithTimeout(ctx, time.Second) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() conn, err := grpc.DialContext(ctx, "localhost:1611", grpc.WithTransportCredentials(ta), grpc.WithBlock()) assert.NoError(t, err, "%v", err) cl := proto.NewGossipClient(conn) diff --git a/gossip/comm/crypto_test.go b/gossip/comm/crypto_test.go index a4732b02721..f4276f9eaa9 100644 --- a/gossip/comm/crypto_test.go +++ b/gossip/comm/crypto_test.go @@ -83,8 +83,8 @@ func TestCertificateExtraction(t *testing.T) { Certificates: []tls.Certificate{clientCert}, InsecureSkipVerify: true, }) - ctx := context.Background() - ctx, _ = context.WithTimeout(ctx, time.Second) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() conn, err := grpc.DialContext(ctx, "localhost:5611", grpc.WithTransportCredentials(ta), grpc.WithBlock()) assert.NoError(t, err, "%v", err) diff --git a/scripts/golinter.sh b/scripts/golinter.sh index 5630f519bb4..56e47edde5f 100755 --- a/scripts/golinter.sh +++ b/scripts/golinter.sh @@ -40,3 +40,13 @@ if [ -n "$OUTPUT" ]; then echo $OUTPUT exit 1 fi +# This block should removed once the lostcancel issues have been corrected and +# the previous invocation of vet should remove the -lostcancel=false option. +# FAB-12082 - gossip +# FAB-12083 - orderer +OUTPUT="$(go vet -lostcancel=true ./... 2>&1 | grep -Ev '^#|(fabric/)?gossip/comm/comm_impl.go|(fabric)?orderer/common/cluster/comm.go' || true)" +if [ -n "$OUTPUT" ]; then + echo "The following files contain go vet errors" + echo $OUTPUT + exit 1 +fi