diff --git a/src/core/ext/client_channel/http_connect_handshaker.c b/src/core/ext/client_channel/http_connect_handshaker.c
index 407b8ce0232c23c98d07083a3ce0615ce6c25e79..971bbe8944c0ae17c51db921de7b57fcb56a6d27 100644
--- a/src/core/ext/client_channel/http_connect_handshaker.c
+++ b/src/core/ext/client_channel/http_connect_handshaker.c
@@ -56,6 +56,8 @@ typedef struct http_connect_handshaker {
gpr_mu mu;
// State saved while performing the handshake.
+ // args will be NULL when either there is no handshake in progress or
+ // when the handshaker is shutting down.
grpc_handshaker_args* args;
grpc_closure* on_handshake_done;
@@ -84,7 +86,7 @@ static void http_connect_handshaker_unref(http_connect_handshaker* handshaker) {
static void on_write_done(grpc_exec_ctx* exec_ctx, void* arg,
grpc_error* error) {
http_connect_handshaker* handshaker = arg;
- if (error != GRPC_ERROR_NONE) {
+ if (error != GRPC_ERROR_NONE || handshaker->args == NULL) {
// If the write failed, invoke the callback immediately with the error.
gpr_mu_lock(&handshaker->mu);
grpc_exec_ctx_sched(exec_ctx, handshaker->on_handshake_done,
@@ -96,7 +98,6 @@ static void on_write_done(grpc_exec_ctx* exec_ctx, void* arg,
// Otherwise, read the response.
// The read callback inherits our ref to the handshaker.
gpr_mu_lock(&handshaker->mu);
- GPR_ASSERT(handshaker->args != NULL);
grpc_endpoint_read(exec_ctx, handshaker->args->endpoint,
handshaker->args->read_buffer,
&handshaker->response_read_closure);
@@ -109,8 +110,7 @@ static void on_read_done(grpc_exec_ctx* exec_ctx, void* arg,
grpc_error* error) {
http_connect_handshaker* handshaker = arg;
gpr_mu_lock(&handshaker->mu);
- GPR_ASSERT(handshaker->args != NULL);
- if (error != GRPC_ERROR_NONE) {
+ if (error != GRPC_ERROR_NONE || handshaker->args == NULL) {
GRPC_ERROR_REF(error); // Take ref to pass to the handshake-done callback.
goto done;
}
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 c4c1e638a97516f0c5671790b30cbbc380b5220d..e0bce57fc28bfb99ed6d4a75b9ed4b145849a7fe 100644
--- a/src/core/ext/transport/chttp2/client/insecure/channel_create.c
+++ b/src/core/ext/transport/chttp2/client/insecure/channel_create.c
@@ -97,6 +97,7 @@ static void on_handshake_done(grpc_exec_ctx *exec_ctx, void *arg,
grpc_handshaker_args *args = arg;
connector *c = args->user_data;
if (error != GRPC_ERROR_NONE) {
+ grpc_endpoint_destroy(exec_ctx, args->endpoint);
grpc_channel_args_destroy(args->args);
gpr_free(args->read_buffer);
} else {
@@ -107,7 +108,6 @@ static void on_handshake_done(grpc_exec_ctx *exec_ctx, void *arg,
args->read_buffer);
c->result->channel_args = args->args;
}
- gpr_free(args);
grpc_closure *notify = c->notify;
c->notify = NULL;
grpc_exec_ctx_sched(exec_ctx, notify, GRPC_ERROR_REF(error), NULL);
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 9478d0db7ebc9d2b81f713ae9a3715cee23a819c..4bf04170eb804f649d792344f000a6e401ef07af 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
@@ -101,6 +101,7 @@ static void on_handshake_done(grpc_exec_ctx *exec_ctx, void *arg,
if (error != GRPC_ERROR_NONE) {
c->connecting_endpoint = NULL;
gpr_mu_unlock(&c->mu);
+ grpc_endpoint_destroy(exec_ctx, args->endpoint);
grpc_channel_args_destroy(args->args);
gpr_free(args->read_buffer);
} else {
diff --git a/src/core/ext/transport/chttp2/server/insecure/server_chttp2.c b/src/core/ext/transport/chttp2/server/insecure/server_chttp2.c
index 669a3869ab3e935bd89c727a10e919b9fa5663aa..5a9d4f89280872ecd28fce25d4a70cd649ce1a73 100644
--- a/src/core/ext/transport/chttp2/server/insecure/server_chttp2.c
+++ b/src/core/ext/transport/chttp2/server/insecure/server_chttp2.c
@@ -62,7 +62,7 @@ static void on_handshake_done(grpc_exec_ctx *exec_ctx, void *arg,
const char *error_str = grpc_error_string(error);
gpr_log(GPR_ERROR, "Handshaking failed: %s", error_str);
grpc_error_free_string(error_str);
- grpc_handshake_manager_shutdown(exec_ctx, state->handshake_mgr);
+ grpc_endpoint_destroy(exec_ctx, args->endpoint);
gpr_free(args->read_buffer);
} else {
// Beware that the call to grpc_create_chttp2_transport() has to happen
@@ -79,7 +79,6 @@ static void on_handshake_done(grpc_exec_ctx *exec_ctx, void *arg,
}
// Clean up.
grpc_channel_args_destroy(args->args);
- gpr_free(args);
grpc_handshake_manager_destroy(exec_ctx, state->handshake_mgr);
gpr_free(state);
}
diff --git a/src/core/ext/transport/chttp2/server/secure/server_secure_chttp2.c b/src/core/ext/transport/chttp2/server/secure/server_secure_chttp2.c
index 59aaf7fdd86b4e2ba634f2b4eda0fb8ff811bebf..3cf5417397139b34fcfdda3088598dc7de3f384d 100644
--- a/src/core/ext/transport/chttp2/server/secure/server_secure_chttp2.c
+++ b/src/core/ext/transport/chttp2/server/secure/server_secure_chttp2.c
@@ -122,9 +122,7 @@ static void on_handshake_done(grpc_exec_ctx *exec_ctx, void *arg,
gpr_log(GPR_ERROR, "Handshaking failed: %s", error_str);
grpc_error_free_string(error_str);
gpr_free(args->read_buffer);
- if (args->endpoint != NULL) {
- grpc_endpoint_destroy(exec_ctx, args->endpoint);
- }
+ grpc_endpoint_destroy(exec_ctx, args->endpoint);
gpr_mu_lock(&connection_state->server_state->mu);
} else {
gpr_mu_lock(&connection_state->server_state->mu);
diff --git a/src/core/lib/channel/handshaker.c b/src/core/lib/channel/handshaker.c
index 89130e995110f92e81d9924be38ead93171130cd..905db118be1929b1466114d3e2186f8aacdfb06d 100644
--- a/src/core/lib/channel/handshaker.c
+++ b/src/core/lib/channel/handshaker.c
@@ -88,6 +88,8 @@ struct grpc_handshake_manager {
// The final callback and user_data to invoke after the last handshaker.
grpc_closure on_handshake_done;
void* user_data;
+ // Handshaker args.
+ grpc_handshaker_args args;
};
grpc_handshake_manager* grpc_handshake_manager_create() {
@@ -205,22 +207,22 @@ void grpc_handshake_manager_do_handshake(
grpc_iomgr_cb_func on_handshake_done, void* user_data) {
// Construct handshaker args. These will be passed through all
// handshakers and eventually be freed by the on_handshake_done callback.
- grpc_handshaker_args* args = gpr_malloc(sizeof(*args));
- args->endpoint = endpoint;
- args->args = grpc_channel_args_copy(channel_args);
- args->read_buffer = gpr_malloc(sizeof(*args->read_buffer));
- grpc_slice_buffer_init(args->read_buffer);
+ mgr->args.endpoint = endpoint;
+ mgr->args.args = grpc_channel_args_copy(channel_args);
+ mgr->args.read_buffer = gpr_malloc(sizeof(*mgr->args.read_buffer));
+ grpc_slice_buffer_init(mgr->args.read_buffer);
// Initialize state needed for calling handshakers.
gpr_mu_lock(&mgr->mu);
GPR_ASSERT(mgr->index == 0);
mgr->acceptor = acceptor;
- grpc_closure_init(&mgr->call_next_handshaker, call_next_handshaker, args);
- grpc_closure_init(&mgr->on_handshake_done, on_handshake_done, args);
+ grpc_closure_init(&mgr->call_next_handshaker, call_next_handshaker,
+ &mgr->args);
+ grpc_closure_init(&mgr->on_handshake_done, on_handshake_done, &mgr->args);
// While chaining between handshakers, we use args->user_data to
// store a pointer to the handshake manager. This will be
// changed to point to the caller-supplied user_data before calling
// the on_handshake_done callback.
- args->user_data = mgr;
+ mgr->args.user_data = mgr;
mgr->user_data = user_data;
// Start deadline timer, which owns a ref.
gpr_ref(&mgr->refs);
@@ -229,6 +231,6 @@ void grpc_handshake_manager_do_handshake(
on_timeout, mgr, gpr_now(GPR_CLOCK_MONOTONIC));
// Start first handshaker, which also owns a ref.
gpr_ref(&mgr->refs);
- call_next_handshaker_locked(exec_ctx, mgr, args, GRPC_ERROR_NONE);
+ call_next_handshaker_locked(exec_ctx, mgr, &mgr->args, GRPC_ERROR_NONE);
gpr_mu_unlock(&mgr->mu);
}
diff --git a/src/core/lib/channel/handshaker.h b/src/core/lib/channel/handshaker.h
index 8cad81c444ed62065a123e810889f89e3d931f20..f0614c354b9972fa6c5b9c768dbfd289ae3499b9 100644
--- a/src/core/lib/channel/handshaker.h
+++ b/src/core/lib/channel/handshaker.h
@@ -55,7 +55,14 @@
typedef struct grpc_handshaker grpc_handshaker;
/// Arguments passed through handshakers and to the on_handshake_done callback.
-/// All data members are owned by the struct.
+///
+/// For handshakers, all members are input/output parameters; for
+/// example, a handshaker may read from \a endpoint and then later
+/// replace it with a wrapped endpoint. Similarly, a handshaker may
+/// modify \a args.
+///
+/// For the on_handshake_done callback, all members are input arguments,
+/// which the callback takes ownership of.
typedef struct {
grpc_endpoint* endpoint;
grpc_channel_args* args;
@@ -117,11 +124,17 @@ void grpc_handshake_manager_shutdown(grpc_exec_ctx* exec_ctx,
grpc_handshake_manager* mgr);
/// 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
/// invoking the first handshaker.
/// \a acceptor will be NULL for client-side handshakers.
-/// When done, invokes \a on_handshake_done with an argument of a
-/// grpc_handshaker_args object, which the callback takes ownership of.
+///
+/// When done, invokes \a on_handshake_done with a grpc_handshaker_args
+/// object as its argument. If the callback is invoked with error !=
+/// GRPC_ERROR_NONE, then handshaking failed and the resulting endpoint
+/// will have already been shut down (although the caller will still be
+/// responsible for destroying it).
void grpc_handshake_manager_do_handshake(
grpc_exec_ctx* exec_ctx, grpc_handshake_manager* mgr,
grpc_endpoint* endpoint, const grpc_channel_args* channel_args,