Code review changes.

4 files changed, 33 insertions, 22 deletions

diff --git a/src/core/ext/client_channel/http_connect_handshaker.c b/src/core/ext/client_channel/http_connect_handshaker.c
index 6a34e390bc..e0b717dac5 100644
--- a/src/core/ext/client_channel/http_connect_handshaker.c
+++ b/src/core/ext/client_channel/http_connect_handshaker.c
@@ -228,6 +228,9 @@ static void on_read_done(grpc_exec_ctx* exec_ctx, void* arg,
   // Success.  Invoke handshake-done callback.
   grpc_exec_ctx_sched(exec_ctx, handshaker->on_handshake_done, error, NULL);
 done:
+  // Set shutdown to true so that subsequent calls to
+  // http_connect_handshaker_shutdown() do nothing.
+  handshaker->shutdown = true;
   gpr_mu_unlock(&handshaker->mu);
   http_connect_handshaker_unref(exec_ctx, handshaker);
 }
diff --git a/src/core/ext/transport/chttp2/client/insecure/channel_create.c b/src/core/ext/transport/chttp2/client/insecure/channel_create.c
index 00b272de27..d42e6e98aa 100644
--- a/src/core/ext/transport/chttp2/client/insecure/channel_create.c
+++ b/src/core/ext/transport/chttp2/client/insecure/channel_create.c
@@ -39,6 +39,7 @@
 #include <grpc/slice.h>
 #include <grpc/slice_buffer.h>
 #include <grpc/support/alloc.h>
+#include <grpc/support/string_util.h>
 
 #include "src/core/ext/client_channel/client_channel.h"
 #include "src/core/ext/client_channel/http_connect_handshaker.h"
@@ -60,6 +61,8 @@ typedef struct {
   grpc_connector base;
   gpr_refcount refs;
 
+  char *server_name;
+
   grpc_closure *notify;
   grpc_connect_in_args args;
   grpc_connect_out_args *result;
@@ -82,7 +85,7 @@ static void connector_unref(grpc_exec_ctx *exec_ctx, grpc_connector *con) {
   connector *c = (connector *)con;
   if (gpr_unref(&c->refs)) {
     /* c->initial_string_buffer does not need to be destroyed */
-    grpc_handshake_manager_destroy(exec_ctx, c->handshake_mgr);
+    gpr_free(c->server_name);
     gpr_free(c);
   }
 }
@@ -107,6 +110,7 @@ static void on_handshake_done(grpc_exec_ctx *exec_ctx, void *arg,
   grpc_closure *notify = c->notify;
   c->notify = NULL;
   grpc_exec_ctx_sched(exec_ctx, notify, GRPC_ERROR_REF(error), NULL);
+  grpc_handshake_manager_destroy(exec_ctx, c->handshake_mgr);
 }
 
 static void connected(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) {
@@ -123,6 +127,14 @@ static void connected(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) {
       grpc_endpoint_write(exec_ctx, tcp, &c->initial_string_buffer,
                           &c->initial_string_sent);
     } else {
+      c->handshake_mgr = grpc_handshake_manager_create();
+      char *proxy_name = grpc_get_http_proxy_server();
+      if (proxy_name != NULL) {
+        grpc_handshake_manager_add(
+            c->handshake_mgr,
+            grpc_http_connect_handshaker_create(proxy_name, c->server_name));
+        gpr_free(proxy_name);
+      }
       grpc_handshake_manager_do_handshake(
           exec_ctx, c->handshake_mgr, tcp, c->args.channel_args,
           c->args.deadline, NULL /* acceptor */, on_handshake_done, c);
@@ -174,14 +186,7 @@ static grpc_subchannel *client_channel_factory_create_subchannel(
   memset(c, 0, sizeof(*c));
   c->base.vtable = &connector_vtable;
   gpr_ref_init(&c->refs, 1);
-  c->handshake_mgr = grpc_handshake_manager_create();
-  char *proxy_name = grpc_get_http_proxy_server();
-  if (proxy_name != NULL) {
-    grpc_handshake_manager_add(
-        c->handshake_mgr,
-        grpc_http_connect_handshaker_create(proxy_name, args->server_name));
-    gpr_free(proxy_name);
-  }
+  c->server_name = gpr_strdup(args->server_name);
   grpc_subchannel *s = grpc_subchannel_create(exec_ctx, &c->base, args);
   grpc_connector_unref(exec_ctx, &c->base);
   return s;
diff --git a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.c b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.c
index b4a30f94fc..93023af275 100644
--- a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.c
+++ b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.c
@@ -39,6 +39,7 @@
 #include <grpc/slice.h>
 #include <grpc/slice_buffer.h>
 #include <grpc/support/alloc.h>
+#include <grpc/support/string_util.h>
 
 #include "src/core/ext/client_channel/client_channel.h"
 #include "src/core/ext/client_channel/http_connect_handshaker.h"
@@ -63,6 +64,7 @@ typedef struct {
   gpr_refcount refs;
 
   grpc_channel_security_connector *security_connector;
+  char *server_name;
 
   grpc_closure *notify;
   grpc_connect_in_args args;
@@ -92,7 +94,7 @@ static void connector_unref(grpc_exec_ctx *exec_ctx, grpc_connector *con) {
   if (gpr_unref(&c->refs)) {
     /* c->initial_string_buffer does not need to be destroyed */
     grpc_channel_args_destroy(c->tmp_args);
-    grpc_handshake_manager_destroy(exec_ctx, c->handshake_mgr);
+    gpr_free(c->server_name);
     gpr_free(c);
   }
 }
@@ -146,11 +148,20 @@ static void on_handshake_done(grpc_exec_ctx *exec_ctx, void *arg,
         exec_ctx, c->security_connector, args->endpoint, args->read_buffer,
         c->args.deadline, on_secure_handshake_done, c);
   }
+  grpc_handshake_manager_destroy(exec_ctx, c->handshake_mgr);
 }
 
 static void on_initial_connect_string_sent(grpc_exec_ctx *exec_ctx, void *arg,
                                            grpc_error *error) {
   connector *c = arg;
+  c->handshake_mgr = grpc_handshake_manager_create();
+  char *proxy_name = grpc_get_http_proxy_server();
+  if (proxy_name != NULL) {
+    grpc_handshake_manager_add(
+        c->handshake_mgr,
+        grpc_http_connect_handshaker_create(proxy_name, c->server_name));
+    gpr_free(proxy_name);
+  }
   grpc_handshake_manager_do_handshake(
       exec_ctx, c->handshake_mgr, c->connecting_endpoint, c->args.channel_args,
       c->args.deadline, NULL /* acceptor */, on_handshake_done, c);
@@ -173,9 +184,8 @@ static void connected(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) {
       grpc_endpoint_write(exec_ctx, tcp, &c->initial_string_buffer,
                           &c->initial_string_sent);
     } else {
-      grpc_handshake_manager_do_handshake(
-          exec_ctx, c->handshake_mgr, tcp, c->args.channel_args,
-          c->args.deadline, NULL /* acceptor */, on_handshake_done, c);
+      // Start handshaking.
+      on_initial_connect_string_sent(exec_ctx, c, error);
     }
   } else {
     memset(c->result, 0, sizeof(*c->result));
@@ -252,14 +262,7 @@ static grpc_subchannel *client_channel_factory_create_subchannel(
   memset(c, 0, sizeof(*c));
   c->base.vtable = &connector_vtable;
   c->security_connector = f->security_connector;
-  c->handshake_mgr = grpc_handshake_manager_create();
-  char *proxy_name = grpc_get_http_proxy_server();
-  if (proxy_name != NULL) {
-    grpc_handshake_manager_add(
-        c->handshake_mgr,
-        grpc_http_connect_handshaker_create(proxy_name, args->server_name));
-    gpr_free(proxy_name);
-  }
+  c->server_name = gpr_strdup(args->server_name);
   gpr_mu_init(&c->mu);
   gpr_ref_init(&c->refs, 1);
   grpc_subchannel *s = grpc_subchannel_create(exec_ctx, &c->base, args);
diff --git a/src/core/lib/channel/handshaker.h b/src/core/lib/channel/handshaker.h
index 2e1f543512..ebbc1ff7f3 100644
--- a/src/core/lib/channel/handshaker.h
+++ b/src/core/lib/channel/handshaker.h
@@ -133,7 +133,7 @@ void grpc_handshake_manager_shutdown(grpc_exec_ctx* exec_ctx,
 /// Invokes handshakers in the order they were added.
 /// Takes ownership of \a endpoint, and then passes that ownership to
 /// the \a on_handshake_done callback.
-/// Does NOT take ownership of \a args.  Instead, makes a copy before
+/// Does NOT take ownership of \a channel_args.  Instead, makes a copy before
 /// invoking the first handshaker.
 /// \a acceptor will be NULL for client-side handshakers.
 ///