Merge branch 'tis-but-thy-name' of github.com:ctiller/grpc into tis-but-thy-name

15 files changed, 100 insertions, 45 deletions

diff --git a/src/core/security/security_connector.c b/src/core/security/security_connector.c
index 5512bb177a..34cb0395a2 100644
--- a/src/core/security/security_connector.c
+++ b/src/core/security/security_connector.c
@@ -196,12 +196,12 @@ typedef struct {
 static void fake_channel_destroy(grpc_security_connector *sc) {
   grpc_channel_security_connector *c = (grpc_channel_security_connector *)sc;
   grpc_credentials_unref(c->request_metadata_creds);
-  grpc_auth_context_unref(sc->auth_context);
+  GRPC_AUTH_CONTEXT_UNREF(sc->auth_context, "connector");
   gpr_free(sc);
 }
 
 static void fake_server_destroy(grpc_security_connector *sc) {
-  grpc_auth_context_unref(sc->auth_context);
+  GRPC_AUTH_CONTEXT_UNREF(sc->auth_context, "connector");
   gpr_free(sc);
 }
 
@@ -242,7 +242,7 @@ static grpc_security_status fake_check_peer(grpc_security_connector *sc,
     status = GRPC_SECURITY_ERROR;
     goto end;
   }
-  grpc_auth_context_unref(sc->auth_context);
+  GRPC_AUTH_CONTEXT_UNREF(sc->auth_context, "connector");
   sc->auth_context = grpc_auth_context_create(NULL, 1);
   sc->auth_context->properties[0] = grpc_auth_property_init_from_cstring(
       GRPC_TRANSPORT_SECURITY_TYPE_PROPERTY_NAME,
@@ -323,7 +323,7 @@ static void ssl_channel_destroy(grpc_security_connector *sc) {
   if (c->target_name != NULL) gpr_free(c->target_name);
   if (c->overridden_target_name != NULL) gpr_free(c->overridden_target_name);
   tsi_peer_destruct(&c->peer);
-  grpc_auth_context_unref(sc->auth_context);
+  GRPC_AUTH_CONTEXT_UNREF(sc->auth_context, "connector");
   gpr_free(sc);
 }
 
@@ -333,7 +333,7 @@ static void ssl_server_destroy(grpc_security_connector *sc) {
   if (c->handshaker_factory != NULL) {
     tsi_ssl_handshaker_factory_destroy(c->handshaker_factory);
   }
-  grpc_auth_context_unref(sc->auth_context);
+  GRPC_AUTH_CONTEXT_UNREF(sc->auth_context, "connector");
   gpr_free(sc);
 }
 
@@ -437,6 +437,9 @@ static grpc_security_status ssl_check_peer(grpc_security_connector *sc,
     gpr_log(GPR_ERROR, "Peer name %s is not in peer certificate", peer_name);
     return GRPC_SECURITY_ERROR;
   }
+  if (sc->auth_context != NULL) {
+    GRPC_AUTH_CONTEXT_UNREF(sc->auth_context, "connector");
+  }
   sc->auth_context = tsi_ssl_peer_to_auth_context(peer);
   return GRPC_SECURITY_OK;
 }
diff --git a/src/core/security/security_context.c b/src/core/security/security_context.c
index 9aba1e7f91..4d56549f9b 100644
--- a/src/core/security/security_context.c
+++ b/src/core/security/security_context.c
@@ -89,7 +89,7 @@ grpc_client_security_context *grpc_client_security_context_create(void) {
 void grpc_client_security_context_destroy(void *ctx) {
   grpc_client_security_context *c = (grpc_client_security_context *)ctx;
   grpc_credentials_unref(c->creds);
-  grpc_auth_context_unref(c->auth_context);
+  GRPC_AUTH_CONTEXT_UNREF(c->auth_context, "client_security_context");
   gpr_free(ctx);
 }
 
@@ -104,7 +104,7 @@ grpc_server_security_context *grpc_server_security_context_create(void) {
 
 void grpc_server_security_context_destroy(void *ctx) {
   grpc_server_security_context *c = (grpc_server_security_context *)ctx;
-  grpc_auth_context_unref(c->auth_context);
+  GRPC_AUTH_CONTEXT_UNREF(c->auth_context, "server_security_context");
   gpr_free(ctx);
 }
 
@@ -120,21 +120,40 @@ grpc_auth_context *grpc_auth_context_create(grpc_auth_context *chained,
   memset(ctx->properties, 0, property_count * sizeof(grpc_auth_property));
   ctx->property_count = property_count;
   gpr_ref_init(&ctx->refcount, 1);
-  if (chained != NULL) ctx->chained = grpc_auth_context_ref(chained);
+  if (chained != NULL) ctx->chained = GRPC_AUTH_CONTEXT_REF(chained, "chained");
   return ctx;
 }
 
+#ifdef GRPC_AUTH_CONTEXT_REFCOUNT_DEBUG
+grpc_auth_context *grpc_auth_context_ref(grpc_auth_context *ctx,
+                                         const char *file, int line,
+                                         const char *reason) {
+  if (ctx == NULL) return NULL;
+  gpr_log(file, line, GPR_LOG_SEVERITY_DEBUG,
+          "AUTH_CONTEXT:%p   ref %d -> %d %s", ctx, (int)ctx->refcount.count,
+          (int)ctx->refcount.count + 1, reason);
+#else
 grpc_auth_context *grpc_auth_context_ref(grpc_auth_context *ctx) {
   if (ctx == NULL) return NULL;
+#endif
   gpr_ref(&ctx->refcount);
   return ctx;
 }
 
+#ifdef GRPC_AUTH_CONTEXT_REFCOUNT_DEBUG
+void grpc_auth_context_unref(grpc_auth_context *ctx, const char *file, int line,
+                             const char *reason) {
+  if (ctx == NULL) return;
+  gpr_log(file, line, GPR_LOG_SEVERITY_DEBUG,
+          "AUTH_CONTEXT:%p unref %d -> %d %s", ctx, (int)ctx->refcount.count,
+          (int)ctx->refcount.count - 1, reason);
+#else
 void grpc_auth_context_unref(grpc_auth_context *ctx) {
   if (ctx == NULL) return;
+#endif
   if (gpr_unref(&ctx->refcount)) {
     size_t i;
-    grpc_auth_context_unref(ctx->chained);
+    GRPC_AUTH_CONTEXT_UNREF(ctx->chained, "chained");
     if (ctx->properties != NULL) {
       for (i = 0; i < ctx->property_count; i++) {
         grpc_auth_property_reset(&ctx->properties[i]);
@@ -223,8 +242,8 @@ grpc_auth_property grpc_auth_property_init(const char *name, const char *value,
 }
 
 void grpc_auth_property_reset(grpc_auth_property *property) {
-  if (property->name != NULL) gpr_free(property->name);
-  if (property->value != NULL) gpr_free(property->value);
+  gpr_free(property->name);
+  gpr_free(property->value);
   memset(property, 0, sizeof(grpc_auth_property));
 }
 
diff --git a/src/core/security/security_context.h b/src/core/security/security_context.h
index d8909cd6f1..20c4390898 100644
--- a/src/core/security/security_context.h
+++ b/src/core/security/security_context.h
@@ -55,9 +55,22 @@ grpc_auth_context *grpc_auth_context_create(grpc_auth_context *chained,
                                             size_t property_count);
 
 /* Refcounting. */
-grpc_auth_context *grpc_auth_context_ref(
-    grpc_auth_context *ctx);
-void grpc_auth_context_unref(grpc_auth_context *ctx);
+#ifdef GRPC_AUTH_CONTEXT_REFCOUNT_DEBUG
+#define GRPC_AUTH_CONTEXT_REF(p, r) \
+  grpc_auth_context_ref((p), __FILE__, __LINE__, (r))
+#define GRPC_AUTH_CONTEXT_UNREF(p, r) \
+  grpc_auth_context_unref((p), __FILE__, __LINE__, (r))
+grpc_auth_context *grpc_auth_context_ref(grpc_auth_context *policy,
+                                         const char *file, int line,
+                                         const char *reason);
+void grpc_auth_context_unref(grpc_auth_context *policy, const char *file,
+                             int line, const char *reason);
+#else
+#define GRPC_AUTH_CONTEXT_REF(p, r) grpc_auth_context_ref((p))
+#define GRPC_AUTH_CONTEXT_UNREF(p, r) grpc_auth_context_unref((p))
+grpc_auth_context *grpc_auth_context_ref(grpc_auth_context *policy);
+void grpc_auth_context_unref(grpc_auth_context *policy);
+#endif
 
 grpc_auth_property grpc_auth_property_init_from_cstring(const char *name,
                                                         const char *value);
diff --git a/src/core/security/server_auth_filter.c b/src/core/security/server_auth_filter.c
index a0dbeb58cd..5675c06402 100644
--- a/src/core/security/server_auth_filter.c
+++ b/src/core/security/server_auth_filter.c
@@ -75,9 +75,13 @@ static void init_call_elem(grpc_call_element *elem,
 
   /* Create a security context for the call and reference the auth context from
      the channel. */
+  if (initial_op->context[GRPC_CONTEXT_SECURITY].value != NULL) {
+    initial_op->context[GRPC_CONTEXT_SECURITY].destroy(
+        initial_op->context[GRPC_CONTEXT_SECURITY].value);
+  }
   server_ctx = grpc_server_security_context_create();
-  server_ctx->auth_context =
-      grpc_auth_context_ref(chand->security_connector->auth_context);
+  server_ctx->auth_context = GRPC_AUTH_CONTEXT_REF(
+      chand->security_connector->auth_context, "server_security_context");
   initial_op->context[GRPC_CONTEXT_SECURITY].value = server_ctx;
   initial_op->context[GRPC_CONTEXT_SECURITY].destroy =
       grpc_server_security_context_destroy;
diff --git a/src/core/surface/init.c b/src/core/surface/init.c
index 9b9ecae27d..3847ded28c 100644
--- a/src/core/surface/init.c
+++ b/src/core/surface/init.c
@@ -92,6 +92,7 @@ void grpc_shutdown(void) {
     census_shutdown();
     grpc_timers_global_destroy();
     grpc_tracer_shutdown();
+    grpc_resolver_registry_shutdown();
   }
   gpr_mu_unlock(&g_init_mu);
 }
diff --git a/src/core/transport/chttp2/incoming_metadata.c b/src/core/transport/chttp2/incoming_metadata.c
index e81927ab20..a4b7174329 100644
--- a/src/core/transport/chttp2/incoming_metadata.c
+++ b/src/core/transport/chttp2/incoming_metadata.c
@@ -47,6 +47,10 @@ void grpc_chttp2_incoming_metadata_buffer_init(
 
 void grpc_chttp2_incoming_metadata_buffer_destroy(
     grpc_chttp2_incoming_metadata_buffer *buffer) {
+  size_t i;
+  for (i = 0; i < buffer->count; i++) {
+    grpc_mdelem_unref(buffer->elems[i].md);
+  }
   gpr_free(buffer->elems);
 }
 
diff --git a/src/core/transport/chttp2_transport.c b/src/core/transport/chttp2_transport.c
index b2b4b1c802..4bcb317fe7 100644
--- a/src/core/transport/chttp2_transport.c
+++ b/src/core/transport/chttp2_transport.c
@@ -51,8 +51,6 @@
 #include "src/core/transport/chttp2/timeout_encoding.h"
 #include "src/core/transport/transport_impl.h"
 
-/* #define REFCOUNTING_DEBUG */
-
 #define DEFAULT_WINDOW 65535
 #define DEFAULT_CONNECTION_WINDOW_TARGET (1024 * 1024)
 #define MAX_WINDOW 0x7fffffffu
@@ -405,6 +403,7 @@ static void destroy_stream(grpc_transport *gt, grpc_stream *gs) {
   GPR_ASSERT(s->global.outgoing_sopb == NULL);
   GPR_ASSERT(s->global.publish_sopb == NULL);
   grpc_sopb_destroy(&s->writing.sopb);
+  grpc_sopb_destroy(&s->global.incoming_sopb);
   grpc_chttp2_data_parser_destroy(&s->parsing.data_parser);
   grpc_chttp2_incoming_metadata_buffer_destroy(&s->parsing.incoming_metadata);
   grpc_chttp2_incoming_metadata_buffer_destroy(&s->global.incoming_metadata);
@@ -865,11 +864,22 @@ static void update_global_window(void *args, gpr_uint32 id, void *stream) {
   stream_global->outgoing_window += t->parsing.initial_window_update;
 }
 
+static void read_error_locked(grpc_chttp2_transport *t) {
+  t->endpoint_reading = 0;
+  if (!t->writing_active && t->ep) {
+    grpc_endpoint_destroy(t->ep);
+    t->ep = NULL;
+    /* safe as we still have a ref for read */
+    UNREF_TRANSPORT(t, "disconnect");
+  }
+}
+
 /* tcp read callback */
 static void recv_data(void *tp, gpr_slice *slices, size_t nslices,
                       grpc_endpoint_cb_status error) {
   grpc_chttp2_transport *t = tp;
   size_t i;
+  int unref = 0;
 
   switch (error) {
     case GRPC_ENDPOINT_CB_SHUTDOWN:
@@ -877,15 +887,9 @@ static void recv_data(void *tp, gpr_slice *slices, size_t nslices,
     case GRPC_ENDPOINT_CB_ERROR:
       lock(t);
       drop_connection(t);
-      t->endpoint_reading = 0;
-      if (!t->writing_active && t->ep) {
-        grpc_endpoint_destroy(t->ep);
-        t->ep = NULL;
-        UNREF_TRANSPORT(
-            t, "disconnect"); /* safe as we still have a ref for read */
-      }
+      read_error_locked(t);
       unlock(t);
-      UNREF_TRANSPORT(t, "recv_data");
+      unref = 1;
       for (i = 0; i < nslices; i++) gpr_slice_unref(slices[i]);
       break;
     case GRPC_ENDPOINT_CB_OK:
@@ -922,11 +926,17 @@ static void recv_data(void *tp, gpr_slice *slices, size_t nslices,
       }
       if (i == nslices) {
         grpc_chttp2_schedule_closure(&t->global, &t->reading_action, 1);
+      } else {
+        read_error_locked(t);
+        unref = 1;
       }
       unlock(t);
       for (; i < nslices; i++) gpr_slice_unref(slices[i]);
       break;
   }
+  if (unref) {
+    UNREF_TRANSPORT(t, "recv_data");
+  }
 }
 
 static void reading_action(void *pt, int iomgr_success_ignored) {
diff --git a/src/core/transport/metadata.c b/src/core/transport/metadata.c
index e75b449e12..c80d67823f 100644
--- a/src/core/transport/metadata.c
+++ b/src/core/transport/metadata.c
@@ -120,7 +120,7 @@ static void unlock(grpc_mdctx *ctx) {
   if (ctx->refs == 0) {
     /* uncomment if you're having trouble diagnosing an mdelem leak to make
        things clearer (slows down destruction a lot, however) */
-    /* gc_mdtab(ctx); */
+    gc_mdtab(ctx);
     if (ctx->mdtab_count && ctx->mdtab_count == ctx->mdtab_free) {
       discard_metadata(ctx);
     }
diff --git a/src/core/transport/transport.c b/src/core/transport/transport.c
index 1397e21933..fe565944ed 100644
--- a/src/core/transport/transport.c
+++ b/src/core/transport/transport.c
@@ -83,8 +83,8 @@ void grpc_transport_stream_op_add_cancellation(grpc_transport_stream_op *op,
                                                grpc_mdstr *message) {
   if (op->cancel_with_status == GRPC_STATUS_OK) {
     op->cancel_with_status = status;
-    op->cancel_message = message;
-  } else if (message) {
+  }
+  if (message) {
     grpc_mdstr_unref(message);
   }
 }
diff --git a/src/core/transport/transport.h b/src/core/transport/transport.h
index 599edc871f..579bcc943f 100644
--- a/src/core/transport/transport.h
+++ b/src/core/transport/transport.h
@@ -77,7 +77,6 @@ typedef struct grpc_transport_stream_op {
   grpc_pollset *bind_pollset;
 
   grpc_status_code cancel_with_status;
-  grpc_mdstr *cancel_message;
 
   /* Indexes correspond to grpc_context_index enum values */
   grpc_call_context_element *context;
diff --git a/src/core/transport/transport_op_string.c b/src/core/transport/transport_op_string.c
index 43e14a7792..516aa9d4d8 100644
--- a/src/core/transport/transport_op_string.c
+++ b/src/core/transport/transport_op_string.c
@@ -144,11 +144,6 @@ char *grpc_transport_stream_op_string(grpc_transport_stream_op *op) {
     first = 0;
     gpr_asprintf(&tmp, "CANCEL:%d", op->cancel_with_status);
     gpr_strvec_add(&b, tmp);
-    if (op->cancel_message) {
-      gpr_asprintf(&tmp, ";msg='%s'",
-                   grpc_mdstr_as_c_string(op->cancel_message));
-      gpr_strvec_add(&b, tmp);
-    }
   }
 
   out = gpr_strvec_flatten(&b, NULL);
diff --git a/test/core/bad_client/bad_client.c b/test/core/bad_client/bad_client.c
index 0b0a682607..9b6a246075 100644
--- a/test/core/bad_client/bad_client.c
+++ b/test/core/bad_client/bad_client.c
@@ -41,6 +41,7 @@
 #include "src/core/support/string.h"
 #include "src/core/transport/chttp2_transport.h"
 
+#include <grpc/support/alloc.h>
 #include <grpc/support/sync.h>
 #include <grpc/support/thd.h>
 
@@ -91,6 +92,8 @@ void grpc_run_bad_client_test(grpc_bad_client_server_side_validator validator,
   /* Add a debug log */
   gpr_log(GPR_INFO, "TEST: %s", hex);
 
+  gpr_free(hex);
+
   /* Init grpc */
   grpc_init();
 
diff --git a/test/core/client_config/uri_parser_test.c b/test/core/client_config/uri_parser_test.c
index 287a9d1add..e5f9017ce0 100644
--- a/test/core/client_config/uri_parser_test.c
+++ b/test/core/client_config/uri_parser_test.c
@@ -58,7 +58,12 @@ int main(int argc, char **argv) {
   test_succeeds("http://www.google.com", "http", "www.google.com", "");
   test_succeeds("dns:///foo", "dns", "", "/foo");
   test_succeeds("http://www.google.com:90", "http", "www.google.com:90", "");
+  test_succeeds("a192.4-df:foo.coom", "a192.4-df", "", "foo.coom");
+  test_succeeds("a+b:foo.coom", "a+b", "", "foo.coom");
   test_fails("xyz");
   test_fails("http://www.google.com?why-are-you-using-queries");
+  test_fails("dns:foo.com#fragments-arent-supported-here");
+  test_fails("http:?huh");
+  test_fails("unix:#yeah-right");
   return 0;
 }
diff --git a/test/core/security/auth_context_test.c b/test/core/security/auth_context_test.c
index 2fb0c01f6f..2b12551a06 100644
--- a/test/core/security/auth_context_test.c
+++ b/test/core/security/auth_context_test.c
@@ -52,7 +52,7 @@ static void test_empty_context(void) {
   GPR_ASSERT(grpc_auth_property_iterator_next(&it) == NULL);
   it = grpc_auth_context_find_properties_by_name(ctx, "foo");
   GPR_ASSERT(grpc_auth_property_iterator_next(&it) == NULL);
-  grpc_auth_context_unref(ctx);
+  GRPC_AUTH_CONTEXT_UNREF(ctx, "test");
 }
 
 static void test_simple_context(void) {
@@ -86,7 +86,7 @@ static void test_simple_context(void) {
   GPR_ASSERT(grpc_auth_property_iterator_next(&it) == &ctx->properties[1]);
   GPR_ASSERT(grpc_auth_property_iterator_next(&it) == NULL);
 
-  grpc_auth_context_unref(ctx);
+  GRPC_AUTH_CONTEXT_UNREF(ctx, "test");
 }
 
 static void test_chained_context(void) {
@@ -96,7 +96,7 @@ static void test_chained_context(void) {
   size_t i;
 
   gpr_log(GPR_INFO, "test_chained_context");
-  grpc_auth_context_unref(chained);
+  GRPC_AUTH_CONTEXT_UNREF(chained, "chained");
   chained->properties[0] =
       grpc_auth_property_init_from_cstring("name", "padapo");
   chained->properties[1] = grpc_auth_property_init_from_cstring("foo", "baz");
@@ -129,7 +129,7 @@ static void test_chained_context(void) {
   GPR_ASSERT(grpc_auth_property_iterator_next(&it) == &chained->properties[0]);
   GPR_ASSERT(grpc_auth_property_iterator_next(&it) == NULL);
 
-  grpc_auth_context_unref(ctx);
+  GRPC_AUTH_CONTEXT_UNREF(ctx, "test");
 }
 
 
diff --git a/test/core/security/security_connector_test.c b/test/core/security/security_connector_test.c
index 4ad8beb727..b37fd7213d 100644
--- a/test/core/security/security_connector_test.c
+++ b/test/core/security/security_connector_test.c
@@ -73,7 +73,7 @@ static void test_unauthenticated_ssl_peer(void) {
   GPR_ASSERT(check_transport_security_type(ctx));
 
   tsi_peer_destruct(&peer);
-  grpc_auth_context_unref(ctx);
+  GRPC_AUTH_CONTEXT_UNREF(ctx, "test");
 }
 
 static int check_identity(const grpc_auth_context *ctx,
@@ -145,7 +145,7 @@ static void test_cn_only_ssl_peer_to_auth_context(void) {
   GPR_ASSERT(check_x509_cn(ctx, expected_cn));
 
   tsi_peer_destruct(&peer);
-  grpc_auth_context_unref(ctx);
+  GRPC_AUTH_CONTEXT_UNREF(ctx, "test");
 }
 
 static void test_cn_and_one_san_ssl_peer_to_auth_context(void) {
@@ -171,7 +171,7 @@ static void test_cn_and_one_san_ssl_peer_to_auth_context(void) {
   GPR_ASSERT(check_x509_cn(ctx, expected_cn));
 
   tsi_peer_destruct(&peer);
-  grpc_auth_context_unref(ctx);
+  GRPC_AUTH_CONTEXT_UNREF(ctx, "test");
 }
 
 static void test_cn_and_multiple_sans_ssl_peer_to_auth_context(void) {
@@ -202,7 +202,7 @@ static void test_cn_and_multiple_sans_ssl_peer_to_auth_context(void) {
   GPR_ASSERT(check_x509_cn(ctx, expected_cn));
 
   tsi_peer_destruct(&peer);
-  grpc_auth_context_unref(ctx);
+  GRPC_AUTH_CONTEXT_UNREF(ctx, "test");
 }
 
 static void test_cn_and_multiple_sans_and_others_ssl_peer_to_auth_context(
@@ -238,8 +238,7 @@ static void test_cn_and_multiple_sans_and_others_ssl_peer_to_auth_context(
   GPR_ASSERT(check_x509_cn(ctx, expected_cn));
 
   tsi_peer_destruct(&peer);
-  grpc_auth_context_unref(ctx);
-
+  GRPC_AUTH_CONTEXT_UNREF(ctx, "test");
 }
 
 int main(int argc, char **argv) {