Merge pull request #2208 from ctiller/tis-but-thy-name

Client configuration for core

7 files changed, 102 insertions, 77 deletions

diff --git a/src/core/security/client_auth_filter.c b/src/core/security/client_auth_filter.c
index e9bd45db68..f8d18d9b17 100644
--- a/src/core/security/client_auth_filter.c
+++ b/src/core/security/client_auth_filter.c
@@ -58,7 +58,7 @@ typedef struct {
      so that work can progress when this call wants work to
      progress */
   grpc_pollset *pollset;
-  grpc_transport_op op;
+  grpc_transport_stream_op op;
   size_t op_md_idx;
   int sent_initial_metadata;
   grpc_linked_mdelem md_links[MAX_CREDENTIALS_METADATA_COUNT];
@@ -77,7 +77,7 @@ typedef struct {
 static void bubble_up_error(grpc_call_element *elem, const char *error_msg) {
   call_data *calld = elem->call_data;
   channel_data *chand = elem->channel_data;
-  grpc_transport_op_add_cancellation(
+  grpc_transport_stream_op_add_cancellation(
       &calld->op, GRPC_STATUS_UNAUTHENTICATED,
       grpc_mdstr_from_string(chand->md_ctx, error_msg));
   grpc_call_next_op(elem, &calld->op);
@@ -90,7 +90,7 @@ static void on_credentials_metadata(void *user_data,
   grpc_call_element *elem = (grpc_call_element *)user_data;
   call_data *calld = elem->call_data;
   channel_data *chand = elem->channel_data;
-  grpc_transport_op *op = &calld->op;
+  grpc_transport_stream_op *op = &calld->op;
   grpc_metadata_batch *mdb;
   size_t i;
   if (status != GRPC_CREDENTIALS_OK) {
@@ -131,7 +131,7 @@ static char *build_service_url(const char *url_scheme, call_data *calld) {
 }
 
 static void send_security_metadata(grpc_call_element *elem,
-                                   grpc_transport_op *op) {
+                                   grpc_transport_stream_op *op) {
   call_data *calld = elem->call_data;
   channel_data *chand = elem->channel_data;
   grpc_client_security_context *ctx =
@@ -193,7 +193,7 @@ static void on_host_checked(void *user_data, grpc_security_status status) {
    op contains type and call direction information, in addition to the data
    that is being sent or received. */
 static void auth_start_transport_op(grpc_call_element *elem,
-                                    grpc_transport_op *op) {
+                                    grpc_transport_stream_op *op) {
   /* grab pointers to our data from the call element */
   call_data *calld = elem->call_data;
   channel_data *chand = elem->channel_data;
@@ -253,17 +253,10 @@ static void auth_start_transport_op(grpc_call_element *elem,
   grpc_call_next_op(elem, op);
 }
 
-/* Called on special channel events, such as disconnection or new incoming
-   calls on the server */
-static void channel_op(grpc_channel_element *elem,
-                       grpc_channel_element *from_elem, grpc_channel_op *op) {
-  grpc_channel_next_op(elem, op);
-}
-
 /* Constructor for call_data */
 static void init_call_elem(grpc_call_element *elem,
                            const void *server_transport_data,
-                           grpc_transport_op *initial_op) {
+                           grpc_transport_stream_op *initial_op) {
   call_data *calld = elem->call_data;
   calld->creds = NULL;
   calld->host = NULL;
@@ -287,7 +280,7 @@ static void destroy_call_elem(grpc_call_element *elem) {
 }
 
 /* Constructor for channel_data */
-static void init_channel_elem(grpc_channel_element *elem,
+static void init_channel_elem(grpc_channel_element *elem, grpc_channel *master,
                               const grpc_channel_args *args,
                               grpc_mdctx *metadata_context, int is_first,
                               int is_last) {
@@ -298,14 +291,14 @@ static void init_channel_elem(grpc_channel_element *elem,
   /* The first and the last filters tend to be implemented differently to
      handle the case that there's no 'next' filter to call on the up or down
      path */
-  GPR_ASSERT(!is_first);
   GPR_ASSERT(!is_last);
   GPR_ASSERT(sc != NULL);
 
   /* initialize members */
   GPR_ASSERT(sc->is_client_side);
   chand->security_connector =
-      (grpc_channel_security_connector *)grpc_security_connector_ref(sc);
+      (grpc_channel_security_connector *)GRPC_SECURITY_CONNECTOR_REF(
+          sc, "client_auth_filter");
   chand->md_ctx = metadata_context;
   chand->authority_string = grpc_mdstr_from_string(chand->md_ctx, ":authority");
   chand->path_string = grpc_mdstr_from_string(chand->md_ctx, ":path");
@@ -318,7 +311,8 @@ static void destroy_channel_elem(grpc_channel_element *elem) {
   /* grab pointers to our data from the channel element */
   channel_data *chand = elem->channel_data;
   grpc_channel_security_connector *ctx = chand->security_connector;
-  if (ctx != NULL) grpc_security_connector_unref(&ctx->base);
+  if (ctx != NULL)
+    GRPC_SECURITY_CONNECTOR_UNREF(&ctx->base, "client_auth_filter");
   if (chand->authority_string != NULL) {
     grpc_mdstr_unref(chand->authority_string);
   }
@@ -334,6 +328,6 @@ static void destroy_channel_elem(grpc_channel_element *elem) {
 }
 
 const grpc_channel_filter grpc_client_auth_filter = {
-    auth_start_transport_op, channel_op,           sizeof(call_data),
+    auth_start_transport_op, grpc_channel_next_op, sizeof(call_data),
     init_call_elem,          destroy_call_elem,    sizeof(channel_data),
     init_channel_elem,       destroy_channel_elem, "client-auth"};
diff --git a/src/core/security/credentials.c b/src/core/security/credentials.c
index cf663faf2d..e79e9ce351 100644
--- a/src/core/security/credentials.c
+++ b/src/core/security/credentials.c
@@ -225,7 +225,7 @@ static grpc_security_status ssl_create_security_connector(
   arg.type = GRPC_ARG_STRING;
   arg.key = GRPC_ARG_HTTP2_SCHEME;
   arg.value.string = "https";
-  *new_args = grpc_channel_args_copy_and_add(args, &arg);
+  *new_args = grpc_channel_args_copy_and_add(args, &arg, 1);
   return status;
 }
 
diff --git a/src/core/security/secure_transport_setup.c b/src/core/security/secure_transport_setup.c
index 1b39ab141e..731b382f09 100644
--- a/src/core/security/secure_transport_setup.c
+++ b/src/core/security/secure_transport_setup.c
@@ -74,7 +74,7 @@ static void secure_transport_setup_done(grpc_secure_transport_setup *s,
   if (s->handshaker != NULL) tsi_handshaker_destroy(s->handshaker);
   if (s->handshake_buffer != NULL) gpr_free(s->handshake_buffer);
   gpr_slice_buffer_destroy(&s->left_overs);
-  grpc_security_connector_unref(s->connector);
+  GRPC_SECURITY_CONNECTOR_UNREF(s->connector, "secure_transport_setup");
   gpr_free(s);
 }
 
@@ -234,8 +234,9 @@ static void on_handshake_data_received_from_peer(
                          gpr_slice_split_tail(&slices[i], consumed_slice_size));
     gpr_slice_unref(slices[i]); /* split_tail above increments refcount. */
   }
-  gpr_slice_buffer_addn(&s->left_overs, &slices[i + 1],
-                        num_left_overs - (size_t)has_left_overs_in_current_slice);
+  gpr_slice_buffer_addn(
+      &s->left_overs, &slices[i + 1],
+      num_left_overs - (size_t)has_left_overs_in_current_slice);
   check_peer(s);
 }
 
@@ -275,7 +276,8 @@ void grpc_setup_secure_transport(grpc_security_connector *connector,
     secure_transport_setup_done(s, 0);
     return;
   }
-  s->connector = grpc_security_connector_ref(connector);
+  s->connector =
+      GRPC_SECURITY_CONNECTOR_REF(connector, "secure_transport_setup");
   s->handshake_buffer_size = GRPC_INITIAL_HANDSHAKE_BUFFER_SIZE;
   s->handshake_buffer = gpr_malloc(s->handshake_buffer_size);
   s->endpoint = nonsecure_endpoint;
diff --git a/src/core/security/security_connector.c b/src/core/security/security_connector.c
index 34cb0395a2..f6e423eb27 100644
--- a/src/core/security/security_connector.c
+++ b/src/core/security/security_connector.c
@@ -84,12 +84,12 @@ static const char *ssl_cipher_suites(void) {
 /* -- Common methods. -- */
 
 /* Returns the first property with that name. */
-const tsi_peer_property *tsi_peer_get_property_by_name(
-    const tsi_peer *peer, const char *name) {
+const tsi_peer_property *tsi_peer_get_property_by_name(const tsi_peer *peer,
+                                                       const char *name) {
   size_t i;
   if (peer == NULL) return NULL;
   for (i = 0; i < peer->property_count; i++) {
-    const tsi_peer_property* property = &peer->properties[i];
+    const tsi_peer_property *property = &peer->properties[i];
     if (name == NULL && property->name == NULL) {
       return property;
     }
@@ -124,24 +124,44 @@ grpc_security_status grpc_channel_security_connector_check_call_host(
   return sc->check_call_host(sc, host, cb, user_data);
 }
 
-void grpc_security_connector_unref(grpc_security_connector *sc) {
-  if (sc == NULL) return;
-  if (gpr_unref(&sc->refcount)) sc->vtable->destroy(sc);
-}
-
+#ifdef GRPC_SECURITY_CONNECTOR_REFCOUNT_DEBUG
+grpc_security_connector *grpc_security_connector_ref(
+    grpc_security_connector *sc, const char *file, int line,
+    const char *reason) {
+  if (sc == NULL) return NULL;
+  gpr_log(file, line, GPR_LOG_SEVERITY_DEBUG,
+          "SECURITY_CONNECTOR:%p   ref %d -> %d %s", sc,
+          (int)sc->refcount.count, (int)sc->refcount.count + 1, reason);
+#else
 grpc_security_connector *grpc_security_connector_ref(
     grpc_security_connector *sc) {
   if (sc == NULL) return NULL;
+#endif
   gpr_ref(&sc->refcount);
   return sc;
 }
 
+#ifdef GRPC_SECURITY_CONNECTOR_REFCOUNT_DEBUG
+void grpc_security_connector_unref(grpc_security_connector *sc,
+                                   const char *file, int line,
+                                   const char *reason) {
+  if (sc == NULL) return;
+  gpr_log(file, line, GPR_LOG_SEVERITY_DEBUG,
+          "SECURITY_CONNECTOR:%p unref %d -> %d %s", sc,
+          (int)sc->refcount.count, (int)sc->refcount.count - 1, reason);
+#else
+void grpc_security_connector_unref(grpc_security_connector *sc) {
+  if (sc == NULL) return;
+#endif
+  if (gpr_unref(&sc->refcount)) sc->vtable->destroy(sc);
+}
+
 static void connector_pointer_arg_destroy(void *p) {
-  grpc_security_connector_unref(p);
+  GRPC_SECURITY_CONNECTOR_UNREF(p, "connector_pointer_arg");
 }
 
 static void *connector_pointer_arg_copy(void *p) {
-  return grpc_security_connector_ref(p);
+  return GRPC_SECURITY_CONNECTOR_REF(p, "connector_pointer_arg");
 }
 
 grpc_arg grpc_security_connector_to_arg(grpc_security_connector *sc) {
@@ -580,7 +600,8 @@ grpc_security_status grpc_ssl_channel_security_connector_create(
       config->pem_private_key, config->pem_private_key_size,
       config->pem_cert_chain, config->pem_cert_chain_size, pem_root_certs,
       pem_root_certs_size, ssl_cipher_suites(), alpn_protocol_strings,
-      alpn_protocol_string_lengths, (uint16_t)num_alpn_protocols, &c->handshaker_factory);
+      alpn_protocol_string_lengths, (uint16_t)num_alpn_protocols,
+      &c->handshaker_factory);
   if (result != TSI_OK) {
     gpr_log(GPR_ERROR, "Handshaker factory creation failed with %s.",
             tsi_result_to_string(result));
@@ -633,8 +654,8 @@ grpc_security_status grpc_ssl_server_security_connector_create(
       (const unsigned char **)config->pem_cert_chains,
       config->pem_cert_chains_sizes, config->num_key_cert_pairs,
       config->pem_root_certs, config->pem_root_certs_size, ssl_cipher_suites(),
-      alpn_protocol_strings, alpn_protocol_string_lengths, (uint16_t)num_alpn_protocols,
-      &c->handshaker_factory);
+      alpn_protocol_strings, alpn_protocol_string_lengths,
+      (uint16_t)num_alpn_protocols, &c->handshaker_factory);
   if (result != TSI_OK) {
     gpr_log(GPR_ERROR, "Handshaker factory creation failed with %s.",
             tsi_result_to_string(result));
diff --git a/src/core/security/security_connector.h b/src/core/security/security_connector.h
index ee3057b43b..a4c723f026 100644
--- a/src/core/security/security_connector.h
+++ b/src/core/security/security_connector.h
@@ -80,12 +80,25 @@ struct grpc_security_connector {
   grpc_auth_context *auth_context; /* Populated after the peer is checked. */
 };
 
-/* Increments the refcount. */
+/* Refcounting. */
+#ifdef GRPC_SECURITY_CONNECTOR_REFCOUNT_DEBUG
+#define GRPC_SECURITY_CONNECTOR_REF(p, r) \
+  grpc_security_connector_ref((p), __FILE__, __LINE__, (r))
+#define GRPC_SECURITY_CONNECTOR_UNREF(p, r) \
+  grpc_security_connector_unref((p), __FILE__, __LINE__, (r))
 grpc_security_connector *grpc_security_connector_ref(
-    grpc_security_connector *sc);
-
-/* Decrements the refcount and destroys the object if it reaches 0. */
-void grpc_security_connector_unref(grpc_security_connector *sc);
+    grpc_security_connector *policy, const char *file, int line,
+    const char *reason);
+void grpc_security_connector_unref(grpc_security_connector *policy,
+                                   const char *file, int line,
+                                   const char *reason);
+#else
+#define GRPC_SECURITY_CONNECTOR_REF(p, r) grpc_security_connector_ref((p))
+#define GRPC_SECURITY_CONNECTOR_UNREF(p, r) grpc_security_connector_unref((p))
+grpc_security_connector *grpc_security_connector_ref(
+    grpc_security_connector *policy);
+void grpc_security_connector_unref(grpc_security_connector *policy);
+#endif
 
 /* Handshake creation. */
 grpc_security_status grpc_security_connector_create_handshaker(
@@ -172,9 +185,9 @@ typedef struct {
   specific error code otherwise.
 */
 grpc_security_status grpc_ssl_channel_security_connector_create(
-    grpc_credentials *request_metadata_creds,
-    const grpc_ssl_config *config, const char *target_name,
-    const char *overridden_target_name, grpc_channel_security_connector **sc);
+    grpc_credentials *request_metadata_creds, const grpc_ssl_config *config,
+    const char *target_name, const char *overridden_target_name,
+    grpc_channel_security_connector **sc);
 
 /* Gets the default ssl roots. */
 size_t grpc_get_default_ssl_roots(const unsigned char **pem_root_certs);
@@ -200,8 +213,8 @@ grpc_security_status grpc_ssl_server_security_connector_create(
     const grpc_ssl_server_config *config, grpc_security_connector **sc);
 
 /* Util. */
-const tsi_peer_property *tsi_peer_get_property_by_name(
-    const tsi_peer *peer, const char *name);
+const tsi_peer_property *tsi_peer_get_property_by_name(const tsi_peer *peer,
+                                                       const char *name);
 
 /* Exposed for testing only. */
 grpc_auth_context *tsi_ssl_peer_to_auth_context(const tsi_peer *peer);
diff --git a/src/core/security/server_auth_filter.c b/src/core/security/server_auth_filter.c
index b8639287a5..10eef6d237 100644
--- a/src/core/security/server_auth_filter.c
+++ b/src/core/security/server_auth_filter.c
@@ -51,24 +51,17 @@ typedef struct channel_data {
    op contains type and call direction information, in addition to the data
    that is being sent or received. */
 static void auth_start_transport_op(grpc_call_element *elem,
-                                    grpc_transport_op *op) {
+                                    grpc_transport_stream_op *op) {
   /* TODO(jboeuf): Get the metadata and get a new context from it. */
 
   /* pass control down the stack */
   grpc_call_next_op(elem, op);
 }
 
-/* Called on special channel events, such as disconnection or new incoming
-   calls on the server */
-static void channel_op(grpc_channel_element *elem,
-                       grpc_channel_element *from_elem, grpc_channel_op *op) {
-  grpc_channel_next_op(elem, op);
-}
-
 /* Constructor for call_data */
 static void init_call_elem(grpc_call_element *elem,
                            const void *server_transport_data,
-                           grpc_transport_op *initial_op) {
+                           grpc_transport_stream_op *initial_op) {
   /* grab pointers to our data from the call element */
   call_data *calld = elem->call_data;
   channel_data *chand = elem->channel_data;
@@ -95,11 +88,10 @@ static void init_call_elem(grpc_call_element *elem,
 }
 
 /* Destructor for call_data */
-static void destroy_call_elem(grpc_call_element *elem) {
-}
+static void destroy_call_elem(grpc_call_element *elem) {}
 
 /* Constructor for channel_data */
-static void init_channel_elem(grpc_channel_element *elem,
+static void init_channel_elem(grpc_channel_element *elem, grpc_channel *master,
                               const grpc_channel_args *args, grpc_mdctx *mdctx,
                               int is_first, int is_last) {
   grpc_security_connector *sc = grpc_find_security_connector_in_args(args);
@@ -115,17 +107,19 @@ static void init_channel_elem(grpc_channel_element *elem,
 
   /* initialize members */
   GPR_ASSERT(!sc->is_client_side);
-  chand->security_connector = grpc_security_connector_ref(sc);
+  chand->security_connector =
+      GRPC_SECURITY_CONNECTOR_REF(sc, "server_auth_filter");
 }
 
 /* Destructor for channel data */
 static void destroy_channel_elem(grpc_channel_element *elem) {
   /* grab pointers to our data from the channel element */
   channel_data *chand = elem->channel_data;
-  grpc_security_connector_unref(chand->security_connector);
+  GRPC_SECURITY_CONNECTOR_UNREF(chand->security_connector,
+                                "server_auth_filter");
 }
 
 const grpc_channel_filter grpc_server_auth_filter = {
-    auth_start_transport_op, channel_op, sizeof(call_data), init_call_elem,
-    destroy_call_elem, sizeof(channel_data), init_channel_elem,
-    destroy_channel_elem, "server-auth"};
+    auth_start_transport_op, grpc_channel_next_op, sizeof(call_data),
+    init_call_elem,          destroy_call_elem,    sizeof(channel_data),
+    init_channel_elem,       destroy_channel_elem, "server-auth"};
diff --git a/src/core/security/server_secure_chttp2.c b/src/core/security/server_secure_chttp2.c
index 53afa1caad..8a7ada07af 100644
--- a/src/core/security/server_secure_chttp2.c
+++ b/src/core/security/server_secure_chttp2.c
@@ -70,38 +70,39 @@ static void state_unref(grpc_server_secure_state *state) {
     gpr_mu_lock(&state->mu);
     gpr_mu_unlock(&state->mu);
     /* clean up */
-    grpc_security_connector_unref(state->sc);
+    GRPC_SECURITY_CONNECTOR_UNREF(state->sc, "server");
     gpr_free(state);
   }
 }
 
-static grpc_transport_setup_result setup_transport(void *statep,
-                                                   grpc_transport *transport,
-                                                   grpc_mdctx *mdctx) {
+static void setup_transport(void *statep, grpc_transport *transport,
+                            grpc_mdctx *mdctx) {
   static grpc_channel_filter const *extra_filters[] = {
       &grpc_server_auth_filter, &grpc_http_server_filter};
   grpc_server_secure_state *state = statep;
-  grpc_transport_setup_result result;
   grpc_arg connector_arg = grpc_security_connector_to_arg(state->sc);
   grpc_channel_args *args_copy = grpc_channel_args_copy_and_add(
-      grpc_server_get_channel_args(state->server), &connector_arg);
-  result = grpc_server_setup_transport(state->server, transport, extra_filters,
-                                       GPR_ARRAY_SIZE(extra_filters), mdctx,
-                                       args_copy);
+      grpc_server_get_channel_args(state->server), &connector_arg, 1);
+  grpc_server_setup_transport(state->server, transport, extra_filters,
+                              GPR_ARRAY_SIZE(extra_filters), mdctx, args_copy);
   grpc_channel_args_destroy(args_copy);
-  return result;
 }
 
 static void on_secure_transport_setup_done(void *statep,
                                            grpc_security_status status,
                                            grpc_endpoint *secure_endpoint) {
   grpc_server_secure_state *state = statep;
+  grpc_transport *transport;
+  grpc_mdctx *mdctx;
   if (status == GRPC_SECURITY_OK) {
     gpr_mu_lock(&state->mu);
     if (!state->is_shutdown) {
-      grpc_create_chttp2_transport(
-          setup_transport, state, grpc_server_get_channel_args(state->server),
-          secure_endpoint, NULL, 0, grpc_mdctx_create(), 0);
+      mdctx = grpc_mdctx_create();
+      transport = grpc_create_chttp2_transport(
+          grpc_server_get_channel_args(state->server), secure_endpoint, mdctx,
+          0);
+      setup_transport(state, transport, mdctx);
+      grpc_chttp2_transport_start_reading(transport, NULL, 0);
     } else {
       /* We need to consume this here, because the server may already have gone
        * away. */
@@ -220,7 +221,7 @@ int grpc_server_add_secure_http2_port(grpc_server *server, const char *addr,
 /* Error path: cleanup and return */
 error:
   if (sc) {
-    grpc_security_connector_unref(sc);
+    GRPC_SECURITY_CONNECTOR_UNREF(sc, "server");
   }
   if (resolved) {
     grpc_resolved_addresses_destroy(resolved);