Cleanup conn.Close() handling in newHTTP2Client()

Removes the need to close the underlying connection in tlsCreds.ClientHandshake().
grpc · Aug 8, 2016 · efa105d · efa105d
1 parent 35896af
commit efa105d
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 7 deletions.
diff --git a/credentials/credentials.go b/credentials/credentials.go
@@ -139,7 +139,7 @@ func (c *tlsCreds) RequireTransportSecurity() bool {
 func (c *tlsCreds) ClientHandshake(ctx context.Context, addr string, rawConn net.Conn) (_ net.Conn, _ AuthInfo, err error) {
 	// use local cfg to avoid clobbering ServerName if using multiple endpoints
 	cfg := cloneTLSConfig(c.config)
-	if c.config.ServerName == "" {
+	if cfg.ServerName == "" {
 		colonPos := strings.LastIndex(addr, ":")
 		if colonPos == -1 {
 			colonPos = len(addr)
@@ -154,7 +154,6 @@ func (c *tlsCreds) ClientHandshake(ctx context.Context, addr string, rawConn net
 	select {
 	case err := <-errChannel:
 		if err != nil {
-			rawConn.Close()
 			return nil, nil, err
 		}
 	case <-ctx.Done():

diff --git a/transport/http2_client.go b/transport/http2_client.go
@@ -123,6 +123,12 @@ func newHTTP2Client(ctx context.Context, addr string, opts ConnectOptions) (_ Cl
 	if connErr != nil {
 		return nil, ConnectionErrorf(true, connErr, "transport: %v", connErr)
 	}
+	// Any further errors will close the underlying connection
+	defer func(conn net.Conn) {
+		if err != nil {
+			conn.Close()
+		}
+	}(conn)
 	var authInfo credentials.AuthInfo
 	if creds := opts.TransportCredentials; creds != nil {
 		scheme = "https"
@@ -132,11 +138,6 @@ func newHTTP2Client(ctx context.Context, addr string, opts ConnectOptions) (_ Cl
 		// Credentials handshake error is not a temporary error.
 		return nil, ConnectionErrorf(false, connErr, "transport: %v", connErr)
 	}
-	defer func() {
-		if err != nil {
-			conn.Close()
-		}
-	}()
 	ua := primaryUA
 	if opts.UserAgent != "" {
 		ua = opts.UserAgent + " " + ua