From bc85be10ef420f35d660bc12afb47580acec3143 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 24 Aug 2015 10:36:39 -0700 Subject: [PATCH 01/21] Refactor default host name resolution Rephrase API's such that resolvers are constrained to be able to provide a default host given just the text of the URI channel target. This avoids needing to rewrite such details in the core library during retries, and generally makes things much saner to debug. --- src/core/channel/http_client_filter.c | 33 -------- src/core/client_config/resolver_factory.c | 6 ++ src/core/client_config/resolver_factory.h | 9 +++ src/core/client_config/resolver_registry.c | 79 ++++++++++--------- src/core/client_config/resolver_registry.h | 15 ++-- .../client_config/resolvers/dns_resolver.c | 25 +++--- .../resolvers/sockaddr_resolver.c | 49 ++++++++---- .../resolvers/zookeeper_resolver.c | 11 ++- src/core/surface/channel.c | 24 ++++++ src/core/surface/init.c | 8 +- test/core/end2end/tests/default_host.c | 2 +- 11 files changed, 148 insertions(+), 113 deletions(-) diff --git a/src/core/channel/http_client_filter.c b/src/core/channel/http_client_filter.c index 48c623d3591..2b61d33c29d 100644 --- a/src/core/channel/http_client_filter.c +++ b/src/core/channel/http_client_filter.c @@ -45,7 +45,6 @@ typedef struct call_data { grpc_linked_mdelem content_type; grpc_linked_mdelem user_agent; int sent_initial_metadata; - int sent_authority; int got_initial_metadata; grpc_stream_op_buffer *recv_ops; @@ -64,7 +63,6 @@ typedef struct channel_data { grpc_mdelem *scheme; grpc_mdelem *content_type; grpc_mdelem *status; - grpc_mdelem *default_authority; /** complete user agent mdelem */ grpc_mdelem *user_agent; } channel_data; @@ -103,7 +101,6 @@ static void hc_on_recv(void *user_data, int success) { static grpc_mdelem *client_strip_filter(void *user_data, grpc_mdelem *md) { grpc_call_element *elem = user_data; - call_data *calld = elem->call_data; channel_data *channeld = elem->channel_data; /* eat the things we'd like to set ourselves */ if (md->key == channeld->method->key) return NULL; @@ -111,10 +108,6 @@ static grpc_mdelem *client_strip_filter(void *user_data, grpc_mdelem *md) { if (md->key == channeld->te_trailers->key) return NULL; if (md->key == channeld->content_type->key) return NULL; if (md->key == channeld->user_agent->key) return NULL; - if (channeld->default_authority && - channeld->default_authority->key == md->key) { - calld->sent_authority = 1; - } return md; } @@ -138,11 +131,6 @@ static void hc_mutate_op(grpc_call_element *elem, GRPC_MDELEM_REF(channeld->method)); grpc_metadata_batch_add_head(&op->data.metadata, &calld->scheme, GRPC_MDELEM_REF(channeld->scheme)); - if (channeld->default_authority && !calld->sent_authority) { - grpc_metadata_batch_add_head( - &op->data.metadata, &calld->authority, - GRPC_MDELEM_REF(channeld->default_authority)); - } grpc_metadata_batch_add_tail(&op->data.metadata, &calld->te_trailers, GRPC_MDELEM_REF(channeld->te_trailers)); grpc_metadata_batch_add_tail(&op->data.metadata, &calld->content_type, @@ -175,7 +163,6 @@ static void init_call_elem(grpc_call_element *elem, call_data *calld = elem->call_data; calld->sent_initial_metadata = 0; calld->got_initial_metadata = 0; - calld->sent_authority = 0; calld->on_done_recv = NULL; grpc_iomgr_closure_init(&calld->hc_on_recv, hc_on_recv, elem); if (initial_op) hc_mutate_op(elem, initial_op); @@ -257,8 +244,6 @@ static grpc_mdstr *user_agent_from_args(grpc_mdctx *mdctx, static void init_channel_elem(grpc_channel_element *elem, grpc_channel *master, const grpc_channel_args *channel_args, grpc_mdctx *mdctx, int is_first, int is_last) { - size_t i; - /* grab pointers to our data from the channel element */ channel_data *channeld = elem->channel_data; @@ -267,21 +252,6 @@ static void init_channel_elem(grpc_channel_element *elem, grpc_channel *master, path */ GPR_ASSERT(!is_last); - channeld->default_authority = NULL; - if (channel_args) { - for (i = 0; i < channel_args->num_args; i++) { - if (0 == strcmp(channel_args->args[i].key, GRPC_ARG_DEFAULT_AUTHORITY)) { - if (channel_args->args[i].type != GRPC_ARG_STRING) { - gpr_log(GPR_ERROR, "%s: must be an string", - GRPC_ARG_DEFAULT_AUTHORITY); - } else { - channeld->default_authority = grpc_mdelem_from_strings( - mdctx, ":authority", channel_args->args[i].value.string); - } - } - } - } - /* initialize members */ channeld->te_trailers = grpc_mdelem_from_strings(mdctx, "te", "trailers"); channeld->method = grpc_mdelem_from_strings(mdctx, ":method", "POST"); @@ -306,9 +276,6 @@ static void destroy_channel_elem(grpc_channel_element *elem) { GRPC_MDELEM_UNREF(channeld->content_type); GRPC_MDELEM_UNREF(channeld->status); GRPC_MDELEM_UNREF(channeld->user_agent); - if (channeld->default_authority) { - GRPC_MDELEM_UNREF(channeld->default_authority); - } } const grpc_channel_filter grpc_http_client_filter = { diff --git a/src/core/client_config/resolver_factory.c b/src/core/client_config/resolver_factory.c index 6721977e21a..bf631cefd13 100644 --- a/src/core/client_config/resolver_factory.c +++ b/src/core/client_config/resolver_factory.c @@ -48,3 +48,9 @@ grpc_resolver *grpc_resolver_factory_create_resolver( if (!factory) return NULL; return factory->vtable->create_resolver(factory, uri, subchannel_factory); } + +char *grpc_resolver_factory_get_default_authority( + grpc_resolver_factory *factory, grpc_uri *uri) { + if (!factory) return NULL; + return factory->vtable->get_default_authority(factory, uri); +} diff --git a/src/core/client_config/resolver_factory.h b/src/core/client_config/resolver_factory.h index c5d85499c6e..73a05642303 100644 --- a/src/core/client_config/resolver_factory.h +++ b/src/core/client_config/resolver_factory.h @@ -54,6 +54,10 @@ struct grpc_resolver_factory_vtable { grpc_resolver *(*create_resolver)( grpc_resolver_factory *factory, grpc_uri *uri, grpc_subchannel_factory *subchannel_factory); + + char *(*get_default_authority)(grpc_resolver_factory *factory, grpc_uri *uri); + + const char *scheme; }; void grpc_resolver_factory_ref(grpc_resolver_factory *resolver); @@ -64,4 +68,9 @@ grpc_resolver *grpc_resolver_factory_create_resolver( grpc_resolver_factory *factory, grpc_uri *uri, grpc_subchannel_factory *subchannel_factory); +/** Return a (freshly allocated with gpr_malloc) string representing + the default authority to use for this scheme. */ +char *grpc_resolver_factory_get_default_authority( + grpc_resolver_factory *factory, grpc_uri *uri); + #endif /* GRPC_INTERNAL_CORE_CONFIG_RESOLVER_FACTORY_H */ diff --git a/src/core/client_config/resolver_registry.c b/src/core/client_config/resolver_registry.c index 16be2da9940..3aff4630fba 100644 --- a/src/core/client_config/resolver_registry.c +++ b/src/core/client_config/resolver_registry.c @@ -41,41 +41,33 @@ #define MAX_RESOLVERS 10 -typedef struct { - char *scheme; - grpc_resolver_factory *factory; -} registered_resolver; - -static registered_resolver g_all_of_the_resolvers[MAX_RESOLVERS]; +static grpc_resolver_factory *g_all_of_the_resolvers[MAX_RESOLVERS]; static int g_number_of_resolvers = 0; -static char *g_default_resolver_scheme; +static char *g_default_resolver_prefix; -void grpc_resolver_registry_init(const char *default_resolver_scheme) { +void grpc_resolver_registry_init(const char *default_resolver_prefix) { g_number_of_resolvers = 0; - g_default_resolver_scheme = gpr_strdup(default_resolver_scheme); + g_default_resolver_prefix = gpr_strdup(default_resolver_prefix); } void grpc_resolver_registry_shutdown(void) { int i; for (i = 0; i < g_number_of_resolvers; i++) { - gpr_free(g_all_of_the_resolvers[i].scheme); - grpc_resolver_factory_unref(g_all_of_the_resolvers[i].factory); + grpc_resolver_factory_unref(g_all_of_the_resolvers[i]); } - gpr_free(g_default_resolver_scheme); + gpr_free(g_default_resolver_prefix); } -void grpc_register_resolver_type(const char *scheme, - grpc_resolver_factory *factory) { +void grpc_register_resolver_type(grpc_resolver_factory *factory) { int i; for (i = 0; i < g_number_of_resolvers; i++) { - GPR_ASSERT(0 != strcmp(scheme, g_all_of_the_resolvers[i].scheme)); + GPR_ASSERT(0 != strcmp(factory->vtable->scheme, + g_all_of_the_resolvers[i]->vtable->scheme)); } GPR_ASSERT(g_number_of_resolvers != MAX_RESOLVERS); - g_all_of_the_resolvers[g_number_of_resolvers].scheme = gpr_strdup(scheme); grpc_resolver_factory_ref(factory); - g_all_of_the_resolvers[g_number_of_resolvers].factory = factory; - g_number_of_resolvers++; + g_all_of_the_resolvers[g_number_of_resolvers++] = factory; } static grpc_resolver_factory *lookup_factory(grpc_uri *uri) { @@ -85,40 +77,53 @@ static grpc_resolver_factory *lookup_factory(grpc_uri *uri) { if (!uri) return NULL; for (i = 0; i < g_number_of_resolvers; i++) { - if (0 == strcmp(uri->scheme, g_all_of_the_resolvers[i].scheme)) { - return g_all_of_the_resolvers[i].factory; + if (0 == strcmp(uri->scheme, g_all_of_the_resolvers[i]->vtable->scheme)) { + return g_all_of_the_resolvers[i]; } } return NULL; } -grpc_resolver *grpc_resolver_create( - const char *name, grpc_subchannel_factory *subchannel_factory) { - grpc_uri *uri; +static grpc_resolver_factory *resolve_factory(const char *target, + grpc_uri **uri) { char *tmp; grpc_resolver_factory *factory = NULL; - grpc_resolver *resolver; - - uri = grpc_uri_parse(name, 1); - factory = lookup_factory(uri); - if (factory == NULL && g_default_resolver_scheme != NULL) { - grpc_uri_destroy(uri); - gpr_asprintf(&tmp, "%s%s", g_default_resolver_scheme, name); - uri = grpc_uri_parse(tmp, 1); - factory = lookup_factory(uri); + + *uri = grpc_uri_parse(target, 1); + factory = lookup_factory(*uri); + if (factory == NULL && g_default_resolver_prefix != NULL) { + grpc_uri_destroy(*uri); + gpr_asprintf(&tmp, "%s%s", g_default_resolver_prefix, target); + *uri = grpc_uri_parse(tmp, 1); + factory = lookup_factory(*uri); if (factory == NULL) { - grpc_uri_destroy(grpc_uri_parse(name, 0)); + grpc_uri_destroy(grpc_uri_parse(target, 0)); grpc_uri_destroy(grpc_uri_parse(tmp, 0)); - gpr_log(GPR_ERROR, "don't know how to resolve '%s' or '%s'", name, tmp); + gpr_log(GPR_ERROR, "don't know how to resolve '%s' or '%s'", target, tmp); } gpr_free(tmp); } else if (factory == NULL) { - grpc_uri_destroy(grpc_uri_parse(name, 0)); - gpr_log(GPR_ERROR, "don't know how to resolve '%s'", name); + grpc_uri_destroy(grpc_uri_parse(target, 0)); + gpr_log(GPR_ERROR, "don't know how to resolve '%s'", target); } - resolver = + return factory; +} + +grpc_resolver *grpc_resolver_create( + const char *target, grpc_subchannel_factory *subchannel_factory) { + grpc_uri *uri = NULL; + grpc_resolver_factory *factory = resolve_factory(target, &uri); + grpc_resolver *resolver = grpc_resolver_factory_create_resolver(factory, uri, subchannel_factory); grpc_uri_destroy(uri); return resolver; } + +char *grpc_get_default_authority(const char *target) { + grpc_uri *uri = NULL; + grpc_resolver_factory *factory = resolve_factory(target, &uri); + char *authority = grpc_resolver_factory_get_default_authority(factory, uri); + grpc_uri_destroy(uri); + return authority; +} diff --git a/src/core/client_config/resolver_registry.h b/src/core/client_config/resolver_registry.h index 31aa47620ab..5a7193b7ae2 100644 --- a/src/core/client_config/resolver_registry.h +++ b/src/core/client_config/resolver_registry.h @@ -44,19 +44,22 @@ void grpc_resolver_registry_shutdown(void); If \a priority is greater than zero, then the resolver will be eligible to resolve names that are passed in with no scheme. Higher priority resolvers will be tried before lower priority schemes. */ -void grpc_register_resolver_type(const char *scheme, - grpc_resolver_factory *factory); +void grpc_register_resolver_type(grpc_resolver_factory *factory); -/** Create a resolver given \a name. - First tries to parse \a name as a URI. If this succeeds, tries +/** Create a resolver given \a target. + First tries to parse \a target as a URI. If this succeeds, tries to locate a registered resolver factory based on the URI scheme. If parsing or location fails, prefixes default_prefix from - grpc_resolver_registry_init to name, and tries again (if default_prefix + grpc_resolver_registry_init to target, and tries again (if default_prefix was not NULL). If a resolver factory was found, use it to instantiate a resolver and return it. If a resolver factory was not found, return NULL. */ grpc_resolver *grpc_resolver_create( - const char *name, grpc_subchannel_factory *subchannel_factory); + const char *target, grpc_subchannel_factory *subchannel_factory); + +/** Given a target, return a (freshly allocated with gpr_malloc) string + representing the default authority to pass from a client. */ +char *grpc_get_default_authority(const char *target); #endif /* GRPC_INTERNAL_CORE_CLIENT_CONFIG_RESOLVER_REGISTRY_H */ diff --git a/src/core/client_config/resolvers/dns_resolver.c b/src/core/client_config/resolvers/dns_resolver.c index 7b35b7902f7..84643c464a9 100644 --- a/src/core/client_config/resolvers/dns_resolver.c +++ b/src/core/client_config/resolvers/dns_resolver.c @@ -203,9 +203,6 @@ static grpc_resolver *dns_create( grpc_subchannel_factory *subchannel_factory) { dns_resolver *r; const char *path = uri->path; - grpc_arg default_host_arg; - char *host; - char *port; if (0 != strcmp(uri->authority, "")) { gpr_log(GPR_ERROR, "authority based uri's not supported"); @@ -214,17 +211,6 @@ static grpc_resolver *dns_create( if (path[0] == '/') ++path; - gpr_split_host_port(path, &host, &port); - - default_host_arg.type = GRPC_ARG_STRING; - default_host_arg.key = GRPC_ARG_DEFAULT_AUTHORITY; - default_host_arg.value.string = host; - subchannel_factory = grpc_subchannel_factory_add_channel_arg( - subchannel_factory, &default_host_arg); - - gpr_free(host); - gpr_free(port); - r = gpr_malloc(sizeof(dns_resolver)); memset(r, 0, sizeof(*r)); gpr_ref_init(&r->refs, 1); @@ -233,6 +219,7 @@ static grpc_resolver *dns_create( r->name = gpr_strdup(path); r->default_port = gpr_strdup(default_port); r->subchannel_factory = subchannel_factory; + grpc_subchannel_factory_ref(subchannel_factory); r->lb_policy_factory = lb_policy_factory; return &r->base; } @@ -252,8 +239,16 @@ static grpc_resolver *dns_factory_create_resolver( subchannel_factory); } +char *dns_factory_get_default_host_name(grpc_resolver_factory *factory, + grpc_uri *uri) { + const char *path = uri->path; + if (path[0] == '/') ++path; + return gpr_strdup(path); +} + static const grpc_resolver_factory_vtable dns_factory_vtable = { - dns_factory_ref, dns_factory_unref, dns_factory_create_resolver}; + dns_factory_ref, dns_factory_unref, dns_factory_create_resolver, + dns_factory_get_default_host_name, "dns"}; static grpc_resolver_factory dns_resolver_factory = {&dns_factory_vtable}; grpc_resolver_factory *grpc_dns_resolver_factory_create() { diff --git a/src/core/client_config/resolvers/sockaddr_resolver.c b/src/core/client_config/resolvers/sockaddr_resolver.c index 74584e7e2c6..eeafef4174e 100644 --- a/src/core/client_config/resolvers/sockaddr_resolver.c +++ b/src/core/client_config/resolvers/sockaddr_resolver.c @@ -156,8 +156,29 @@ static int parse_unix(grpc_uri *uri, struct sockaddr_storage *addr, int *len) { return 1; } + +static char *unix_get_default_authority(grpc_resolver_factory *factory, + grpc_uri *uri) { + return gpr_strdup("localhost"); +} #endif +static char *ip_get_default_authority(grpc_uri *uri) { + const char *path = uri->path; + if (path[0] == '/') ++path; + return gpr_strdup(path); +} + +static char *ipv4_get_default_authority(grpc_resolver_factory *factory, + grpc_uri *uri) { + return ip_get_default_authority(uri); +} + +static char *ipv6_get_default_authority(grpc_resolver_factory *factory, + grpc_uri *uri) { + return ip_get_default_authority(uri); +} + static int parse_ipv4(grpc_uri *uri, struct sockaddr_storage *addr, int *len) { const char *host_port = uri->path; char *host; @@ -276,20 +297,20 @@ static void sockaddr_factory_ref(grpc_resolver_factory *factory) {} static void sockaddr_factory_unref(grpc_resolver_factory *factory) {} -#define DECL_FACTORY(name) \ - static grpc_resolver *name##_factory_create_resolver( \ - grpc_resolver_factory *factory, grpc_uri *uri, \ - grpc_subchannel_factory *subchannel_factory) { \ - return sockaddr_create(uri, grpc_create_pick_first_lb_policy, \ - subchannel_factory, parse_##name); \ - } \ - static const grpc_resolver_factory_vtable name##_factory_vtable = { \ - sockaddr_factory_ref, sockaddr_factory_unref, \ - name##_factory_create_resolver}; \ - static grpc_resolver_factory name##_resolver_factory = { \ - &name##_factory_vtable}; \ - grpc_resolver_factory *grpc_##name##_resolver_factory_create() { \ - return &name##_resolver_factory; \ +#define DECL_FACTORY(name) \ + static grpc_resolver *name##_factory_create_resolver( \ + grpc_resolver_factory *factory, grpc_uri *uri, \ + grpc_subchannel_factory *subchannel_factory) { \ + return sockaddr_create(uri, grpc_create_pick_first_lb_policy, \ + subchannel_factory, parse_##name); \ + } \ + static const grpc_resolver_factory_vtable name##_factory_vtable = { \ + sockaddr_factory_ref, sockaddr_factory_unref, \ + name##_factory_create_resolver, name##_get_default_authority, #name}; \ + static grpc_resolver_factory name##_resolver_factory = { \ + &name##_factory_vtable}; \ + grpc_resolver_factory *grpc_##name##_resolver_factory_create() { \ + return &name##_resolver_factory; \ } #ifdef GPR_POSIX_SOCKET diff --git a/src/core/client_config/resolvers/zookeeper_resolver.c b/src/core/client_config/resolvers/zookeeper_resolver.c index acb2ba136e9..da399f99548 100644 --- a/src/core/client_config/resolvers/zookeeper_resolver.c +++ b/src/core/client_config/resolvers/zookeeper_resolver.c @@ -467,8 +467,7 @@ static grpc_resolver *zookeeper_create( } static void zookeeper_plugin_init() { - grpc_register_resolver_type("zookeeper", - grpc_zookeeper_resolver_factory_create()); + grpc_register_resolver_type(grpc_zookeeper_resolver_factory_create()); } void grpc_zookeeper_register() { @@ -483,6 +482,11 @@ static void zookeeper_factory_ref(grpc_resolver_factory *factory) {} static void zookeeper_factory_unref(grpc_resolver_factory *factory) {} +static char *zookeeper_factory_get_default_hostname( + grpc_resolver_factory *factory, grpc_uri *uri) { + return NULL; +} + static grpc_resolver *zookeeper_factory_create_resolver( grpc_resolver_factory *factory, grpc_uri *uri, grpc_subchannel_factory *subchannel_factory) { @@ -492,7 +496,8 @@ static grpc_resolver *zookeeper_factory_create_resolver( static const grpc_resolver_factory_vtable zookeeper_factory_vtable = { zookeeper_factory_ref, zookeeper_factory_unref, - zookeeper_factory_create_resolver}; + zookeeper_factory_create_resolver, zookeeper_factory_get_default_hostname, + "zookeeper"}; static grpc_resolver_factory zookeeper_resolver_factory = { &zookeeper_factory_vtable}; diff --git a/src/core/surface/channel.c b/src/core/surface/channel.c index e50251566d5..7378929ca4d 100644 --- a/src/core/surface/channel.c +++ b/src/core/surface/channel.c @@ -40,6 +40,7 @@ #include #include +#include "src/core/client_config/resolver_registry.h" #include "src/core/iomgr/iomgr.h" #include "src/core/support/string.h" #include "src/core/surface/call.h" @@ -70,6 +71,7 @@ struct grpc_channel { grpc_mdstr *grpc_message_string; grpc_mdstr *path_string; grpc_mdstr *authority_string; + grpc_mdelem *default_authority; /** mdelem for grpc-status: 0 thru grpc-status: 2 */ grpc_mdelem *grpc_status_elem[NUM_CACHED_STATUS_ELEMS]; @@ -134,10 +136,27 @@ grpc_channel *grpc_channel_create_from_filters( } else { channel->max_message_length = args->args[i].value.integer; } + } else if (0 == strcmp(args->args[i].key, GRPC_ARG_DEFAULT_AUTHORITY)) { + if (args->args[i].type != GRPC_ARG_STRING) { + gpr_log(GPR_ERROR, "%s: must be an string", + GRPC_ARG_DEFAULT_AUTHORITY); + } else { + channel->default_authority = grpc_mdelem_from_strings( + mdctx, ":authority", args->args[i].value.string); + } } } } + if (channel->is_client && channel->default_authority == NULL) { + char *default_authority = grpc_get_default_authority(target); + if (default_authority) { + channel->default_authority = grpc_mdelem_from_strings( + channel->metadata_context, ":authority", default_authority); + } + gpr_free(default_authority); + } + grpc_channel_stack_init(filters, num_filters, channel, args, channel->metadata_context, CHANNEL_STACK_FROM_CHANNEL(channel)); @@ -161,6 +180,8 @@ static grpc_call *grpc_channel_create_call_internal( send_metadata[num_metadata++] = path_mdelem; if (authority_mdelem != NULL) { send_metadata[num_metadata++] = authority_mdelem; + } else if (channel->default_authority != NULL) { + send_metadata[num_metadata++] = GRPC_MDELEM_REF(channel->default_authority); } return grpc_call_create(channel, parent_call, propagation_mask, cq, NULL, @@ -251,6 +272,9 @@ static void destroy_channel(void *p, int ok) { } gpr_free(rc); } + if (channel->default_authority != NULL) { + GRPC_MDELEM_UNREF(channel->default_authority); + } grpc_mdctx_unref(channel->metadata_context); gpr_mu_destroy(&channel->registered_call_mu); gpr_free(channel->target); diff --git a/src/core/surface/init.c b/src/core/surface/init.c index d9044549f21..0d48cd42d72 100644 --- a/src/core/surface/init.c +++ b/src/core/surface/init.c @@ -86,11 +86,11 @@ void grpc_init(void) { if (++g_initializations == 1) { gpr_time_init(); grpc_resolver_registry_init("dns:///"); - grpc_register_resolver_type("dns", grpc_dns_resolver_factory_create()); - grpc_register_resolver_type("ipv4", grpc_ipv4_resolver_factory_create()); - grpc_register_resolver_type("ipv6", grpc_ipv6_resolver_factory_create()); + grpc_register_resolver_type(grpc_dns_resolver_factory_create()); + grpc_register_resolver_type(grpc_ipv4_resolver_factory_create()); + grpc_register_resolver_type(grpc_ipv6_resolver_factory_create()); #ifdef GPR_POSIX_SOCKET - grpc_register_resolver_type("unix", grpc_unix_resolver_factory_create()); + grpc_register_resolver_type(grpc_unix_resolver_factory_create()); #endif grpc_register_tracer("channel", &grpc_trace_channel); grpc_register_tracer("surface", &grpc_surface_trace); diff --git a/test/core/end2end/tests/default_host.c b/test/core/end2end/tests/default_host.c index 97c19db331d..57f65b834b0 100644 --- a/test/core/end2end/tests/default_host.c +++ b/test/core/end2end/tests/default_host.c @@ -201,7 +201,7 @@ static void simple_request_body(grpc_end2end_test_fixture f) { GPR_ASSERT(status == GRPC_STATUS_UNIMPLEMENTED); GPR_ASSERT(0 == strcmp(details, "xyz")); GPR_ASSERT(0 == strcmp(call_details.method, "/foo")); - GPR_ASSERT(0 == strcmp(call_details.host, "localhost")); + GPR_ASSERT(0 == strncmp(call_details.host, "localhost", 9)); GPR_ASSERT(was_cancelled == 1); gpr_free(details); From 25bf313c5f1cf620879232d864c69b25c39e3f4c Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 24 Aug 2015 12:35:28 -0700 Subject: [PATCH 02/21] Fix for lame clients --- src/core/surface/channel.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/surface/channel.c b/src/core/surface/channel.c index 7378929ca4d..78eeed1f598 100644 --- a/src/core/surface/channel.c +++ b/src/core/surface/channel.c @@ -148,7 +148,8 @@ grpc_channel *grpc_channel_create_from_filters( } } - if (channel->is_client && channel->default_authority == NULL) { + if (channel->is_client && channel->default_authority == NULL && + target != NULL) { char *default_authority = grpc_get_default_authority(target); if (default_authority) { channel->default_authority = grpc_mdelem_from_strings( From a0f149e8732c58241795ba6ea198819e359437d2 Mon Sep 17 00:00:00 2001 From: Hongyu Chen Date: Mon, 24 Aug 2015 16:27:19 -0700 Subject: [PATCH 03/21] Move core/profiling/timers_preciseclock.h to core/support/ such that we have a gpr public function which returns cycle clock. --- BUILD | 5 +- build.json | 4 +- gRPC.podspec | 4 +- include/grpc/support/time.h | 3 + src/core/profiling/basic_timers.c | 9 ++- src/core/support/time_posix.c | 14 ++++- .../time_precise.h} | 57 +++++++++---------- src/core/support/time_win32.c | 4 ++ tools/doxygen/Doxyfile.core.internal | 2 +- tools/run_tests/sources_and_headers.json | 8 +-- tools/run_tests/tests.json | 1 + vsprojects/gpr/gpr.vcxproj | 1 + vsprojects/gpr/gpr.vcxproj.filters | 3 + vsprojects/grpc/grpc.vcxproj | 1 - vsprojects/grpc/grpc.vcxproj.filters | 3 - .../grpc_unsecure/grpc_unsecure.vcxproj | 1 - .../grpc_unsecure.vcxproj.filters | 3 - 17 files changed, 66 insertions(+), 57 deletions(-) rename src/core/{profiling/timers_preciseclock.h => support/time_precise.h} (65%) diff --git a/BUILD b/BUILD index 7eb59797d43..3ba9a6f050a 100644 --- a/BUILD +++ b/BUILD @@ -51,6 +51,7 @@ cc_library( "src/core/support/string.h", "src/core/support/string_win32.h", "src/core/support/thd_internal.h", + "src/core/support/time_precise.h", "src/core/support/alloc.c", "src/core/support/cmdline.c", "src/core/support/cpu_iphone.c", @@ -208,7 +209,6 @@ cc_library( "src/core/json/json_reader.h", "src/core/json/json_writer.h", "src/core/profiling/timers.h", - "src/core/profiling/timers_preciseclock.h", "src/core/surface/byte_buffer_queue.h", "src/core/surface/call.h", "src/core/surface/channel.h", @@ -474,7 +474,6 @@ cc_library( "src/core/json/json_reader.h", "src/core/json/json_writer.h", "src/core/profiling/timers.h", - "src/core/profiling/timers_preciseclock.h", "src/core/surface/byte_buffer_queue.h", "src/core/surface/call.h", "src/core/surface/channel.h", @@ -982,6 +981,7 @@ objc_library( "src/core/support/string.h", "src/core/support/string_win32.h", "src/core/support/thd_internal.h", + "src/core/support/time_precise.h", ], includes = [ "include", @@ -1221,7 +1221,6 @@ objc_library( "src/core/json/json_reader.h", "src/core/json/json_writer.h", "src/core/profiling/timers.h", - "src/core/profiling/timers_preciseclock.h", "src/core/surface/byte_buffer_queue.h", "src/core/surface/call.h", "src/core/surface/channel.h", diff --git a/build.json b/build.json index bd707d2e34c..d9ebce9099b 100644 --- a/build.json +++ b/build.json @@ -179,7 +179,6 @@ "src/core/json/json_reader.h", "src/core/json/json_writer.h", "src/core/profiling/timers.h", - "src/core/profiling/timers_preciseclock.h", "src/core/surface/byte_buffer_queue.h", "src/core/surface/call.h", "src/core/surface/channel.h", @@ -399,7 +398,8 @@ "src/core/support/stack_lockfree.h", "src/core/support/string.h", "src/core/support/string_win32.h", - "src/core/support/thd_internal.h" + "src/core/support/thd_internal.h", + "src/core/support/time_precise.h" ], "src": [ "src/core/support/alloc.c", diff --git a/gRPC.podspec b/gRPC.podspec index d945c299b51..df0ba6d1acd 100644 --- a/gRPC.podspec +++ b/gRPC.podspec @@ -68,6 +68,7 @@ Pod::Spec.new do |s| 'src/core/support/grpc_string.h', 'src/core/support/string_win32.h', 'src/core/support/thd_internal.h', + 'src/core/support/time_precise.h', 'grpc/support/alloc.h', 'grpc/support/atm.h', 'grpc/support/atm_gcc_atomic.h', @@ -210,7 +211,6 @@ Pod::Spec.new do |s| 'src/core/json/json_reader.h', 'src/core/json/json_writer.h', 'src/core/profiling/timers.h', - 'src/core/profiling/timers_preciseclock.h', 'src/core/surface/byte_buffer_queue.h', 'src/core/surface/call.h', 'src/core/surface/channel.h', @@ -401,6 +401,7 @@ Pod::Spec.new do |s| 'src/core/support/string.h', 'src/core/support/string_win32.h', 'src/core/support/thd_internal.h', + 'src/core/support/time_precise.h', 'src/core/security/auth_filters.h', 'src/core/security/base64.h', 'src/core/security/credentials.h', @@ -479,7 +480,6 @@ Pod::Spec.new do |s| 'src/core/json/json_reader.h', 'src/core/json/json_writer.h', 'src/core/profiling/timers.h', - 'src/core/profiling/timers_preciseclock.h', 'src/core/surface/byte_buffer_queue.h', 'src/core/surface/call.h', 'src/core/surface/channel.h', diff --git a/include/grpc/support/time.h b/include/grpc/support/time.h index 4ef9c764592..f8ed893bc02 100644 --- a/include/grpc/support/time.h +++ b/include/grpc/support/time.h @@ -52,6 +52,9 @@ typedef enum { /* Realtime clock. May jump forwards or backwards. Settable by the system administrator. Has its epoch at 0:00:00 UTC 1 Jan 1970. */ GPR_CLOCK_REALTIME, + /* CPU cycle time obtained by rdtsc instruction on x86 platforms. Epoch + undefined. Degrades to GPR_CLOCK_REALTIME on other platforms. */ + GPR_CLOCK_PRECISE, /* Unmeasurable clock type: no base, created by taking the difference between two times */ GPR_TIMESPAN diff --git a/src/core/profiling/basic_timers.c b/src/core/profiling/basic_timers.c index ae37f584ebb..9892a161209 100644 --- a/src/core/profiling/basic_timers.c +++ b/src/core/profiling/basic_timers.c @@ -53,7 +53,7 @@ typedef enum { } marker_type; typedef struct grpc_timer_entry { - grpc_precise_clock tm; + gpr_timespec tm; int tag; const char* tagstr; marker_type type; @@ -71,9 +71,8 @@ static void log_report() { int i; for (i = 0; i < count; i++) { grpc_timer_entry* entry = &(log[i]); - printf("GRPC_LAT_PROF " GRPC_PRECISE_CLOCK_FORMAT - " %p %c %d(%s) %p %s %d\n", - GRPC_PRECISE_CLOCK_PRINTF_ARGS(&entry->tm), + printf("GRPC_LAT_PROF %ld.%09d %p %c %d(%s) %p %s %d\n", + entry->tm.tv_sec, entry->tm.tv_nsec, (void*)(gpr_intptr)gpr_thd_currentid(), entry->type, entry->tag, entry->tagstr, entry->id, entry->file, entry->line); } @@ -93,7 +92,7 @@ static void grpc_timers_log_add(int tag, const char* tagstr, marker_type type, entry = &log[count++]; - grpc_precise_clock_now(&entry->tm); + entry->tm = gpr_now(GPR_CLOCK_PRECISE); entry->tag = tag; entry->tagstr = tagstr; entry->type = type; diff --git a/src/core/support/time_posix.c b/src/core/support/time_posix.c index 841485c4b4a..a2744002437 100644 --- a/src/core/support/time_posix.c +++ b/src/core/support/time_posix.c @@ -32,6 +32,7 @@ */ #include +#include #ifdef GPR_POSIX_TIME @@ -66,8 +67,14 @@ void gpr_time_init(void) {} gpr_timespec gpr_now(gpr_clock_type clock) { struct timespec now; GPR_ASSERT(clock != GPR_TIMESPAN); - clock_gettime(clockid_for_gpr_clock[clock], &now); - return gpr_from_timespec(now, clock); + if (clock == GPR_CLOCK_PRECISE) { + gpr_timespec ret; + gpr_precise_clock_now(&ret); + return ret; + } else { + clock_gettime(clockid_for_gpr_clock[clock], &now); + return gpr_from_timespec(now, clock); + } } #else /* For some reason Apple's OSes haven't implemented clock_gettime. */ @@ -104,6 +111,9 @@ gpr_timespec gpr_now(gpr_clock_type clock) { now.tv_sec = now_dbl * 1e-9; now.tv_nsec = now_dbl - now.tv_sec * 1e9; break; + case GPR_CLOCK_PRECISE: + gpr_precise_clock_now(&now); + break; case GPR_TIMESPAN: abort(); } diff --git a/src/core/profiling/timers_preciseclock.h b/src/core/support/time_precise.h similarity index 65% rename from src/core/profiling/timers_preciseclock.h rename to src/core/support/time_precise.h index 5c251b47e6f..d40ad12d0c3 100644 --- a/src/core/profiling/timers_preciseclock.h +++ b/src/core/support/time_precise.h @@ -31,65 +31,64 @@ * */ -#ifndef GRPC_CORE_PROFILING_TIMERS_PRECISECLOCK_H -#define GRPC_CORE_PROFILING_TIMERS_PRECISECLOCK_H +#ifndef GRPC_CORE_SUPPORT_TIME_PRECISE_H_ +#define GRPC_CORE_SUPPORT_TIME_PRECISE_H_ #include #include #include #ifdef GRPC_TIMERS_RDTSC -typedef long long int grpc_precise_clock; #if defined(__i386__) -static void grpc_precise_clock_now(grpc_precise_clock *clk) { - grpc_precise_clock ret; +static void gpr_get_cycle_counter(long long int *clk) { + long long int ret; __asm__ volatile("rdtsc" : "=A"(ret)); *clk = ret; } // ---------------------------------------------------------------- #elif defined(__x86_64__) || defined(__amd64__) -static void grpc_precise_clock_now(grpc_precise_clock *clk) { +static void gpr_get_cycle_counter(long long int *clk) { unsigned long long low, high; __asm__ volatile("rdtsc" : "=a"(low), "=d"(high)); *clk = (high << 32) | low; } #endif + static gpr_once precise_clock_init = GPR_ONCE_INIT; -static double cycles_per_second = 0.0; -static void grpc_precise_clock_init() { +static long long cycles_per_second = 0; +static void gpr_precise_clock_init() { time_t start = time(NULL); - grpc_precise_clock start_time; - grpc_precise_clock end_time; + gpr_precise_clock start_cycle; + gpr_precise_clock end_cycle; while (time(NULL) == start) ; - grpc_precise_clock_now(&start_time); + gpr_precise_clock_now(); + gpr_get_cycle_counter(&start_cycle); while (time(NULL) == start + 1) ; - grpc_precise_clock_now(&end_time); - cycles_per_second = end_time - start_time; + gpr_get_cycle_counter(&end_cycle); + cycles_per_second = end_cycle - start_cycle; } + static double grpc_precise_clock_scaling_factor() { gpr_once_init(&precise_clock_init, grpc_precise_clock_init); return 1e6 / cycles_per_second; } -#define GRPC_PRECISE_CLOCK_FORMAT "%f" -#define GRPC_PRECISE_CLOCK_PRINTF_ARGS(clk) \ - (*(clk)*grpc_precise_clock_scaling_factor()) -#else -typedef struct grpc_precise_clock grpc_precise_clock; -struct grpc_precise_clock { - gpr_timespec clock; -}; -static void grpc_precise_clock_now(grpc_precise_clock* clk) { - clk->clock = gpr_now(GPR_CLOCK_REALTIME); + +static void gpr_precise_clock_now(gpr_timespec *clk) { + long long int counter; + gpr_get_cycle_counter(&counter); + clk->clock = GPR_CLOCK_REALTIME; + clk->tv_sec = counter / cycles_per_second; + clk->tv_nsec = counter % cycles_per_second; } -#define GRPC_PRECISE_CLOCK_FORMAT "%ld.%09d" -#define GRPC_PRECISE_CLOCK_PRINTF_ARGS(clk) \ - (clk)->clock.tv_sec, (clk)->clock.tv_nsec -static void grpc_precise_clock_print(const grpc_precise_clock* clk, FILE* fp) { - fprintf(fp, "%ld.%09d", clk->clock.tv_sec, clk->clock.tv_nsec); + +#else /* GRPC_TIMERS_RDTSC */ +static void gpr_precise_clock_now(gpr_timespec *clk) { + *clk = gpr_now(GPR_CLOCK_REALTIME); + clk->clock_type = GPR_CLOCK_PRECISE; } #endif /* GRPC_TIMERS_RDTSC */ -#endif /* GRPC_CORE_PROFILING_TIMERS_PRECISECLOCK_H */ +#endif /* GRPC_CORE_SUPPORT_TIME_PRECISE_ */ diff --git a/src/core/support/time_win32.c b/src/core/support/time_win32.c index 7f64c80e276..f794855429b 100644 --- a/src/core/support/time_win32.c +++ b/src/core/support/time_win32.c @@ -38,6 +38,7 @@ #ifdef GPR_WIN32 #include +#include #include static LARGE_INTEGER g_start_time; @@ -68,6 +69,9 @@ gpr_timespec gpr_now(gpr_clock_type clock) { now_tv.tv_sec = (time_t)now_dbl; now_tv.tv_nsec = (int)((now_dbl - (double)now_tv.tv_sec) * 1e9); break; + case GPR_CLOCK_PRECISE: + gpr_precise_clock_now(&now_tv); + break; } return now_tv; } diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 325a293e04e..97f38057d09 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -845,7 +845,6 @@ src/core/json/json_common.h \ src/core/json/json_reader.h \ src/core/json/json_writer.h \ src/core/profiling/timers.h \ -src/core/profiling/timers_preciseclock.h \ src/core/surface/byte_buffer_queue.h \ src/core/surface/call.h \ src/core/surface/channel.h \ @@ -1055,6 +1054,7 @@ src/core/support/stack_lockfree.h \ src/core/support/string.h \ src/core/support/string_win32.h \ src/core/support/thd_internal.h \ +src/core/support/time_precise.h \ src/core/support/alloc.c \ src/core/support/cmdline.c \ src/core/support/cpu_iphone.c \ diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index 50f078586d1..de2d8699d1e 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -12165,7 +12165,8 @@ "src/core/support/stack_lockfree.h", "src/core/support/string.h", "src/core/support/string_win32.h", - "src/core/support/thd_internal.h" + "src/core/support/thd_internal.h", + "src/core/support/time_precise.h" ], "language": "c", "name": "gpr", @@ -12239,6 +12240,7 @@ "src/core/support/thd_win32.c", "src/core/support/time.c", "src/core/support/time_posix.c", + "src/core/support/time_precise.h", "src/core/support/time_win32.c", "src/core/support/tls_pthread.c" ] @@ -12336,7 +12338,6 @@ "src/core/json/json_reader.h", "src/core/json/json_writer.h", "src/core/profiling/timers.h", - "src/core/profiling/timers_preciseclock.h", "src/core/security/auth_filters.h", "src/core/security/base64.h", "src/core/security/credentials.h", @@ -12536,7 +12537,6 @@ "src/core/profiling/basic_timers.c", "src/core/profiling/stap_timers.c", "src/core/profiling/timers.h", - "src/core/profiling/timers_preciseclock.h", "src/core/security/auth_filters.h", "src/core/security/base64.c", "src/core/security/base64.h", @@ -12810,7 +12810,6 @@ "src/core/json/json_reader.h", "src/core/json/json_writer.h", "src/core/profiling/timers.h", - "src/core/profiling/timers_preciseclock.h", "src/core/surface/byte_buffer_queue.h", "src/core/surface/call.h", "src/core/surface/channel.h", @@ -12995,7 +12994,6 @@ "src/core/profiling/basic_timers.c", "src/core/profiling/stap_timers.c", "src/core/profiling/timers.h", - "src/core/profiling/timers_preciseclock.h", "src/core/surface/byte_buffer.c", "src/core/surface/byte_buffer_queue.c", "src/core/surface/byte_buffer_queue.h", diff --git a/tools/run_tests/tests.json b/tools/run_tests/tests.json index 6c06d74834b..127b1dfc408 100644 --- a/tools/run_tests/tests.json +++ b/tools/run_tests/tests.json @@ -1538,6 +1538,7 @@ "posix", "windows" ], + "exclude_configs": [], "flaky": false, "language": "c++", "name": "status_test", diff --git a/vsprojects/gpr/gpr.vcxproj b/vsprojects/gpr/gpr.vcxproj index 83c295625d8..3f8f554fd31 100644 --- a/vsprojects/gpr/gpr.vcxproj +++ b/vsprojects/gpr/gpr.vcxproj @@ -158,6 +158,7 @@ + diff --git a/vsprojects/gpr/gpr.vcxproj.filters b/vsprojects/gpr/gpr.vcxproj.filters index 64b90924ab4..b6ac061e05f 100644 --- a/vsprojects/gpr/gpr.vcxproj.filters +++ b/vsprojects/gpr/gpr.vcxproj.filters @@ -218,6 +218,9 @@ src\core\support + + src\core\support + diff --git a/vsprojects/grpc/grpc.vcxproj b/vsprojects/grpc/grpc.vcxproj index ebdc926ee7e..efb3882cad7 100644 --- a/vsprojects/grpc/grpc.vcxproj +++ b/vsprojects/grpc/grpc.vcxproj @@ -307,7 +307,6 @@ - diff --git a/vsprojects/grpc/grpc.vcxproj.filters b/vsprojects/grpc/grpc.vcxproj.filters index baec0db4f92..45d73b467ad 100644 --- a/vsprojects/grpc/grpc.vcxproj.filters +++ b/vsprojects/grpc/grpc.vcxproj.filters @@ -677,9 +677,6 @@ src\core\profiling - - src\core\profiling - src\core\surface diff --git a/vsprojects/grpc_unsecure/grpc_unsecure.vcxproj b/vsprojects/grpc_unsecure/grpc_unsecure.vcxproj index 1d60839b702..3b93092eced 100644 --- a/vsprojects/grpc_unsecure/grpc_unsecure.vcxproj +++ b/vsprojects/grpc_unsecure/grpc_unsecure.vcxproj @@ -290,7 +290,6 @@ - diff --git a/vsprojects/grpc_unsecure/grpc_unsecure.vcxproj.filters b/vsprojects/grpc_unsecure/grpc_unsecure.vcxproj.filters index cb9e8c2741b..9166acd4ff9 100644 --- a/vsprojects/grpc_unsecure/grpc_unsecure.vcxproj.filters +++ b/vsprojects/grpc_unsecure/grpc_unsecure.vcxproj.filters @@ -575,9 +575,6 @@ src\core\profiling - - src\core\profiling - src\core\surface From df77673c6cd578741b1017588190602bc97b4f7d Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 25 Aug 2015 06:59:51 -0700 Subject: [PATCH 04/21] Dont try to parallelize OpenSSL build --- Makefile | 2 +- templates/Makefile.template | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 1053c517fe4..54eadeeb873 100644 --- a/Makefile +++ b/Makefile @@ -1649,7 +1649,7 @@ else $(Q)(cd third_party/openssl ; CC="$(CC) $(PIC_CPPFLAGS) -fvisibility=hidden $(CPPFLAGS_$(CONFIG)) $(OPENSSL_CFLAGS_$(CONFIG)) $(OPENSSL_CFLAGS_EXTRA)" ./config no-asm $(OPENSSL_CONFIG_$(CONFIG))) endif endif - $(Q)$(MAKE) -C third_party/openssl clean + $(Q)$(MAKE) -j 1 -C third_party/openssl clean $(Q)(unset CPPFLAGS; $(MAKE) -C third_party/openssl build_crypto build_ssl) $(Q)mkdir -p $(LIBDIR)/$(CONFIG)/openssl $(Q)cp third_party/openssl/libssl.a third_party/openssl/libcrypto.a $(LIBDIR)/$(CONFIG)/openssl diff --git a/templates/Makefile.template b/templates/Makefile.template index 00582a22f8d..0af3dbde7ac 100644 --- a/templates/Makefile.template +++ b/templates/Makefile.template @@ -859,7 +859,7 @@ else $(Q)(cd third_party/openssl ; CC="$(CC) $(PIC_CPPFLAGS) -fvisibility=hidden $(CPPFLAGS_$(CONFIG)) $(OPENSSL_CFLAGS_$(CONFIG)) $(OPENSSL_CFLAGS_EXTRA)" ./config no-asm $(OPENSSL_CONFIG_$(CONFIG))) endif endif - $(Q)$(MAKE) -C third_party/openssl clean + $(Q)$(MAKE) -j 1 -C third_party/openssl clean $(Q)(unset CPPFLAGS; $(MAKE) -C third_party/openssl build_crypto build_ssl) $(Q)mkdir -p $(LIBDIR)/$(CONFIG)/openssl $(Q)cp third_party/openssl/libssl.a third_party/openssl/libcrypto.a $(LIBDIR)/$(CONFIG)/openssl From dc31ef38d2530ed814d18150755710545293a8ed Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 25 Aug 2015 07:11:35 -0700 Subject: [PATCH 05/21] Loosen test requirements to better fit spec --- test/cpp/end2end/generic_end2end_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/cpp/end2end/generic_end2end_test.cc b/test/cpp/end2end/generic_end2end_test.cc index de7eab8dc29..809eef058cf 100644 --- a/test/cpp/end2end/generic_end2end_test.cc +++ b/test/cpp/end2end/generic_end2end_test.cc @@ -160,7 +160,7 @@ class GenericEnd2endTest : public ::testing::Test { srv_cq_.get(), tag(4)); verify_ok(srv_cq_.get(), 4, true); - EXPECT_EQ(server_host_, srv_ctx.host()); + EXPECT_EQ(server_host_, srv_ctx.host().substr(0, server_host_.length())); EXPECT_EQ(kMethodName, srv_ctx.method()); ByteBuffer recv_buffer; stream.Read(&recv_buffer, tag(5)); @@ -233,7 +233,7 @@ TEST_F(GenericEnd2endTest, SimpleBidiStreaming) { srv_cq_.get(), tag(2)); verify_ok(srv_cq_.get(), 2, true); - EXPECT_EQ(server_host_, srv_ctx.host()); + EXPECT_EQ(server_host_, srv_ctx.host().substr(0, server_host_.length())); EXPECT_EQ(kMethodName, srv_ctx.method()); std::unique_ptr send_buffer = From a96ce800a8df5c62ffd264317836ecf3433c4344 Mon Sep 17 00:00:00 2001 From: Hongyu Chen Date: Tue, 25 Aug 2015 10:57:17 -0700 Subject: [PATCH 06/21] Update include path of core/profiling/basic_timers.c --- src/core/profiling/basic_timers.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/profiling/basic_timers.c b/src/core/profiling/basic_timers.c index 9892a161209..4b6a0d2f56b 100644 --- a/src/core/profiling/basic_timers.c +++ b/src/core/profiling/basic_timers.c @@ -36,7 +36,6 @@ #ifdef GRPC_BASIC_PROFILER #include "src/core/profiling/timers.h" -#include "src/core/profiling/timers_preciseclock.h" #include #include From 775ec1decd597d9ecdf916e3d220b44283075ed9 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 25 Aug 2015 11:03:53 -0700 Subject: [PATCH 07/21] Use SSL override as a default host name if none is specified --- include/grpc/grpc.h | 8 ++++++++ include/grpc/grpc_security.h | 9 --------- src/core/surface/channel.c | 19 +++++++++++++++++++ 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/include/grpc/grpc.h b/include/grpc/grpc.h index 101fc88d8f6..145052b6d3c 100644 --- a/include/grpc/grpc.h +++ b/include/grpc/grpc.h @@ -134,6 +134,14 @@ typedef struct { /** Secondary user agent: goes at the end of the user-agent metadata sent on each request */ #define GRPC_ARG_SECONDARY_USER_AGENT_STRING "grpc.secondary_user_agent" +/* The caller of the secure_channel_create functions may override the target + name used for SSL host name checking using this channel argument which is of + type GRPC_ARG_STRING. This *should* be used for testing only. + If this argument is not specified, the name used for SSL host name checking + will be the target parameter (assuming that the secure channel is an SSL + channel). If this parameter is specified and the underlying is not an SSL + channel, it will just be ignored. */ +#define GRPC_SSL_TARGET_NAME_OVERRIDE_ARG "grpc.ssl_target_name_override" /** Connectivity state of a channel. */ typedef enum { diff --git a/include/grpc/grpc_security.h b/include/grpc/grpc_security.h index 7f8f4d4a053..de565b2d2fc 100644 --- a/include/grpc/grpc_security.h +++ b/include/grpc/grpc_security.h @@ -142,15 +142,6 @@ grpc_credentials *grpc_iam_credentials_create(const char *authorization_token, /* --- Secure channel creation. --- */ -/* The caller of the secure_channel_create functions may override the target - name used for SSL host name checking using this channel argument which is of - type GRPC_ARG_STRING. This *should* be used for testing only. - If this argument is not specified, the name used for SSL host name checking - will be the target parameter (assuming that the secure channel is an SSL - channel). If this parameter is specified and the underlying is not an SSL - channel, it will just be ignored. */ -#define GRPC_SSL_TARGET_NAME_OVERRIDE_ARG "grpc.ssl_target_name_override" - /* Creates a secure channel using the passed-in credentials. */ grpc_channel *grpc_secure_channel_create(grpc_credentials *creds, const char *target, diff --git a/src/core/surface/channel.c b/src/core/surface/channel.c index 78eeed1f598..586402e21c3 100644 --- a/src/core/surface/channel.c +++ b/src/core/surface/channel.c @@ -141,9 +141,28 @@ grpc_channel *grpc_channel_create_from_filters( gpr_log(GPR_ERROR, "%s: must be an string", GRPC_ARG_DEFAULT_AUTHORITY); } else { + if (channel->default_authority) { + /* setting this takes precedence over anything else */ + GRPC_MDELEM_UNREF(channel->default_authority); + } channel->default_authority = grpc_mdelem_from_strings( mdctx, ":authority", args->args[i].value.string); } + } else if (0 == + strcmp(args->args[i].key, GRPC_SSL_TARGET_NAME_OVERRIDE_ARG)) { + if (args->args[i].type != GRPC_ARG_STRING) { + gpr_log(GPR_ERROR, "%s: must be an string", + GRPC_SSL_TARGET_NAME_OVERRIDE_ARG); + } else { + if (channel->default_authority) { + /* other ways of setting this (notably ssl) take precedence */ + gpr_log(GPR_ERROR, "%s: default host already set some other way", + GRPC_ARG_DEFAULT_AUTHORITY); + } else { + channel->default_authority = grpc_mdelem_from_strings( + mdctx, ":authority", args->args[i].value.string); + } + } } } } From d5539ec6e28c275eeb71afaf9bcc5d91a4410e6e Mon Sep 17 00:00:00 2001 From: yang-g Date: Tue, 25 Aug 2015 11:05:29 -0700 Subject: [PATCH 08/21] remove constexpr since gcc 4.4 or vs2010 does not support it --- include/grpc++/support/string_ref.h | 28 +++++++++++++--------------- src/cpp/util/string_ref.cc | 2 +- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/include/grpc++/support/string_ref.h b/include/grpc++/support/string_ref.h index 0ec39a9b0a6..70bacc94451 100644 --- a/include/grpc++/support/string_ref.h +++ b/include/grpc++/support/string_ref.h @@ -50,22 +50,22 @@ class string_ref { typedef std::reverse_iterator const_reverse_iterator; // constants - static constexpr size_t npos = size_t(-1); + static size_t npos = size_t(-1); // construct/copy. - constexpr string_ref() : data_(nullptr), length_(0) {} - constexpr string_ref(const string_ref& other) + string_ref() : data_(nullptr), length_(0) {} + string_ref(const string_ref& other) : data_(other.data_), length_(other.length_) {} string_ref& operator=(const string_ref& rhs); string_ref(const char* s); - constexpr string_ref(const char* s, size_t l) : data_(s), length_(l) {} + string_ref(const char* s, size_t l) : data_(s), length_(l) {} string_ref(const grpc::string& s) : data_(s.data()), length_(s.length()) {} // iterators - constexpr const_iterator begin() const { return data_; } - constexpr const_iterator end() const { return data_ + length_; } - constexpr const_iterator cbegin() const { return data_; } - constexpr const_iterator cend() const { return data_ + length_; } + const_iterator begin() const { return data_; } + const_iterator end() const { return data_ + length_; } + const_iterator cbegin() const { return data_; } + const_iterator cend() const { return data_ + length_; } const_reverse_iterator rbegin() const { return const_reverse_iterator(end()); } @@ -80,10 +80,10 @@ class string_ref { } // capacity - constexpr size_t size() const { return length_; } - constexpr size_t length() const { return length_; } - constexpr size_t max_size() const { return length_; } - constexpr bool empty() const { return length_ == 0; } + size_t size() const { return length_; } + size_t length() const { return length_; } + size_t max_size() const { return length_; } + bool empty() const { return length_ == 0; } // element access const char* data() const { return data_; } @@ -95,9 +95,7 @@ class string_ref { size_t find(string_ref s) const; size_t find(char c) const; - // Defined as constexpr in n3442 but C++11 constexpr semantics do not allow - // the implementation of this function to comply. - /* constrexpr */ string_ref substr(size_t pos, size_t n = npos) const; + string_ref substr(size_t pos, size_t n = npos) const; private: const char* data_; diff --git a/src/cpp/util/string_ref.cc b/src/cpp/util/string_ref.cc index 8483e8c2eea..ff6b7617a3a 100644 --- a/src/cpp/util/string_ref.cc +++ b/src/cpp/util/string_ref.cc @@ -39,7 +39,7 @@ namespace grpc { -constexpr size_t string_ref::npos; +size_t string_ref::npos; string_ref& string_ref::operator=(const string_ref& rhs) { data_ = rhs.data_; From 1eeb21c4df03194654252d1feaee779514ecefad Mon Sep 17 00:00:00 2001 From: yang-g Date: Tue, 25 Aug 2015 11:05:50 -0700 Subject: [PATCH 09/21] api change to string_ref --- include/grpc++/client_context.h | 10 ++++++---- include/grpc++/server_context.h | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/include/grpc++/client_context.h b/include/grpc++/client_context.h index ee28f360cbe..c4724cc1f01 100644 --- a/include/grpc++/client_context.h +++ b/include/grpc++/client_context.h @@ -138,12 +138,14 @@ class ClientContext { void AddMetadata(const grpc::string& meta_key, const grpc::string& meta_value); - const std::multimap& GetServerInitialMetadata() { + const std::multimap& + GetServerInitialMetadata() { GPR_ASSERT(initial_metadata_received_); return recv_initial_metadata_; } - const std::multimap& GetServerTrailingMetadata() { + const std::multimap& + GetServerTrailingMetadata() { // TODO(yangg) check finished return trailing_metadata_; } @@ -234,8 +236,8 @@ class ClientContext { mutable std::shared_ptr auth_context_; struct census_context* census_context_; std::multimap send_initial_metadata_; - std::multimap recv_initial_metadata_; - std::multimap trailing_metadata_; + std::multimap recv_initial_metadata_; + std::multimap trailing_metadata_; grpc_call* propagate_from_call_; PropagationOptions propagation_options_; diff --git a/include/grpc++/server_context.h b/include/grpc++/server_context.h index ce3cb47a237..a165d84944c 100644 --- a/include/grpc++/server_context.h +++ b/include/grpc++/server_context.h @@ -103,7 +103,7 @@ class ServerContext { bool IsCancelled() const; - const std::multimap& client_metadata() { + const std::multimap& client_metadata() { return client_metadata_; } @@ -185,7 +185,7 @@ class ServerContext { CompletionQueue* cq_; bool sent_initial_metadata_; mutable std::shared_ptr auth_context_; - std::multimap client_metadata_; + std::multimap client_metadata_; std::multimap initial_metadata_; std::multimap trailing_metadata_; From c9a3b0d55fc9bf29522699f4be8267b2eb2fdc24 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 25 Aug 2015 11:20:02 -0700 Subject: [PATCH 10/21] Respond to review --- src/core/client_config/resolver_factory.c | 4 ++-- src/core/client_config/resolver_registry.c | 27 ++++++++++++---------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/core/client_config/resolver_factory.c b/src/core/client_config/resolver_factory.c index bf631cefd13..5b859a8d10b 100644 --- a/src/core/client_config/resolver_factory.c +++ b/src/core/client_config/resolver_factory.c @@ -45,12 +45,12 @@ void grpc_resolver_factory_unref(grpc_resolver_factory *factory) { grpc_resolver *grpc_resolver_factory_create_resolver( grpc_resolver_factory *factory, grpc_uri *uri, grpc_subchannel_factory *subchannel_factory) { - if (!factory) return NULL; + if (factory == NULL) return NULL; return factory->vtable->create_resolver(factory, uri, subchannel_factory); } char *grpc_resolver_factory_get_default_authority( grpc_resolver_factory *factory, grpc_uri *uri) { - if (!factory) return NULL; + if (factory == NULL) return NULL; return factory->vtable->get_default_authority(factory, uri); } diff --git a/src/core/client_config/resolver_registry.c b/src/core/client_config/resolver_registry.c index 3aff4630fba..af263df26a9 100644 --- a/src/core/client_config/resolver_registry.c +++ b/src/core/client_config/resolver_registry.c @@ -92,20 +92,23 @@ static grpc_resolver_factory *resolve_factory(const char *target, *uri = grpc_uri_parse(target, 1); factory = lookup_factory(*uri); - if (factory == NULL && g_default_resolver_prefix != NULL) { - grpc_uri_destroy(*uri); - gpr_asprintf(&tmp, "%s%s", g_default_resolver_prefix, target); - *uri = grpc_uri_parse(tmp, 1); - factory = lookup_factory(*uri); - if (factory == NULL) { + if (factory == NULL) { + if (g_default_resolver_prefix != NULL) { + grpc_uri_destroy(*uri); + gpr_asprintf(&tmp, "%s%s", g_default_resolver_prefix, target); + *uri = grpc_uri_parse(tmp, 1); + factory = lookup_factory(*uri); + if (factory == NULL) { + grpc_uri_destroy(grpc_uri_parse(target, 0)); + grpc_uri_destroy(grpc_uri_parse(tmp, 0)); + gpr_log(GPR_ERROR, "don't know how to resolve '%s' or '%s'", target, + tmp); + } + gpr_free(tmp); + } else { grpc_uri_destroy(grpc_uri_parse(target, 0)); - grpc_uri_destroy(grpc_uri_parse(tmp, 0)); - gpr_log(GPR_ERROR, "don't know how to resolve '%s' or '%s'", target, tmp); + gpr_log(GPR_ERROR, "don't know how to resolve '%s'", target); } - gpr_free(tmp); - } else if (factory == NULL) { - grpc_uri_destroy(grpc_uri_parse(target, 0)); - gpr_log(GPR_ERROR, "don't know how to resolve '%s'", target); } return factory; } From c0136b9a270e406f6eee4962ce01722c7e72dc6f Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 25 Aug 2015 11:21:43 -0700 Subject: [PATCH 11/21] Add comments --- src/core/client_config/resolver_factory.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/core/client_config/resolver_factory.h b/src/core/client_config/resolver_factory.h index 73a05642303..e243b23988a 100644 --- a/src/core/client_config/resolver_factory.h +++ b/src/core/client_config/resolver_factory.h @@ -51,12 +51,15 @@ struct grpc_resolver_factory_vtable { void (*ref)(grpc_resolver_factory *factory); void (*unref)(grpc_resolver_factory *factory); + /** Implementation of grpc_resolver_factory_create_resolver */ grpc_resolver *(*create_resolver)( grpc_resolver_factory *factory, grpc_uri *uri, grpc_subchannel_factory *subchannel_factory); + /** Implementation of grpc_resolver_factory_get_default_authority */ char *(*get_default_authority)(grpc_resolver_factory *factory, grpc_uri *uri); + /** URI scheme that this factory implements */ const char *scheme; }; From d01b6f49a2f28eae3d5dcbd779e68a4ac54ee38c Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 25 Aug 2015 12:58:07 -0700 Subject: [PATCH 12/21] Its not enough to just serialize the clean step... --- Makefile | 2 +- templates/Makefile.template | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 674b83ab157..8820ca05fe0 100644 --- a/Makefile +++ b/Makefile @@ -1651,7 +1651,7 @@ else endif endif $(Q)$(MAKE) -j 1 -C third_party/openssl clean - $(Q)(unset CPPFLAGS; $(MAKE) -C third_party/openssl build_crypto build_ssl) + $(Q)(unset CPPFLAGS; $(MAKE) -j 1 -C third_party/openssl build_crypto build_ssl) $(Q)mkdir -p $(LIBDIR)/$(CONFIG)/openssl $(Q)cp third_party/openssl/libssl.a third_party/openssl/libcrypto.a $(LIBDIR)/$(CONFIG)/openssl diff --git a/templates/Makefile.template b/templates/Makefile.template index 0af3dbde7ac..797f0ab57fa 100644 --- a/templates/Makefile.template +++ b/templates/Makefile.template @@ -860,7 +860,7 @@ else endif endif $(Q)$(MAKE) -j 1 -C third_party/openssl clean - $(Q)(unset CPPFLAGS; $(MAKE) -C third_party/openssl build_crypto build_ssl) + $(Q)(unset CPPFLAGS; $(MAKE) -j 1 -C third_party/openssl build_crypto build_ssl) $(Q)mkdir -p $(LIBDIR)/$(CONFIG)/openssl $(Q)cp third_party/openssl/libssl.a third_party/openssl/libcrypto.a $(LIBDIR)/$(CONFIG)/openssl From f5cf6fa478f2dde957b052739a11d45dcdd0e111 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 25 Aug 2015 13:31:00 -0700 Subject: [PATCH 13/21] Assert uri not null --- src/core/client_config/resolver_registry.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/client_config/resolver_registry.c b/src/core/client_config/resolver_registry.c index af263df26a9..886ca5d06d9 100644 --- a/src/core/client_config/resolver_registry.c +++ b/src/core/client_config/resolver_registry.c @@ -90,6 +90,7 @@ static grpc_resolver_factory *resolve_factory(const char *target, char *tmp; grpc_resolver_factory *factory = NULL; + GPR_ASSERT(uri); *uri = grpc_uri_parse(target, 1); factory = lookup_factory(*uri); if (factory == NULL) { From e21908fcee3ffac086575e247be3442860ac7b3a Mon Sep 17 00:00:00 2001 From: yang-g Date: Tue, 25 Aug 2015 13:47:51 -0700 Subject: [PATCH 14/21] impl --- Makefile | 3 ++ build.json | 2 + include/grpc++/client_context.h | 1 + include/grpc++/impl/call.h | 9 +++-- include/grpc++/server_context.h | 1 + include/grpc++/support/string_ref.h | 2 +- src/cpp/common/call.cc | 11 +++--- src/cpp/server/server.cc | 11 +++--- src/cpp/server/server_context.cc | 7 ++-- src/cpp/util/string_ref.cc | 2 +- test/cpp/end2end/async_end2end_test.cc | 37 +++++++++++++------ test/cpp/end2end/end2end_test.cc | 27 ++++++++------ test/cpp/util/cli_call.cc | 10 ++--- test/cpp/util/cli_call.h | 12 ++++-- test/cpp/util/cli_call_test.cc | 13 ++++--- test/cpp/util/grpc_cli.cc | 22 +++++++---- test/cpp/util/string_ref_helper.cc | 44 ++++++++++++++++++++++ test/cpp/util/string_ref_helper.h | 47 ++++++++++++++++++++++++ tools/run_tests/sources_and_headers.json | 3 ++ vsprojects/Grpc.mak | 4 +- 20 files changed, 200 insertions(+), 68 deletions(-) create mode 100644 test/cpp/util/string_ref_helper.cc create mode 100644 test/cpp/util/string_ref_helper.h diff --git a/Makefile b/Makefile index 975633f998a..5fbe3d98c1d 100644 --- a/Makefile +++ b/Makefile @@ -4793,6 +4793,7 @@ LIBGRPC++_TEST_UTIL_SRC = \ $(GENDIR)/test/cpp/util/echo_duplicate.pb.cc $(GENDIR)/test/cpp/util/echo_duplicate.grpc.pb.cc \ test/cpp/util/cli_call.cc \ test/cpp/util/create_test_channel.cc \ + test/cpp/util/string_ref_helper.cc \ test/cpp/util/subprocess.cc \ @@ -4839,6 +4840,7 @@ endif endif $(OBJDIR)/$(CONFIG)/test/cpp/util/cli_call.o: $(GENDIR)/test/cpp/util/messages.pb.cc $(GENDIR)/test/cpp/util/messages.grpc.pb.cc $(GENDIR)/test/cpp/util/echo.pb.cc $(GENDIR)/test/cpp/util/echo.grpc.pb.cc $(GENDIR)/test/cpp/util/echo_duplicate.pb.cc $(GENDIR)/test/cpp/util/echo_duplicate.grpc.pb.cc $(OBJDIR)/$(CONFIG)/test/cpp/util/create_test_channel.o: $(GENDIR)/test/cpp/util/messages.pb.cc $(GENDIR)/test/cpp/util/messages.grpc.pb.cc $(GENDIR)/test/cpp/util/echo.pb.cc $(GENDIR)/test/cpp/util/echo.grpc.pb.cc $(GENDIR)/test/cpp/util/echo_duplicate.pb.cc $(GENDIR)/test/cpp/util/echo_duplicate.grpc.pb.cc +$(OBJDIR)/$(CONFIG)/test/cpp/util/string_ref_helper.o: $(GENDIR)/test/cpp/util/messages.pb.cc $(GENDIR)/test/cpp/util/messages.grpc.pb.cc $(GENDIR)/test/cpp/util/echo.pb.cc $(GENDIR)/test/cpp/util/echo.grpc.pb.cc $(GENDIR)/test/cpp/util/echo_duplicate.pb.cc $(GENDIR)/test/cpp/util/echo_duplicate.grpc.pb.cc $(OBJDIR)/$(CONFIG)/test/cpp/util/subprocess.o: $(GENDIR)/test/cpp/util/messages.pb.cc $(GENDIR)/test/cpp/util/messages.grpc.pb.cc $(GENDIR)/test/cpp/util/echo.pb.cc $(GENDIR)/test/cpp/util/echo.grpc.pb.cc $(GENDIR)/test/cpp/util/echo_duplicate.pb.cc $(GENDIR)/test/cpp/util/echo_duplicate.grpc.pb.cc @@ -20672,6 +20674,7 @@ test/cpp/qps/timer.cc: $(OPENSSL_DEP) test/cpp/util/benchmark_config.cc: $(OPENSSL_DEP) test/cpp/util/cli_call.cc: $(OPENSSL_DEP) test/cpp/util/create_test_channel.cc: $(OPENSSL_DEP) +test/cpp/util/string_ref_helper.cc: $(OPENSSL_DEP) test/cpp/util/subprocess.cc: $(OPENSSL_DEP) test/cpp/util/test_config.cc: $(OPENSSL_DEP) endif diff --git a/build.json b/build.json index 4f9017a0ad4..8990d925a44 100644 --- a/build.json +++ b/build.json @@ -660,6 +660,7 @@ "headers": [ "test/cpp/util/cli_call.h", "test/cpp/util/create_test_channel.h", + "test/cpp/util/string_ref_helper.h", "test/cpp/util/subprocess.h" ], "src": [ @@ -668,6 +669,7 @@ "test/cpp/util/echo_duplicate.proto", "test/cpp/util/cli_call.cc", "test/cpp/util/create_test_channel.cc", + "test/cpp/util/string_ref_helper.cc", "test/cpp/util/subprocess.cc" ], "deps": [ diff --git a/include/grpc++/client_context.h b/include/grpc++/client_context.h index c4724cc1f01..62e5260a180 100644 --- a/include/grpc++/client_context.h +++ b/include/grpc++/client_context.h @@ -45,6 +45,7 @@ #include #include #include +#include #include struct census_context; diff --git a/include/grpc++/impl/call.h b/include/grpc++/impl/call.h index e5da6c9e2af..fca56030479 100644 --- a/include/grpc++/impl/call.h +++ b/include/grpc++/impl/call.h @@ -54,8 +54,9 @@ namespace grpc { class ByteBuffer; class Call; -void FillMetadataMap(grpc_metadata_array* arr, - std::multimap* metadata); +void FillMetadataMap( + grpc_metadata_array* arr, + std::multimap* metadata); grpc_metadata* FillMetadataArray( const std::multimap& metadata); @@ -418,7 +419,7 @@ class CallOpRecvInitialMetadata { } private: - std::multimap* recv_initial_metadata_; + std::multimap* recv_initial_metadata_; grpc_metadata_array recv_initial_metadata_arr_; }; @@ -461,7 +462,7 @@ class CallOpClientRecvStatus { } private: - std::multimap* recv_trailing_metadata_; + std::multimap* recv_trailing_metadata_; Status* recv_status_; grpc_metadata_array recv_trailing_metadata_arr_; grpc_status_code status_code_; diff --git a/include/grpc++/server_context.h b/include/grpc++/server_context.h index a165d84944c..4b17a280471 100644 --- a/include/grpc++/server_context.h +++ b/include/grpc++/server_context.h @@ -41,6 +41,7 @@ #include #include #include +#include #include struct gpr_timespec; diff --git a/include/grpc++/support/string_ref.h b/include/grpc++/support/string_ref.h index 70bacc94451..348c42cbbaa 100644 --- a/include/grpc++/support/string_ref.h +++ b/include/grpc++/support/string_ref.h @@ -50,7 +50,7 @@ class string_ref { typedef std::reverse_iterator const_reverse_iterator; // constants - static size_t npos = size_t(-1); + const static size_t npos = size_t(-1); // construct/copy. string_ref() : data_(nullptr), length_(0) {} diff --git a/src/cpp/common/call.cc b/src/cpp/common/call.cc index 16aa2c9fb9d..5b87c2a8067 100644 --- a/src/cpp/common/call.cc +++ b/src/cpp/common/call.cc @@ -41,13 +41,14 @@ namespace grpc { -void FillMetadataMap(grpc_metadata_array* arr, - std::multimap* metadata) { +void FillMetadataMap( + grpc_metadata_array* arr, + std::multimap* metadata) { for (size_t i = 0; i < arr->count; i++) { // TODO(yangg) handle duplicates? - metadata->insert(std::pair( - arr->metadata[i].key, - grpc::string(arr->metadata[i].value, arr->metadata[i].value_length))); + metadata->insert(std::pair( + arr->metadata[i].key, grpc::string_ref(arr->metadata[i].value, + arr->metadata[i].value_length))); } grpc_metadata_array_destroy(arr); grpc_metadata_array_init(arr); diff --git a/src/cpp/server/server.cc b/src/cpp/server/server.cc index 66cd27cc339..bb83c7d887b 100644 --- a/src/cpp/server/server.cc +++ b/src/cpp/server/server.cc @@ -439,11 +439,12 @@ Server::BaseAsyncRequest::~BaseAsyncRequest() {} bool Server::BaseAsyncRequest::FinalizeResult(void** tag, bool* status) { if (*status) { for (size_t i = 0; i < initial_metadata_array_.count; i++) { - context_->client_metadata_.insert(std::make_pair( - grpc::string(initial_metadata_array_.metadata[i].key), - grpc::string(initial_metadata_array_.metadata[i].value, - initial_metadata_array_.metadata[i].value + - initial_metadata_array_.metadata[i].value_length))); + context_->client_metadata_.insert( + std::pair( + initial_metadata_array_.metadata[i].key, + grpc::string_ref( + initial_metadata_array_.metadata[i].value, + initial_metadata_array_.metadata[i].value_length))); } } grpc_metadata_array_destroy(&initial_metadata_array_); diff --git a/src/cpp/server/server_context.cc b/src/cpp/server/server_context.cc index acc163d6b5d..8193e70660d 100644 --- a/src/cpp/server/server_context.cc +++ b/src/cpp/server/server_context.cc @@ -136,10 +136,9 @@ ServerContext::ServerContext(gpr_timespec deadline, grpc_metadata* metadata, cq_(nullptr), sent_initial_metadata_(false) { for (size_t i = 0; i < metadata_count; i++) { - client_metadata_.insert(std::make_pair( - grpc::string(metadata[i].key), - grpc::string(metadata[i].value, - metadata[i].value + metadata[i].value_length))); + client_metadata_.insert(std::pair( + metadata[i].key, + grpc::string_ref(metadata[i].value, metadata[i].value_length))); } } diff --git a/src/cpp/util/string_ref.cc b/src/cpp/util/string_ref.cc index ff6b7617a3a..d9c9019da89 100644 --- a/src/cpp/util/string_ref.cc +++ b/src/cpp/util/string_ref.cc @@ -39,7 +39,7 @@ namespace grpc { -size_t string_ref::npos; +const size_t string_ref::npos; string_ref& string_ref::operator=(const string_ref& rhs) { data_ = rhs.data_; diff --git a/test/cpp/end2end/async_end2end_test.cc b/test/cpp/end2end/async_end2end_test.cc index 6343810ee93..41b91e459bc 100644 --- a/test/cpp/end2end/async_end2end_test.cc +++ b/test/cpp/end2end/async_end2end_test.cc @@ -50,6 +50,7 @@ #include "test/core/util/test_config.h" #include "test/cpp/util/echo_duplicate.grpc.pb.h" #include "test/cpp/util/echo.grpc.pb.h" +#include "test/cpp/util/string_ref_helper.h" #ifdef GPR_POSIX_SOCKET #include "src/core/iomgr/pollset_posix.h" @@ -484,8 +485,10 @@ TEST_P(AsyncEnd2endTest, ClientInitialMetadataRpc) { Verifier(GetParam()).Expect(2, true).Verify(cq_.get()); EXPECT_EQ(send_request.message(), recv_request.message()); auto client_initial_metadata = srv_ctx.client_metadata(); - EXPECT_EQ(meta1.second, client_initial_metadata.find(meta1.first)->second); - EXPECT_EQ(meta2.second, client_initial_metadata.find(meta2.first)->second); + EXPECT_EQ(meta1.second, + ToString(client_initial_metadata.find(meta1.first)->second)); + EXPECT_EQ(meta2.second, + ToString(client_initial_metadata.find(meta2.first)->second)); EXPECT_GE(client_initial_metadata.size(), static_cast(2)); send_response.set_message(recv_request.message()); @@ -532,8 +535,10 @@ TEST_P(AsyncEnd2endTest, ServerInitialMetadataRpc) { response_reader->ReadInitialMetadata(tag(4)); Verifier(GetParam()).Expect(4, true).Verify(cq_.get()); auto server_initial_metadata = cli_ctx.GetServerInitialMetadata(); - EXPECT_EQ(meta1.second, server_initial_metadata.find(meta1.first)->second); - EXPECT_EQ(meta2.second, server_initial_metadata.find(meta2.first)->second); + EXPECT_EQ(meta1.second, + ToString(server_initial_metadata.find(meta1.first)->second)); + EXPECT_EQ(meta2.second, + ToString(server_initial_metadata.find(meta2.first)->second)); EXPECT_EQ(static_cast(2), server_initial_metadata.size()); send_response.set_message(recv_request.message()); @@ -586,8 +591,10 @@ TEST_P(AsyncEnd2endTest, ServerTrailingMetadataRpc) { EXPECT_EQ(send_response.message(), recv_response.message()); EXPECT_TRUE(recv_status.ok()); auto server_trailing_metadata = cli_ctx.GetServerTrailingMetadata(); - EXPECT_EQ(meta1.second, server_trailing_metadata.find(meta1.first)->second); - EXPECT_EQ(meta2.second, server_trailing_metadata.find(meta2.first)->second); + EXPECT_EQ(meta1.second, + ToString(server_trailing_metadata.find(meta1.first)->second)); + EXPECT_EQ(meta2.second, + ToString(server_trailing_metadata.find(meta2.first)->second)); EXPECT_EQ(static_cast(2), server_trailing_metadata.size()); } @@ -631,8 +638,10 @@ TEST_P(AsyncEnd2endTest, MetadataRpc) { Verifier(GetParam()).Expect(2, true).Verify(cq_.get()); EXPECT_EQ(send_request.message(), recv_request.message()); auto client_initial_metadata = srv_ctx.client_metadata(); - EXPECT_EQ(meta1.second, client_initial_metadata.find(meta1.first)->second); - EXPECT_EQ(meta2.second, client_initial_metadata.find(meta2.first)->second); + EXPECT_EQ(meta1.second, + ToString(client_initial_metadata.find(meta1.first)->second)); + EXPECT_EQ(meta2.second, + ToString(client_initial_metadata.find(meta2.first)->second)); EXPECT_GE(client_initial_metadata.size(), static_cast(2)); srv_ctx.AddInitialMetadata(meta3.first, meta3.second); @@ -642,8 +651,10 @@ TEST_P(AsyncEnd2endTest, MetadataRpc) { response_reader->ReadInitialMetadata(tag(4)); Verifier(GetParam()).Expect(4, true).Verify(cq_.get()); auto server_initial_metadata = cli_ctx.GetServerInitialMetadata(); - EXPECT_EQ(meta3.second, server_initial_metadata.find(meta3.first)->second); - EXPECT_EQ(meta4.second, server_initial_metadata.find(meta4.first)->second); + EXPECT_EQ(meta3.second, + ToString(server_initial_metadata.find(meta3.first)->second)); + EXPECT_EQ(meta4.second, + ToString(server_initial_metadata.find(meta4.first)->second)); EXPECT_GE(server_initial_metadata.size(), static_cast(2)); send_response.set_message(recv_request.message()); @@ -658,8 +669,10 @@ TEST_P(AsyncEnd2endTest, MetadataRpc) { EXPECT_EQ(send_response.message(), recv_response.message()); EXPECT_TRUE(recv_status.ok()); auto server_trailing_metadata = cli_ctx.GetServerTrailingMetadata(); - EXPECT_EQ(meta5.second, server_trailing_metadata.find(meta5.first)->second); - EXPECT_EQ(meta6.second, server_trailing_metadata.find(meta6.first)->second); + EXPECT_EQ(meta5.second, + ToString(server_trailing_metadata.find(meta5.first)->second)); + EXPECT_EQ(meta6.second, + ToString(server_trailing_metadata.find(meta6.first)->second)); EXPECT_GE(server_trailing_metadata.size(), static_cast(2)); } diff --git a/test/cpp/end2end/end2end_test.cc b/test/cpp/end2end/end2end_test.cc index 2728dce07e5..0d5bf36df72 100644 --- a/test/cpp/end2end/end2end_test.cc +++ b/test/cpp/end2end/end2end_test.cc @@ -53,6 +53,7 @@ #include "test/core/util/test_config.h" #include "test/cpp/util/echo_duplicate.grpc.pb.h" #include "test/cpp/util/echo.grpc.pb.h" +#include "test/cpp/util/string_ref_helper.h" using grpc::cpp::test::util::EchoRequest; using grpc::cpp::test::util::EchoResponse; @@ -152,12 +153,13 @@ class TestServiceImpl : public ::grpc::cpp::test::util::TestService::Service { } if (request->has_param() && request->param().echo_metadata()) { - const std::multimap& client_metadata = + const std::multimap& client_metadata = context->client_metadata(); - for (std::multimap::const_iterator iter = - client_metadata.begin(); + for (std::multimap::const_iterator + iter = client_metadata.begin(); iter != client_metadata.end(); ++iter) { - context->AddTrailingMetadata((*iter).first, (*iter).second); + context->AddTrailingMetadata(ToString(iter->first), + ToString(iter->second)); } } if (request->has_param() && request->param().check_auth_context()) { @@ -182,12 +184,12 @@ class TestServiceImpl : public ::grpc::cpp::test::util::TestService::Service { EchoRequest request; response->set_message(""); int cancel_after_reads = 0; - const std::multimap client_initial_metadata = - context->client_metadata(); + const std::multimap& + client_initial_metadata = context->client_metadata(); if (client_initial_metadata.find(kServerCancelAfterReads) != client_initial_metadata.end()) { - std::istringstream iss( - client_initial_metadata.find(kServerCancelAfterReads)->second); + std::istringstream iss(ToString( + client_initial_metadata.find(kServerCancelAfterReads)->second)); iss >> cancel_after_reads; gpr_log(GPR_INFO, "cancel_after_reads %d", cancel_after_reads); } @@ -721,14 +723,15 @@ TEST_F(End2endTest, RpcMaxMessageSize) { EXPECT_FALSE(s.ok()); } -bool MetadataContains(const std::multimap& metadata, - const grpc::string& key, const grpc::string& value) { +bool MetadataContains( + const std::multimap& metadata, + const grpc::string& key, const grpc::string& value) { int count = 0; - for (std::multimap::const_iterator iter = + for (std::multimap::const_iterator iter = metadata.begin(); iter != metadata.end(); ++iter) { - if ((*iter).first == key && (*iter).second == value) { + if (ToString(iter->first) == key && ToString(iter->second) == value) { count++; } } diff --git a/test/cpp/util/cli_call.cc b/test/cpp/util/cli_call.cc index d60cee9c024..9a769848a4b 100644 --- a/test/cpp/util/cli_call.cc +++ b/test/cpp/util/cli_call.cc @@ -51,14 +51,14 @@ void* tag(int i) { return (void*)(gpr_intptr)i; } Status CliCall::Call(std::shared_ptr channel, const grpc::string& method, const grpc::string& request, - grpc::string* response, const MetadataContainer& metadata, - MetadataContainer* server_initial_metadata, - MetadataContainer* server_trailing_metadata) { + grpc::string* response, + const OutgoingMetadataContainer& metadata, + IncomingMetadataContainer* server_initial_metadata, + IncomingMetadataContainer* server_trailing_metadata) { std::unique_ptr stub(new grpc::GenericStub(channel)); grpc::ClientContext ctx; if (!metadata.empty()) { - for (std::multimap::const_iterator iter = - metadata.begin(); + for (OutgoingMetadataContainer::const_iterator iter = metadata.begin(); iter != metadata.end(); ++iter) { ctx.AddMetadata(iter->first, iter->second); } diff --git a/test/cpp/util/cli_call.h b/test/cpp/util/cli_call.h index 7a3dcf2e9f6..2fbc9618b64 100644 --- a/test/cpp/util/cli_call.h +++ b/test/cpp/util/cli_call.h @@ -38,18 +38,22 @@ #include #include +#include namespace grpc { namespace testing { class CliCall GRPC_FINAL { public: - typedef std::multimap MetadataContainer; + typedef std::multimap OutgoingMetadataContainer; + typedef std::multimap + IncomingMetadataContainer; static Status Call(std::shared_ptr channel, const grpc::string& method, const grpc::string& request, - grpc::string* response, const MetadataContainer& metadata, - MetadataContainer* server_initial_metadata, - MetadataContainer* server_trailing_metadata); + grpc::string* response, + const OutgoingMetadataContainer& metadata, + IncomingMetadataContainer* server_initial_metadata, + IncomingMetadataContainer* server_trailing_metadata); }; } // namespace testing diff --git a/test/cpp/util/cli_call_test.cc b/test/cpp/util/cli_call_test.cc index 35bfad202f0..111a0e9f762 100644 --- a/test/cpp/util/cli_call_test.cc +++ b/test/cpp/util/cli_call_test.cc @@ -47,6 +47,7 @@ #include "test/core/util/port.h" #include "test/core/util/test_config.h" #include "test/cpp/util/echo.grpc.pb.h" +#include "test/cpp/util/string_ref_helper.h" using grpc::cpp::test::util::EchoRequest; using grpc::cpp::test::util::EchoResponse; @@ -59,10 +60,11 @@ class TestServiceImpl : public ::grpc::cpp::test::util::TestService::Service { Status Echo(ServerContext* context, const EchoRequest* request, EchoResponse* response) GRPC_OVERRIDE { if (!context->client_metadata().empty()) { - for (std::multimap::const_iterator iter = - context->client_metadata().begin(); + for (std::multimap::const_iterator + iter = context->client_metadata().begin(); iter != context->client_metadata().end(); ++iter) { - context->AddInitialMetadata(iter->first, iter->second); + context->AddInitialMetadata(ToString(iter->first), + ToString(iter->second)); } } context->AddTrailingMetadata("trailing_key", "trailing_value"); @@ -119,8 +121,9 @@ TEST_F(CliCallTest, SimpleRpc) { grpc::string request_bin, response_bin, expected_response_bin; EXPECT_TRUE(request.SerializeToString(&request_bin)); EXPECT_TRUE(response.SerializeToString(&expected_response_bin)); - std::multimap client_metadata, - server_initial_metadata, server_trailing_metadata; + std::multimap client_metadata; + std::multimap server_initial_metadata, + server_trailing_metadata; client_metadata.insert(std::pair("key1", "val1")); Status s2 = CliCall::Call(channel_, kMethod, request_bin, &response_bin, client_metadata, &server_initial_metadata, diff --git a/test/cpp/util/grpc_cli.cc b/test/cpp/util/grpc_cli.cc index 746d67deeb9..a4888efebe2 100644 --- a/test/cpp/util/grpc_cli.cc +++ b/test/cpp/util/grpc_cli.cc @@ -68,8 +68,10 @@ #include #include #include +#include #include "test/cpp/util/cli_call.h" +#include "test/cpp/util/string_ref_helper.h" #include "test/cpp/util/test_config.h" DEFINE_bool(enable_ssl, true, "Whether to use ssl/tls."); @@ -104,16 +106,19 @@ void ParseMetadataFlag( } } -void PrintMetadata(const std::multimap& m, - const grpc::string& message) { +template +void PrintMetadata(const T& m, const grpc::string& message) { if (m.empty()) { return; } std::cout << message << std::endl; - for (std::multimap::const_iterator iter = - m.begin(); - iter != m.end(); ++iter) { - std::cout << iter->first << " : " << iter->second << std::endl; + grpc::string pair; + for (typename T::const_iterator iter = m.begin(); iter != m.end(); ++iter) { + pair.clear(); + pair.append(iter->first.data(), iter->first.size()); + pair.append(" : "); + pair.append(iter->second.data(), iter->second.size()); + std::cout << pair << std::endl; } } @@ -157,8 +162,9 @@ int main(int argc, char** argv) { grpc::CreateChannel(server_address, creds, grpc::ChannelArguments()); grpc::string response; - std::multimap client_metadata, - server_initial_metadata, server_trailing_metadata; + std::multimap client_metadata; + std::multimap server_initial_metadata, + server_trailing_metadata; ParseMetadataFlag(&client_metadata); PrintMetadata(client_metadata, "Sending client initial metadata:"); grpc::Status s = grpc::testing::CliCall::Call( diff --git a/test/cpp/util/string_ref_helper.cc b/test/cpp/util/string_ref_helper.cc new file mode 100644 index 00000000000..4eb4fe03575 --- /dev/null +++ b/test/cpp/util/string_ref_helper.cc @@ -0,0 +1,44 @@ +/* + * + * Copyright 2015, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#include "test/cpp/util/string_ref_helper.h" + +namespace grpc { +namespace testing { + +grpc::string ToString(const grpc::string_ref& r) { + return grpc::string(r.data(), r.size()); +} + +} // namespace testing +} // namespace grpc diff --git a/test/cpp/util/string_ref_helper.h b/test/cpp/util/string_ref_helper.h new file mode 100644 index 00000000000..ac94bcd018a --- /dev/null +++ b/test/cpp/util/string_ref_helper.h @@ -0,0 +1,47 @@ +/* + * + * Copyright 2015, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#ifndef GRPC_TEST_CPP_UTIL_STRING_REF_HELPER_H +#define GRPC_TEST_CPP_UTIL_STRING_REF_HELPER_H + +#include + +namespace grpc { +namespace testing { + +grpc::string ToString(const grpc::string_ref& r); + +} // namespace testing +} // namespace grpc + +#endif // GRPC_TEST_CPP_UTIL_STRING_REF_HELPER_H diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index a7fd884cd80..d4dcc92bdec 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -13270,6 +13270,7 @@ "test/cpp/util/echo_duplicate.pb.h", "test/cpp/util/messages.grpc.pb.h", "test/cpp/util/messages.pb.h", + "test/cpp/util/string_ref_helper.h", "test/cpp/util/subprocess.h" ], "language": "c++", @@ -13279,6 +13280,8 @@ "test/cpp/util/cli_call.h", "test/cpp/util/create_test_channel.cc", "test/cpp/util/create_test_channel.h", + "test/cpp/util/string_ref_helper.cc", + "test/cpp/util/string_ref_helper.h", "test/cpp/util/subprocess.cc", "test/cpp/util/subprocess.h" ] diff --git a/vsprojects/Grpc.mak b/vsprojects/Grpc.mak index 2893b72c5de..19cb39741e0 100644 --- a/vsprojects/Grpc.mak +++ b/vsprojects/Grpc.mak @@ -4744,8 +4744,8 @@ Debug\grpc++_test_config.lib: $(OUT_DIR) Debug\grpc++_test_util.lib: $(OUT_DIR) echo Building grpc++_test_util - $(CC) $(CXXFLAGS) /Fo:$(OUT_DIR)\ $(REPO_ROOT)\test\cpp\util\cli_call.cc $(REPO_ROOT)\test\cpp\util\create_test_channel.cc $(REPO_ROOT)\test\cpp\util\subprocess.cc $(REPO_ROOT)\test\cpp\util\messages.pb.cc $(REPO_ROOT)\test\cpp\util\messages.grpc.pb.cc $(REPO_ROOT)\test\cpp\util\echo.pb.cc $(REPO_ROOT)\test\cpp\util\echo.grpc.pb.cc $(REPO_ROOT)\test\cpp\util\echo_duplicate.pb.cc $(REPO_ROOT)\test\cpp\util\echo_duplicate.grpc.pb.cc - $(LIBTOOL) /OUT:"Debug\grpc++_test_util.lib" $(OUT_DIR)\cli_call.obj $(OUT_DIR)\create_test_channel.obj $(OUT_DIR)\subprocess.obj $(OUT_DIR)\messages.pb.obj $(OUT_DIR)\messages.grpc.pb.obj $(OUT_DIR)\echo.pb.obj $(OUT_DIR)\echo.grpc.pb.obj $(OUT_DIR)\echo_duplicate.pb.obj $(OUT_DIR)\echo_duplicate.grpc.pb.obj + $(CC) $(CXXFLAGS) /Fo:$(OUT_DIR)\ $(REPO_ROOT)\test\cpp\util\cli_call.cc $(REPO_ROOT)\test\cpp\util\create_test_channel.cc $(REPO_ROOT)\test\cpp\util\string_ref_helper.cc $(REPO_ROOT)\test\cpp\util\subprocess.cc $(REPO_ROOT)\test\cpp\util\messages.pb.cc $(REPO_ROOT)\test\cpp\util\messages.grpc.pb.cc $(REPO_ROOT)\test\cpp\util\echo.pb.cc $(REPO_ROOT)\test\cpp\util\echo.grpc.pb.cc $(REPO_ROOT)\test\cpp\util\echo_duplicate.pb.cc $(REPO_ROOT)\test\cpp\util\echo_duplicate.grpc.pb.cc + $(LIBTOOL) /OUT:"Debug\grpc++_test_util.lib" $(OUT_DIR)\cli_call.obj $(OUT_DIR)\create_test_channel.obj $(OUT_DIR)\string_ref_helper.obj $(OUT_DIR)\subprocess.obj $(OUT_DIR)\messages.pb.obj $(OUT_DIR)\messages.grpc.pb.obj $(OUT_DIR)\echo.pb.obj $(OUT_DIR)\echo.grpc.pb.obj $(OUT_DIR)\echo_duplicate.pb.obj $(OUT_DIR)\echo_duplicate.grpc.pb.obj build_grpc++_unsecure: msbuild grpc.sln /t:grpc++_unsecure /p:Configuration=Debug /p:Linkage-grpc_dependencies_zlib=static From 25de92c997117da87cfde68f97400aa32d27d814 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 25 Aug 2015 13:48:49 -0700 Subject: [PATCH 15/21] Assert uri not null better --- src/core/client_config/resolver_registry.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/client_config/resolver_registry.c b/src/core/client_config/resolver_registry.c index 886ca5d06d9..37979b3b86c 100644 --- a/src/core/client_config/resolver_registry.c +++ b/src/core/client_config/resolver_registry.c @@ -90,7 +90,7 @@ static grpc_resolver_factory *resolve_factory(const char *target, char *tmp; grpc_resolver_factory *factory = NULL; - GPR_ASSERT(uri); + GPR_ASSERT(uri != NULL); *uri = grpc_uri_parse(target, 1); factory = lookup_factory(*uri); if (factory == NULL) { From beb580b941af9ee24e770d759f01384c04a5cbd7 Mon Sep 17 00:00:00 2001 From: Hongyu Chen Date: Tue, 25 Aug 2015 16:19:33 -0700 Subject: [PATCH 16/21] More merge conflict fix --- BUILD | 6 ++++++ build.json | 2 ++ gRPC.podspec | 4 ++++ tools/doxygen/Doxyfile.core.internal | 4 ---- tools/run_tests/sources_and_headers.json | 8 -------- vsprojects/grpc/grpc.vcxproj | 4 ---- vsprojects/grpc/grpc.vcxproj.filters | 6 ------ vsprojects/grpc_unsecure/grpc_unsecure.vcxproj | 4 ---- vsprojects/grpc_unsecure/grpc_unsecure.vcxproj.filters | 6 ------ 9 files changed, 12 insertions(+), 32 deletions(-) diff --git a/BUILD b/BUILD index 26251f7ba35..d612d9c1ea4 100644 --- a/BUILD +++ b/BUILD @@ -209,6 +209,8 @@ cc_library( "src/core/json/json_reader.h", "src/core/json/json_writer.h", "src/core/profiling/timers.h", + "src/core/statistics/census_interface.h", + "src/core/statistics/census_rpc_stats.h", "src/core/surface/byte_buffer_queue.h", "src/core/surface/call.h", "src/core/surface/channel.h", @@ -475,6 +477,8 @@ cc_library( "src/core/json/json_reader.h", "src/core/json/json_writer.h", "src/core/profiling/timers.h", + "src/core/statistics/census_interface.h", + "src/core/statistics/census_rpc_stats.h", "src/core/surface/byte_buffer_queue.h", "src/core/surface/call.h", "src/core/surface/channel.h", @@ -1228,6 +1232,8 @@ objc_library( "src/core/json/json_reader.h", "src/core/json/json_writer.h", "src/core/profiling/timers.h", + "src/core/statistics/census_interface.h", + "src/core/statistics/census_rpc_stats.h", "src/core/surface/byte_buffer_queue.h", "src/core/surface/call.h", "src/core/surface/channel.h", diff --git a/build.json b/build.json index f96e6b354a6..dec724629e3 100644 --- a/build.json +++ b/build.json @@ -181,6 +181,8 @@ "src/core/json/json_reader.h", "src/core/json/json_writer.h", "src/core/profiling/timers.h", + "src/core/statistics/census_interface.h", + "src/core/statistics/census_rpc_stats.h", "src/core/surface/byte_buffer_queue.h", "src/core/surface/call.h", "src/core/surface/channel.h", diff --git a/gRPC.podspec b/gRPC.podspec index 497c8445729..55f72c93d88 100644 --- a/gRPC.podspec +++ b/gRPC.podspec @@ -211,6 +211,8 @@ Pod::Spec.new do |s| 'src/core/json/json_reader.h', 'src/core/json/json_writer.h', 'src/core/profiling/timers.h', + 'src/core/statistics/census_interface.h', + 'src/core/statistics/census_rpc_stats.h', 'src/core/surface/byte_buffer_queue.h', 'src/core/surface/call.h', 'src/core/surface/channel.h', @@ -481,6 +483,8 @@ Pod::Spec.new do |s| 'src/core/json/json_reader.h', 'src/core/json/json_writer.h', 'src/core/profiling/timers.h', + 'src/core/statistics/census_interface.h', + 'src/core/statistics/census_rpc_stats.h', 'src/core/surface/byte_buffer_queue.h', 'src/core/surface/call.h', 'src/core/surface/channel.h', diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 7bd6e41b5c9..7d5df66bc18 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -845,12 +845,8 @@ src/core/json/json_common.h \ src/core/json/json_reader.h \ src/core/json/json_writer.h \ src/core/profiling/timers.h \ -<<<<<<< HEAD -======= -src/core/profiling/timers_preciseclock.h \ src/core/statistics/census_interface.h \ src/core/statistics/census_rpc_stats.h \ ->>>>>>> upstream/master src/core/surface/byte_buffer_queue.h \ src/core/surface/call.h \ src/core/surface/channel.h \ diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index f238b27ba46..084523cca4e 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -12818,12 +12818,8 @@ "src/core/json/json_reader.h", "src/core/json/json_writer.h", "src/core/profiling/timers.h", -<<<<<<< HEAD -======= - "src/core/profiling/timers_preciseclock.h", "src/core/statistics/census_interface.h", "src/core/statistics/census_rpc_stats.h", ->>>>>>> upstream/master "src/core/surface/byte_buffer_queue.h", "src/core/surface/call.h", "src/core/surface/channel.h", @@ -13009,12 +13005,8 @@ "src/core/profiling/basic_timers.c", "src/core/profiling/stap_timers.c", "src/core/profiling/timers.h", -<<<<<<< HEAD -======= - "src/core/profiling/timers_preciseclock.h", "src/core/statistics/census_interface.h", "src/core/statistics/census_rpc_stats.h", ->>>>>>> upstream/master "src/core/surface/byte_buffer.c", "src/core/surface/byte_buffer_queue.c", "src/core/surface/byte_buffer_queue.h", diff --git a/vsprojects/grpc/grpc.vcxproj b/vsprojects/grpc/grpc.vcxproj index 5921e513ad8..1fa7e4b4f13 100644 --- a/vsprojects/grpc/grpc.vcxproj +++ b/vsprojects/grpc/grpc.vcxproj @@ -307,12 +307,8 @@ -<<<<<<< HEAD -======= - ->>>>>>> upstream/master diff --git a/vsprojects/grpc/grpc.vcxproj.filters b/vsprojects/grpc/grpc.vcxproj.filters index b858f51e966..b22818aebd0 100644 --- a/vsprojects/grpc/grpc.vcxproj.filters +++ b/vsprojects/grpc/grpc.vcxproj.filters @@ -680,18 +680,12 @@ src\core\profiling -<<<<<<< HEAD -======= - - src\core\profiling - src\core\statistics src\core\statistics ->>>>>>> upstream/master src\core\surface diff --git a/vsprojects/grpc_unsecure/grpc_unsecure.vcxproj b/vsprojects/grpc_unsecure/grpc_unsecure.vcxproj index ed66d6c5706..3883a328e09 100644 --- a/vsprojects/grpc_unsecure/grpc_unsecure.vcxproj +++ b/vsprojects/grpc_unsecure/grpc_unsecure.vcxproj @@ -290,12 +290,8 @@ -<<<<<<< HEAD -======= - ->>>>>>> upstream/master diff --git a/vsprojects/grpc_unsecure/grpc_unsecure.vcxproj.filters b/vsprojects/grpc_unsecure/grpc_unsecure.vcxproj.filters index 77216c19772..d12abc0ad70 100644 --- a/vsprojects/grpc_unsecure/grpc_unsecure.vcxproj.filters +++ b/vsprojects/grpc_unsecure/grpc_unsecure.vcxproj.filters @@ -578,18 +578,12 @@ src\core\profiling -<<<<<<< HEAD -======= - - src\core\profiling - src\core\statistics src\core\statistics ->>>>>>> upstream/master src\core\surface From be95249350a50c7b7849c493741e34c07c6d297c Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 25 Aug 2015 16:59:00 -0700 Subject: [PATCH 17/21] Cleanup test, initialize correctly Test was failing on mac because gpr time code was not initialized. Whilst here, clean up the test so that I can understand it again. --- test/core/iomgr/tcp_client_posix_test.c | 31 +++++++++++++++---------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/test/core/iomgr/tcp_client_posix_test.c b/test/core/iomgr/tcp_client_posix_test.c index dea0b33b8e9..f0e2de24d95 100644 --- a/test/core/iomgr/tcp_client_posix_test.c +++ b/test/core/iomgr/tcp_client_posix_test.c @@ -39,10 +39,12 @@ #include #include -#include "src/core/iomgr/iomgr.h" -#include "src/core/iomgr/socket_utils_posix.h" +#include #include #include + +#include "src/core/iomgr/iomgr.h" +#include "src/core/iomgr/socket_utils_posix.h" #include "test/core/util/test_config.h" static grpc_pollset_set g_pollset_set; @@ -198,16 +200,21 @@ void test_times_out(void) { /* Make sure the event doesn't trigger early */ gpr_mu_lock(GRPC_POLLSET_MU(&g_pollset)); - while (gpr_time_cmp(gpr_time_add(connect_deadline, - gpr_time_from_seconds(2, GPR_TIMESPAN)), - gpr_now(connect_deadline.clock_type)) > 0) { - int is_after_deadline = - gpr_time_cmp(connect_deadline, gpr_now(GPR_CLOCK_MONOTONIC)) <= 0; + for (;;) { grpc_pollset_worker worker; + gpr_timespec now = gpr_now(connect_deadline.clock_type); + gpr_timespec continue_verifying_time = gpr_time_from_seconds(2, GPR_TIMESPAN); + gpr_timespec grace_time = gpr_time_from_seconds(1, GPR_TIMESPAN); + gpr_timespec finish_time = gpr_time_add(connect_deadline, continue_verifying_time); + gpr_timespec restart_verifying_time = gpr_time_add(connect_deadline, grace_time); + int is_after_deadline = gpr_time_cmp(now, connect_deadline) > 0; + if (gpr_time_cmp(now, finish_time) > 0) { + break; + } + gpr_log(GPR_DEBUG, "now=%d.%09d connect_deadline=%d.%09d", + now.tv_sec, now.tv_nsec, connect_deadline.tv_sec, connect_deadline.tv_nsec); if (is_after_deadline && - gpr_time_cmp(gpr_time_add(connect_deadline, - gpr_time_from_seconds(1, GPR_TIMESPAN)), - gpr_now(GPR_CLOCK_MONOTONIC)) > 0) { + gpr_time_cmp(now, restart_verifying_time) <= 0) { /* allow some slack before insisting that things be done */ } else { GPR_ASSERT(g_connections_complete == @@ -228,7 +235,7 @@ static void destroy_pollset(void *p) { grpc_pollset_destroy(p); } int main(int argc, char **argv) { grpc_test_init(argc, argv); - grpc_iomgr_init(); + grpc_init(); grpc_pollset_set_init(&g_pollset_set); grpc_pollset_init(&g_pollset); grpc_pollset_set_add_pollset(&g_pollset_set, &g_pollset); @@ -238,6 +245,6 @@ int main(int argc, char **argv) { test_times_out(); grpc_pollset_set_destroy(&g_pollset_set); grpc_pollset_shutdown(&g_pollset, destroy_pollset, &g_pollset); - grpc_iomgr_shutdown(); + grpc_shutdown(); return 0; } From 940caa2d9724054d84f75ec76586e0ff75e34dbb Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 25 Aug 2015 17:50:50 -0700 Subject: [PATCH 18/21] prevent occasional AppDomainUnloadedException from nunit --- tools/run_tests/run_csharp.bat | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/run_tests/run_csharp.bat b/tools/run_tests/run_csharp.bat index c86136767c2..310cfe0d2fe 100644 --- a/tools/run_tests/run_csharp.bat +++ b/tools/run_tests/run_csharp.bat @@ -8,7 +8,7 @@ cd /d %~dp0\..\..\src\csharp @rem set UUID variable to a random GUID, we will use it to put TestResults.xml to a dedicated directory, so that parallel test runs don't collide for /F %%i in ('powershell -Command "[guid]::NewGuid().ToString()"') do (set UUID=%%i) -packages\NUnit.Runners.2.6.4\tools\nunit-console-x86.exe -labels "%1/bin/Debug/%1.dll" -work test-results/%UUID% || goto :error +packages\NUnit.Runners.2.6.4\tools\nunit-console-x86.exe /domain:None -labels "%1/bin/Debug/%1.dll" -work test-results/%UUID% || goto :error endlocal From abd37fd278372e4fb7c5f4c88c214d29728acf66 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 26 Aug 2015 07:54:01 -0700 Subject: [PATCH 19/21] Make run_tests.py robust against port_server not starting --- tools/run_tests/run_tests.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index beb8805ff91..ba0c8e4141b 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -536,7 +536,8 @@ def _start_port_server(port_server_port): # if not running ==> start a new one # otherwise, leave it up try: - version = urllib2.urlopen('http://localhost:%d/version' % port_server_port).read() + version = urllib2.urlopen('http://localhost:%d/version' % port_server_port, + timeout=1).read() running = True except Exception: running = False @@ -553,13 +554,21 @@ def _start_port_server(port_server_port): ['python', 'tools/run_tests/port_server.py', '-p', '%d' % port_server_port], stderr=subprocess.STDOUT, stdout=port_log) - # ensure port server is up + # ensure port server is + waits = 0 while True: + if waits > 10: + port_server.kill() + print "port_server failed to start" + sys.exit(1) try: - urllib2.urlopen('http://localhost:%d/get' % port_server_port).read() + urllib2.urlopen('http://localhost:%d/get' % port_server_port, + timeout=1).read() break except urllib2.URLError: + print "waiting for port_server" time.sleep(0.5) + waits += 1 except: port_server.kill() raise From 8b5f4dcc7f77dcd1c07b14f42de7c5a91052efb3 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 26 Aug 2015 08:02:01 -0700 Subject: [PATCH 20/21] Fix typo --- tools/run_tests/run_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index ba0c8e4141b..80854001d39 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -554,7 +554,7 @@ def _start_port_server(port_server_port): ['python', 'tools/run_tests/port_server.py', '-p', '%d' % port_server_port], stderr=subprocess.STDOUT, stdout=port_log) - # ensure port server is + # ensure port server is up waits = 0 while True: if waits > 10: From 47ec9a40ade6cf5b205b035b1142e0cc539c4f70 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 26 Aug 2015 09:47:51 -0700 Subject: [PATCH 21/21] Add ostream support for string_ref --- include/grpc++/support/string_ref.h | 3 +++ src/cpp/util/string_ref.cc | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/include/grpc++/support/string_ref.h b/include/grpc++/support/string_ref.h index 348c42cbbaa..fd2b3ad8e7b 100644 --- a/include/grpc++/support/string_ref.h +++ b/include/grpc++/support/string_ref.h @@ -35,6 +35,7 @@ #define GRPCXX_STRING_REF_H #include +#include #include @@ -110,6 +111,8 @@ bool operator>(string_ref x, string_ref y); bool operator<=(string_ref x, string_ref y); bool operator>=(string_ref x, string_ref y); +std::ostream& operator<<(std::ostream& stream, const string_ref& string); + } // namespace grpc #endif // GRPCXX_STRING_REF_H diff --git a/src/cpp/util/string_ref.cc b/src/cpp/util/string_ref.cc index d9c9019da89..eb54f65e3ac 100644 --- a/src/cpp/util/string_ref.cc +++ b/src/cpp/util/string_ref.cc @@ -108,4 +108,8 @@ bool operator>=(string_ref x, string_ref y) { return x.compare(y) >= 0; } +std::ostream& operator<<(std::ostream& out, const string_ref& string) { + return out << grpc::string(string.begin(), string.end()); +} + } // namespace grpc