Change grpc_handshaker_args to be owned by the handshake manager.

Also clean up hand-off semantics of endpoints.

2 files changed, 27 insertions, 12 deletions

diff --git a/src/core/lib/channel/handshaker.c b/src/core/lib/channel/handshaker.c
index 89130e9951..905db118be 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 8cad81c444..f0614c354b 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,