From edc2fffb34cdaaa5274432a197205fc8d3c206be Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 13 Jan 2016 06:54:27 -0800 Subject: [PATCH 01/37] Channel args comparisons --- include/grpc/grpc.h | 9 +++- include/grpc/support/useful.h | 2 + src/core/channel/channel_args.c | 73 +++++++++++++++++++++++--- src/core/channel/channel_args.h | 5 ++ src/core/security/credentials.c | 13 ++++- src/core/security/security_connector.c | 13 ++++- src/core/security/security_context.c | 13 ++++- src/cpp/common/channel_arguments.cc | 22 ++++++-- 8 files changed, 131 insertions(+), 19 deletions(-) diff --git a/include/grpc/grpc.h b/include/grpc/grpc.h index be4c4d2b176..1cf5b5513b3 100644 --- a/include/grpc/grpc.h +++ b/include/grpc/grpc.h @@ -77,6 +77,12 @@ typedef enum { GRPC_ARG_POINTER } grpc_arg_type; +typedef struct grpc_arg_pointer_vtable { + void *(*copy)(void *p); + void (*destroy)(void *p); + int (*cmp)(void *p, void *q); +} grpc_arg_pointer_vtable; + /** A single argument... each argument has a key and a value A note on naming keys: @@ -97,8 +103,7 @@ typedef struct { int integer; struct { void *p; - void *(*copy)(void *p); - void (*destroy)(void *p); + const grpc_arg_pointer_vtable *vtable; } pointer; } value; } grpc_arg; diff --git a/include/grpc/support/useful.h b/include/grpc/support/useful.h index 9f08d788c0d..003e096cf9a 100644 --- a/include/grpc/support/useful.h +++ b/include/grpc/support/useful.h @@ -72,4 +72,6 @@ 0x0f0f0f0f) % \ 255) +#define GPR_ICMP(a, b) ((a) < (b) ? -1 : ((a) > (b) ? 1 : 0)) + #endif /* GRPC_SUPPORT_USEFUL_H */ diff --git a/src/core/channel/channel_args.c b/src/core/channel/channel_args.c index 487db1119a5..cd35d2f701c 100644 --- a/src/core/channel/channel_args.c +++ b/src/core/channel/channel_args.c @@ -54,9 +54,7 @@ static grpc_arg copy_arg(const grpc_arg *src) { break; case GRPC_ARG_POINTER: dst.value.pointer = src->value.pointer; - dst.value.pointer.p = src->value.pointer.copy - ? src->value.pointer.copy(src->value.pointer.p) - : src->value.pointer.p; + dst.value.pointer.p = src->value.pointer.vtable->copy(src->value.pointer.p); break; } return dst; @@ -93,6 +91,60 @@ grpc_channel_args *grpc_channel_args_merge(const grpc_channel_args *a, return grpc_channel_args_copy_and_add(a, b->args, b->num_args); } +static int cmp_arg(const grpc_arg *a, const grpc_arg *b) { + int c = a->type - b->type; + if (c != 0) return c; + c = strcmp(a->key, b->key); + if (c != 0) return c; + switch (a->type) { + case GRPC_ARG_STRING: + c = strcmp(a->value.string, b->value.string); + break; + case GRPC_ARG_INTEGER: + c = GPR_ICMP(a->value.integer, b->value.integer); + break; + case GRPC_ARG_POINTER: + c = GPR_ICMP(a->value.pointer.p, + b->value.pointer.p); + if (c != 0) { + c = GPR_ICMP(a->value.pointer.vtable, + b->value.pointer.vtable); + if (c == 0) { + c = a->value.pointer.vtable->cmp(a->value.pointer.p, + b->value.pointer.p); + } + } + break; + } + return c; +} + +static int cmp_key_stable(const void *ap, const void *bp) { + const grpc_arg *const *a = ap; + const grpc_arg *const *b = bp; + int c = strcmp((*a)->key, (*b)->key); + if (c == 0) c = GPR_ICMP(*a, *b); + return c; +} + +grpc_channel_args *grpc_channel_args_normalize(const grpc_channel_args *a) { + grpc_arg **args = gpr_malloc(sizeof(grpc_arg*) * a->num_args); + for (size_t i = 0; i < a->num_args; i++) { + args[i] = &a->args[i]; + } + qsort(args, a->num_args, sizeof(grpc_arg*), cmp_key_stable); + + grpc_channel_args *b = gpr_malloc(sizeof(grpc_channel_args)); + b->num_args = a->num_args; + b->args = gpr_malloc(sizeof(grpc_arg) * b->num_args); + for (size_t i = 0; i < a->num_args; i++) { + b->args[i] = copy_arg(args[i]); + } + + gpr_free(args); + return b; +} + void grpc_channel_args_destroy(grpc_channel_args *a) { size_t i; for (i = 0; i < a->num_args; i++) { @@ -103,9 +155,7 @@ void grpc_channel_args_destroy(grpc_channel_args *a) { case GRPC_ARG_INTEGER: break; case GRPC_ARG_POINTER: - if (a->args[i].value.pointer.destroy) { - a->args[i].value.pointer.destroy(a->args[i].value.pointer.p); - } + a->args[i].value.pointer.vtable->destroy(a->args[i].value.pointer.p); break; } gpr_free(a->args[i].key); @@ -207,3 +257,14 @@ int grpc_channel_args_compression_algorithm_get_states( return (1u << GRPC_COMPRESS_ALGORITHMS_COUNT) - 1; /* All algs. enabled */ } } + +int grpc_channel_args_compare(const grpc_channel_args *a, + const grpc_channel_args *b) { + int c = GPR_ICMP(a->num_args, b->num_args); + if (c != 0) return c; + for (size_t i = 0; i < a->num_args; i++) { + c = cmp_arg(&a->args[i], &b->args[i]); + if (c != 0) return c; + } + return 0; +} diff --git a/src/core/channel/channel_args.h b/src/core/channel/channel_args.h index 480cc9aec2f..ce848782e3f 100644 --- a/src/core/channel/channel_args.h +++ b/src/core/channel/channel_args.h @@ -40,6 +40,9 @@ /* Copy some arguments */ grpc_channel_args *grpc_channel_args_copy(const grpc_channel_args *src); +/* Copy some arguments, stably sorting keys */ +grpc_channel_args *grpc_channel_args_normalize(const grpc_channel_args *a); + /** Copy some arguments and add the to_add parameter in the end. If to_add is NULL, it is equivalent to call grpc_channel_args_copy. */ grpc_channel_args *grpc_channel_args_copy_and_add(const grpc_channel_args *src, @@ -85,4 +88,6 @@ grpc_channel_args *grpc_channel_args_compression_algorithm_set_state( int grpc_channel_args_compression_algorithm_get_states( const grpc_channel_args *a); +int grpc_channel_args_compare(const grpc_channel_args *a, const grpc_channel_args *b); + #endif /* GRPC_INTERNAL_CORE_CHANNEL_CHANNEL_ARGS_H */ diff --git a/src/core/security/credentials.c b/src/core/security/credentials.c index 8b56c576458..28f41b2041f 100644 --- a/src/core/security/credentials.c +++ b/src/core/security/credentials.c @@ -196,14 +196,23 @@ static void *server_credentials_pointer_arg_copy(void *p) { return grpc_server_credentials_ref(p); } +static int server_credentials_pointer_cmp(void *a, void *b) { + return GPR_ICMP(a, b); +} + +static const grpc_arg_pointer_vtable cred_ptr_vtable = { + server_credentials_pointer_arg_copy, + server_credentials_pointer_arg_destroy, + server_credentials_pointer_cmp +}; + grpc_arg grpc_server_credentials_to_arg(grpc_server_credentials *p) { grpc_arg arg; memset(&arg, 0, sizeof(grpc_arg)); arg.type = GRPC_ARG_POINTER; arg.key = GRPC_SERVER_CREDENTIALS_ARG; arg.value.pointer.p = p; - arg.value.pointer.copy = server_credentials_pointer_arg_copy; - arg.value.pointer.destroy = server_credentials_pointer_arg_destroy; + arg.value.pointer.vtable = &cred_ptr_vtable; return arg; } diff --git a/src/core/security/security_connector.c b/src/core/security/security_connector.c index 61336a1057d..40f486128b7 100644 --- a/src/core/security/security_connector.c +++ b/src/core/security/security_connector.c @@ -194,12 +194,21 @@ static void *connector_pointer_arg_copy(void *p) { return GRPC_SECURITY_CONNECTOR_REF(p, "connector_pointer_arg"); } +static int connector_pointer_cmp(void *a, void *b) { + return GPR_ICMP(a, b); +} + +static const grpc_arg_pointer_vtable connector_pointer_vtable = { + connector_pointer_arg_copy, + connector_pointer_arg_destroy, + connector_pointer_cmp +}; + grpc_arg grpc_security_connector_to_arg(grpc_security_connector *sc) { grpc_arg result; result.type = GRPC_ARG_POINTER; result.key = GRPC_SECURITY_CONNECTOR_ARG; - result.value.pointer.destroy = connector_pointer_arg_destroy; - result.value.pointer.copy = connector_pointer_arg_copy; + result.value.pointer.vtable = &connector_pointer_vtable; result.value.pointer.p = sc; return result; } diff --git a/src/core/security/security_context.c b/src/core/security/security_context.c index 2068c97d78c..6e948f61bc3 100644 --- a/src/core/security/security_context.c +++ b/src/core/security/security_context.c @@ -309,14 +309,23 @@ static void *auth_context_pointer_arg_copy(void *p) { return GRPC_AUTH_CONTEXT_REF(p, "auth_context_pointer_arg"); } +static int auth_context_pointer_cmp(void *a, void *b) { + return GPR_ICMP(a, b); +} + +static const grpc_arg_pointer_vtable auth_context_pointer_vtable = { + auth_context_pointer_arg_copy, + auth_context_pointer_arg_destroy, + auth_context_pointer_cmp +}; + grpc_arg grpc_auth_context_to_arg(grpc_auth_context *p) { grpc_arg arg; memset(&arg, 0, sizeof(grpc_arg)); arg.type = GRPC_ARG_POINTER; arg.key = GRPC_AUTH_CONTEXT_ARG; arg.value.pointer.p = p; - arg.value.pointer.copy = auth_context_pointer_arg_copy; - arg.value.pointer.destroy = auth_context_pointer_arg_destroy; + arg.value.pointer.vtable = &auth_context_pointer_vtable; return arg; } diff --git a/src/cpp/common/channel_arguments.cc b/src/cpp/common/channel_arguments.cc index 90cd5136af3..3536e354287 100644 --- a/src/cpp/common/channel_arguments.cc +++ b/src/cpp/common/channel_arguments.cc @@ -62,9 +62,7 @@ ChannelArguments::ChannelArguments(const ChannelArguments& other) break; case GRPC_ARG_POINTER: ap.value.pointer = a->value.pointer; - ap.value.pointer.p = a->value.pointer.copy - ? a->value.pointer.copy(ap.value.pointer.p) - : ap.value.pointer.p; + ap.value.pointer.p = a->value.pointer.vtable->copy(ap.value.pointer.p); break; } args_.push_back(ap); @@ -92,13 +90,27 @@ void ChannelArguments::SetInt(const grpc::string& key, int value) { } void ChannelArguments::SetPointer(const grpc::string& key, void* value) { + struct VtableMembers { + static void* Copy(void* in) { return in; } + static void Destroy(void* in) {} + static int Compare(void* a, void *b) { + if (a < b) return -1; + if (a > b) return 1; + return 0; + } + }; + static const grpc_arg_pointer_vtable vtable = { + &VtableMembers::Copy, + &VtableMembers::Destroy, + &VtableMembers::Compare + }; + grpc_arg arg; arg.type = GRPC_ARG_POINTER; strings_.push_back(key); arg.key = const_cast(strings_.back().c_str()); arg.value.pointer.p = value; - arg.value.pointer.copy = nullptr; - arg.value.pointer.destroy = nullptr; + arg.value.pointer.vtable = &vtable; args_.push_back(arg); } From 694cf8b0d266f4fadcc5b52f1bfc6f2c0dd1ccb5 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 15 Jan 2016 21:13:25 -0800 Subject: [PATCH 02/37] Shared subchannel sketch --- BUILD | 6 + Makefile | 2 + binding.gyp | 1 + build.yaml | 2 + gRPC.podspec | 3 + src/core/client_config/subchannel.c | 1 + src/core/client_config/subchannel_index.c | 155 ++++++++++++++++++ src/core/client_config/subchannel_index.h | 39 +++++ src/core/surface/init.c | 3 + src/python/grpcio/grpc_core_dependencies.py | 1 + tools/doxygen/Doxyfile.core.internal | 2 + tools/run_tests/sources_and_headers.json | 6 + vsprojects/vcxproj/grpc/grpc.vcxproj | 3 + vsprojects/vcxproj/grpc/grpc.vcxproj.filters | 6 + .../grpc_unsecure/grpc_unsecure.vcxproj | 3 + .../grpc_unsecure.vcxproj.filters | 6 + 16 files changed, 239 insertions(+) create mode 100644 src/core/client_config/subchannel_index.c create mode 100644 src/core/client_config/subchannel_index.h diff --git a/BUILD b/BUILD index 2b386fb814c..a74d2d1c58b 100644 --- a/BUILD +++ b/BUILD @@ -178,6 +178,7 @@ cc_library( "src/core/client_config/resolvers/sockaddr_resolver.h", "src/core/client_config/subchannel.h", "src/core/client_config/subchannel_factory.h", + "src/core/client_config/subchannel_index.h", "src/core/client_config/uri_parser.h", "src/core/compression/algorithm_metadata.h", "src/core/compression/message_compress.h", @@ -316,6 +317,7 @@ cc_library( "src/core/client_config/resolvers/sockaddr_resolver.c", "src/core/client_config/subchannel.c", "src/core/client_config/subchannel_factory.c", + "src/core/client_config/subchannel_index.c", "src/core/client_config/uri_parser.c", "src/core/compression/algorithm.c", "src/core/compression/message_compress.c", @@ -469,6 +471,7 @@ cc_library( "src/core/client_config/resolvers/sockaddr_resolver.h", "src/core/client_config/subchannel.h", "src/core/client_config/subchannel_factory.h", + "src/core/client_config/subchannel_index.h", "src/core/client_config/uri_parser.h", "src/core/compression/algorithm_metadata.h", "src/core/compression/message_compress.h", @@ -587,6 +590,7 @@ cc_library( "src/core/client_config/resolvers/sockaddr_resolver.c", "src/core/client_config/subchannel.c", "src/core/client_config/subchannel_factory.c", + "src/core/client_config/subchannel_index.c", "src/core/client_config/uri_parser.c", "src/core/compression/algorithm.c", "src/core/compression/message_compress.c", @@ -1121,6 +1125,7 @@ objc_library( "src/core/client_config/resolvers/sockaddr_resolver.c", "src/core/client_config/subchannel.c", "src/core/client_config/subchannel_factory.c", + "src/core/client_config/subchannel_index.c", "src/core/client_config/uri_parser.c", "src/core/compression/algorithm.c", "src/core/compression/message_compress.c", @@ -1272,6 +1277,7 @@ objc_library( "src/core/client_config/resolvers/sockaddr_resolver.h", "src/core/client_config/subchannel.h", "src/core/client_config/subchannel_factory.h", + "src/core/client_config/subchannel_index.h", "src/core/client_config/uri_parser.h", "src/core/compression/algorithm_metadata.h", "src/core/compression/message_compress.h", diff --git a/Makefile b/Makefile index 52414cffc18..78c6cb0e955 100644 --- a/Makefile +++ b/Makefile @@ -2349,6 +2349,7 @@ LIBGRPC_SRC = \ src/core/client_config/resolvers/sockaddr_resolver.c \ src/core/client_config/subchannel.c \ src/core/client_config/subchannel_factory.c \ + src/core/client_config/subchannel_index.c \ src/core/client_config/uri_parser.c \ src/core/compression/algorithm.c \ src/core/compression/message_compress.c \ @@ -2651,6 +2652,7 @@ LIBGRPC_UNSECURE_SRC = \ src/core/client_config/resolvers/sockaddr_resolver.c \ src/core/client_config/subchannel.c \ src/core/client_config/subchannel_factory.c \ + src/core/client_config/subchannel_index.c \ src/core/client_config/uri_parser.c \ src/core/compression/algorithm.c \ src/core/compression/message_compress.c \ diff --git a/binding.gyp b/binding.gyp index e5b4b9722e8..888d04cbf30 100644 --- a/binding.gyp +++ b/binding.gyp @@ -210,6 +210,7 @@ 'src/core/client_config/resolvers/sockaddr_resolver.c', 'src/core/client_config/subchannel.c', 'src/core/client_config/subchannel_factory.c', + 'src/core/client_config/subchannel_index.c', 'src/core/client_config/uri_parser.c', 'src/core/compression/algorithm.c', 'src/core/compression/message_compress.c', diff --git a/build.yaml b/build.yaml index aa30bae9f6c..ae3fb854def 100644 --- a/build.yaml +++ b/build.yaml @@ -132,6 +132,7 @@ filegroups: - src/core/client_config/resolvers/sockaddr_resolver.h - src/core/client_config/subchannel.h - src/core/client_config/subchannel_factory.h + - src/core/client_config/subchannel_index.h - src/core/client_config/uri_parser.h - src/core/compression/algorithm_metadata.h - src/core/compression/message_compress.h @@ -247,6 +248,7 @@ filegroups: - src/core/client_config/resolvers/sockaddr_resolver.c - src/core/client_config/subchannel.c - src/core/client_config/subchannel_factory.c + - src/core/client_config/subchannel_index.c - src/core/client_config/uri_parser.c - src/core/compression/algorithm.c - src/core/compression/message_compress.c diff --git a/gRPC.podspec b/gRPC.podspec index 80f26817d09..a49ef2722a0 100644 --- a/gRPC.podspec +++ b/gRPC.podspec @@ -182,6 +182,7 @@ Pod::Spec.new do |s| 'src/core/client_config/resolvers/sockaddr_resolver.h', 'src/core/client_config/subchannel.h', 'src/core/client_config/subchannel_factory.h', + 'src/core/client_config/subchannel_index.h', 'src/core/client_config/uri_parser.h', 'src/core/compression/algorithm_metadata.h', 'src/core/compression/message_compress.h', @@ -327,6 +328,7 @@ Pod::Spec.new do |s| 'src/core/client_config/resolvers/sockaddr_resolver.c', 'src/core/client_config/subchannel.c', 'src/core/client_config/subchannel_factory.c', + 'src/core/client_config/subchannel_index.c', 'src/core/client_config/uri_parser.c', 'src/core/compression/algorithm.c', 'src/core/compression/message_compress.c', @@ -480,6 +482,7 @@ Pod::Spec.new do |s| 'src/core/client_config/resolvers/sockaddr_resolver.h', 'src/core/client_config/subchannel.h', 'src/core/client_config/subchannel_factory.h', + 'src/core/client_config/subchannel_index.h', 'src/core/client_config/uri_parser.h', 'src/core/compression/algorithm_metadata.h', 'src/core/compression/message_compress.h', diff --git a/src/core/client_config/subchannel.c b/src/core/client_config/subchannel.c index 2992da8b79d..9f287c4b03d 100644 --- a/src/core/client_config/subchannel.c +++ b/src/core/client_config/subchannel.c @@ -36,6 +36,7 @@ #include #include +#include #include "src/core/channel/channel_args.h" #include "src/core/channel/client_channel.h" diff --git a/src/core/client_config/subchannel_index.c b/src/core/client_config/subchannel_index.c new file mode 100644 index 00000000000..ffe6c1fe931 --- /dev/null +++ b/src/core/client_config/subchannel_index.c @@ -0,0 +1,155 @@ +/* + * + * 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. + * + */ + +/* a map of subchannel_key --> subchannel, used for detecting connections + to the same destination in order to share them */ +static gpr_avl g_subchannel_index; + +static gpr_mu g_mu; + +struct subchannel_key { + size_t addr_len; + struct sockaddr *addr; + grpc_channel_args *normalized_args; +}; + +GPR_TLS_DECL(subchannel_index_exec_ctx); + +static subchannel_key *subchannel_key_create(struct sockaddr *sockaddr, size_t addr_len, grpc_channel_args *args) { + subchannel_key *k = gpr_malloc(sizeof(*k)); + k->addr_len = addr_len; + k->addr = gpr_malloc(addr_len); + memcpy(k->addr, addr, addr_len); + k->normalized_args = grpc_channel_args_normalize(args); + return k; +} + +static subchannel_key *subchannel_key_copy(subchannel_key *k) { + subchannel_key *k = gpr_malloc(sizeof(*k)); + k->addr_len = addr_len; + k->addr = gpr_malloc(addr_len); + memcpy(k->addr, addr, addr_len); + k->normalized_args = grpc_channel_args_copy(args); + return k; +} + +static int subchannel_key_compare(subchannel_key *a, subchannel_key *b) { + int c = GPR_ICMP(a->addr_len, b->addr_len); + if (c != 0) return c; + c = memcmp(a->addr, b->addr, a->addr_len); + if (c != 0) return c; + return grpc_channel_args_compare(a->normalized_args, b->normalized_args); +} + +static void subchannel_key_destroy(subchannel_key *k) { + gpr_free(k->addr); + grpc_channel_args_destroy(k->normalized_args); + gpr_free(k); +} + +static void sck_avl_destroy(void *p) { + subchannel_key_destroy(p); +} + +static void *sck_avl_copy(void *p) { + return subchannel_key_copy(p); +} + +static void *sck_avl_compare(void *a, void *b) { + return subchannel_key_compare(a, b); +} + +static void scv_avl_destroy(void *p) { + GRPC_SUBCHANNEL_UNREF(exec_ctx, p, "subchannel_index"); +} + +static void *scv_avl_copy(void *p) { + GRPC_SUBCHANNEL_REF(p, "subchannel_index"); + return p; +} + +static const gpr_avl_vtable subchannel_avl_vtable = { + .destroy_key = sck_avl_destroy, + .copy_key = sck_avl_copy, + .compare_keys = sck_avl_compare, + .destroy_value = scv_avl_destroy, + .copy_value = scv_avl_copy +}; + +grpc_subchannel *grpc_subchannel_index_find( + grpc_exec_ctx *ctx, + grpc_connector *connector, + grpc_subchannel_args *args) { + gpr_mu_lock(&g_mu); + gpr_avl index = gpr_avl_ref(g_subchannel_index); + gpr_mu_unlock(&g_mu); + + subchannel_key *key = subchannel_key_create(connector, args); + grpc_subchannel *c = grpc_subchannel_ref(gpr_avl_get(index, key)); + subchannel_key_destroy(key); + gpr_avl_unref(index); + + return c; +} + +grpc_subchannel *grpc_subchannel_index_register( + grpc_exec_ctx *ctx, + grpc_connector *connector, + grpc_subchannel_args *args, + grpc_subchannel *constructed) { + subchannel_key *key = subchannel_key_create(connector, args); + grpc_subchannel *c = NULL; + + while (c == NULL) { + gpr_mu_lock(&g_mu); + gpr_avl index = gpr_avl_ref(g_subchannel_index); + gpr_mu_unlock(&g_mu); + + c = gpr_avl_get(index, key); + if (c != NULL) { + GRPC_SUBCHANNEL_UNREF(constructed); + } else { + gpr_avl updated = gpr_avl_add(index, key, constructed); + + gpr_mu_lock(&g_mu); + if (index.root == g_subchannel_index.root) { + GPR_SWAP(index, g_subchannel_index); + c = constructed; + } + gpr_mu_unlock(&g_mu); + } + gpr_avl_unref(index); + } + + return c; +} diff --git a/src/core/client_config/subchannel_index.h b/src/core/client_config/subchannel_index.h new file mode 100644 index 00000000000..d501e121f1d --- /dev/null +++ b/src/core/client_config/subchannel_index.h @@ -0,0 +1,39 @@ +/* + * + * 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_INTERNAL_CORE_CLIENT_CONFIG_SUBCHANNEL_INDEX_H +#define GRPC_INTERNAL_CORE_CLIENT_CONFIG_SUBCHANNEL_INDEX_H + + + +#endif /* GRPC_INTERNAL_CORE_CLIENT_CONFIG_SUBCHANNEL_INDEX_H */ diff --git a/src/core/surface/init.c b/src/core/surface/init.c index 19cea4c4f6e..8f1936227ed 100644 --- a/src/core/surface/init.c +++ b/src/core/surface/init.c @@ -46,6 +46,7 @@ #include "src/core/client_config/resolver_registry.h" #include "src/core/client_config/resolvers/dns_resolver.h" #include "src/core/client_config/resolvers/sockaddr_resolver.h" +#include "src/core/client_config/subchannel.h" #include "src/core/debug/trace.h" #include "src/core/iomgr/executor.h" #include "src/core/iomgr/iomgr.h" @@ -125,6 +126,7 @@ void grpc_init(void) { } gpr_timers_global_init(); grpc_cq_global_init(); + grpc_subchannel_global_init(); for (i = 0; i < g_number_of_plugins; i++) { if (g_all_of_the_plugins[i].init != NULL) { g_all_of_the_plugins[i].init(); @@ -143,6 +145,7 @@ void grpc_shutdown(void) { grpc_executor_shutdown(); grpc_cq_global_shutdown(); grpc_iomgr_shutdown(); + grpc_subchannel_global_shutdown(); census_shutdown(); gpr_timers_global_destroy(); grpc_tracer_shutdown(); diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 63cf0a4c74c..8963e30de4b 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -119,6 +119,7 @@ CORE_SOURCE_FILES = [ 'src/core/client_config/resolvers/sockaddr_resolver.c', 'src/core/client_config/subchannel.c', 'src/core/client_config/subchannel_factory.c', + 'src/core/client_config/subchannel_index.c', 'src/core/client_config/uri_parser.c', 'src/core/compression/algorithm.c', 'src/core/compression/message_compress.c', diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index aef5bec86b0..9826eaa1a9a 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -807,6 +807,7 @@ src/core/client_config/resolvers/dns_resolver.h \ src/core/client_config/resolvers/sockaddr_resolver.h \ src/core/client_config/subchannel.h \ src/core/client_config/subchannel_factory.h \ +src/core/client_config/subchannel_index.h \ src/core/client_config/uri_parser.h \ src/core/compression/algorithm_metadata.h \ src/core/compression/message_compress.h \ @@ -945,6 +946,7 @@ src/core/client_config/resolvers/dns_resolver.c \ src/core/client_config/resolvers/sockaddr_resolver.c \ src/core/client_config/subchannel.c \ src/core/client_config/subchannel_factory.c \ +src/core/client_config/subchannel_index.c \ src/core/client_config/uri_parser.c \ src/core/compression/algorithm.c \ src/core/compression/message_compress.c \ diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index 8d86fa3bf34..99d4ede22c3 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -2885,6 +2885,7 @@ "src/core/client_config/resolvers/sockaddr_resolver.h", "src/core/client_config/subchannel.h", "src/core/client_config/subchannel_factory.h", + "src/core/client_config/subchannel_index.h", "src/core/client_config/uri_parser.h", "src/core/compression/algorithm_metadata.h", "src/core/compression/message_compress.h", @@ -3057,6 +3058,8 @@ "src/core/client_config/subchannel.h", "src/core/client_config/subchannel_factory.c", "src/core/client_config/subchannel_factory.h", + "src/core/client_config/subchannel_index.c", + "src/core/client_config/subchannel_index.h", "src/core/client_config/uri_parser.c", "src/core/client_config/uri_parser.h", "src/core/compression/algorithm.c", @@ -3397,6 +3400,7 @@ "src/core/client_config/resolvers/sockaddr_resolver.h", "src/core/client_config/subchannel.h", "src/core/client_config/subchannel_factory.h", + "src/core/client_config/subchannel_index.h", "src/core/client_config/uri_parser.h", "src/core/compression/algorithm_metadata.h", "src/core/compression/message_compress.h", @@ -3554,6 +3558,8 @@ "src/core/client_config/subchannel.h", "src/core/client_config/subchannel_factory.c", "src/core/client_config/subchannel_factory.h", + "src/core/client_config/subchannel_index.c", + "src/core/client_config/subchannel_index.h", "src/core/client_config/uri_parser.c", "src/core/client_config/uri_parser.h", "src/core/compression/algorithm.c", diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj b/vsprojects/vcxproj/grpc/grpc.vcxproj index 9d646153e28..34bb322c2b3 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj @@ -314,6 +314,7 @@ + @@ -502,6 +503,8 @@ + + diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters index 055256a7e81..1681800056a 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters @@ -145,6 +145,9 @@ src\core\client_config + + src\core\client_config + src\core\client_config @@ -596,6 +599,9 @@ src\core\client_config + + src\core\client_config + src\core\client_config diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj index b2965212bb4..749429297e7 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj @@ -292,6 +292,7 @@ + @@ -440,6 +441,8 @@ + + diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters index e30ca5f685d..8b57305cc42 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters @@ -85,6 +85,9 @@ src\core\client_config + + src\core\client_config + src\core\client_config @@ -491,6 +494,9 @@ src\core\client_config + + src\core\client_config + src\core\client_config From 7391f133375f0840a4219db24f9a93a96887742e Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 22 Jan 2016 06:39:54 -0800 Subject: [PATCH 03/37] subchannel progress --- src/core/client_config/connector.c | 3 +- src/core/client_config/connector.h | 2 +- src/core/client_config/subchannel.c | 21 ++++- src/core/client_config/subchannel.h | 8 +- src/core/client_config/subchannel_index.c | 104 +++++++++++++++------- src/core/client_config/subchannel_index.h | 22 +++++ src/core/surface/secure_channel_create.c | 2 +- 7 files changed, 123 insertions(+), 39 deletions(-) diff --git a/src/core/client_config/connector.c b/src/core/client_config/connector.c index 1603ffb8be0..eaa215fe8f6 100644 --- a/src/core/client_config/connector.c +++ b/src/core/client_config/connector.c @@ -33,8 +33,9 @@ #include "src/core/client_config/connector.h" -void grpc_connector_ref(grpc_connector* connector) { +grpc_connector *grpc_connector_ref(grpc_connector* connector) { connector->vtable->ref(connector); + return connector; } void grpc_connector_unref(grpc_exec_ctx* exec_ctx, grpc_connector* connector) { diff --git a/src/core/client_config/connector.h b/src/core/client_config/connector.h index b4482fa2eeb..b301e1bb197 100644 --- a/src/core/client_config/connector.h +++ b/src/core/client_config/connector.h @@ -81,7 +81,7 @@ struct grpc_connector_vtable { grpc_connect_out_args *out_args, grpc_closure *notify); }; -void grpc_connector_ref(grpc_connector *connector); +grpc_connector *grpc_connector_ref(grpc_connector *connector); void grpc_connector_unref(grpc_exec_ctx *exec_ctx, grpc_connector *connector); /** Connect using the connector: max one outstanding call at a time */ void grpc_connector_connect(grpc_exec_ctx *exec_ctx, grpc_connector *connector, diff --git a/src/core/client_config/subchannel.c b/src/core/client_config/subchannel.c index 9f287c4b03d..bf2f15a4443 100644 --- a/src/core/client_config/subchannel.c +++ b/src/core/client_config/subchannel.c @@ -42,11 +42,11 @@ #include "src/core/channel/client_channel.h" #include "src/core/channel/connected_channel.h" #include "src/core/client_config/initial_connect_string.h" +#include "src/core/client_config/subchannel_index.h" #include "src/core/iomgr/timer.h" #include "src/core/profiling/timers.h" #include "src/core/surface/channel.h" #include "src/core/transport/connectivity_state.h" -#include "src/core/transport/connectivity_state.h" #define INTERNAL_REF_BITS 16 #define STRONG_REF_MASK (~(gpr_atm)((1 << INTERNAL_REF_BITS) - 1)) @@ -95,6 +95,8 @@ struct grpc_subchannel { struct sockaddr *addr; size_t addr_len; + grpc_subchannel_key *key; + /** initial string to send to peer */ gpr_slice initial_connect_string; @@ -239,6 +241,7 @@ void grpc_subchannel_weak_ref(grpc_subchannel *c static void disconnect(grpc_exec_ctx *exec_ctx, grpc_subchannel *c) { grpc_connected_subchannel *con; + grpc_subchannel_index_unregister(exec_ctx, c->key, c); gpr_mu_lock(&c->mu); GPR_ASSERT(!c->disconnected); c->disconnected = 1; @@ -277,10 +280,19 @@ static uint32_t random_seed() { return (uint32_t)(gpr_time_to_millis(gpr_now(GPR_CLOCK_MONOTONIC))); } -grpc_subchannel *grpc_subchannel_create(grpc_connector *connector, +grpc_subchannel *grpc_subchannel_create(grpc_exec_ctx *exec_ctx, + grpc_connector *connector, grpc_subchannel_args *args) { - grpc_subchannel *c = gpr_malloc(sizeof(*c)); + grpc_subchannel_key *key = grpc_subchannel_key_create(connector, args); + grpc_subchannel *c = grpc_subchannel_index_find(exec_ctx, key); + if (c) { + grpc_subchannel_key_destroy(key); + return c; + } + + c = gpr_malloc(sizeof(*c)); memset(c, 0, sizeof(*c)); + c->key = key; gpr_atm_no_barrier_store(&c->ref_pair, 1 << INTERNAL_REF_BITS); c->connector = connector; grpc_connector_ref(c->connector); @@ -302,7 +314,8 @@ grpc_subchannel *grpc_subchannel_create(grpc_connector *connector, grpc_connectivity_state_init(&c->state_tracker, GRPC_CHANNEL_IDLE, "subchannel"); gpr_mu_init(&c->mu); - return c; + + return grpc_subchannel_index_register(exec_ctx, key, c); } static void continue_connect(grpc_exec_ctx *exec_ctx, grpc_subchannel *c) { diff --git a/src/core/client_config/subchannel.h b/src/core/client_config/subchannel.h index 57c7c9dc674..8fd976276b5 100644 --- a/src/core/client_config/subchannel.h +++ b/src/core/client_config/subchannel.h @@ -48,6 +48,8 @@ typedef struct grpc_subchannel_args grpc_subchannel_args; #ifdef GRPC_STREAM_REFCOUNT_DEBUG #define GRPC_SUBCHANNEL_REF(p, r) \ grpc_subchannel_ref((p), __FILE__, __LINE__, (r)) +#define GRPC_SUBCHANNEL_REF_FROM_WEAK_REF(p, r) \ + grpc_subchannel_ref_from_weak_ref((p), __FILE__, __LINE__, (r)) #define GRPC_SUBCHANNEL_UNREF(cl, p, r) \ grpc_subchannel_unref((cl), (p), __FILE__, __LINE__, (r)) #define GRPC_SUBCHANNEL_WEAK_REF(p, r) \ @@ -66,6 +68,7 @@ typedef struct grpc_subchannel_args grpc_subchannel_args; , const char *file, int line, const char *reason #else #define GRPC_SUBCHANNEL_REF(p, r) grpc_subchannel_ref((p)) +#define GRPC_SUBCHANNEL_REF_FROM_WEAK_REF(p, r) grpc_subchannel_ref_from_weak_ref((p)) #define GRPC_SUBCHANNEL_UNREF(cl, p, r) grpc_subchannel_unref((cl), (p)) #define GRPC_SUBCHANNEL_WEAK_REF(p, r) grpc_subchannel_weak_ref((p)) #define GRPC_SUBCHANNEL_WEAK_UNREF(cl, p, r) \ @@ -146,6 +149,8 @@ grpc_call_stack *grpc_subchannel_call_get_call_stack( grpc_subchannel_call *subchannel_call); struct grpc_subchannel_args { + /* When updating this struct, also update subchannel_index.c */ + /** Channel filters for this channel - wrapped factories will likely want to mutate this */ const grpc_channel_filter **filters; @@ -159,7 +164,8 @@ struct grpc_subchannel_args { }; /** create a subchannel given a connector */ -grpc_subchannel *grpc_subchannel_create(grpc_connector *connector, +grpc_subchannel *grpc_subchannel_create(grpc_exec_ctx *exec_ctx, + grpc_connector *connector, grpc_subchannel_args *args); #endif /* GRPC_INTERNAL_CORE_CLIENT_CONFIG_SUBCHANNEL_H */ diff --git a/src/core/client_config/subchannel_index.c b/src/core/client_config/subchannel_index.c index ffe6c1fe931..9f6ecca2959 100644 --- a/src/core/client_config/subchannel_index.c +++ b/src/core/client_config/subchannel_index.c @@ -31,49 +31,83 @@ * */ +#include "src/core/client_config/subchannel_index.h" + +#include + +#include +#include +#include + +#include "src/core/channel/channel_args.h" + /* a map of subchannel_key --> subchannel, used for detecting connections to the same destination in order to share them */ static gpr_avl g_subchannel_index; static gpr_mu g_mu; -struct subchannel_key { - size_t addr_len; - struct sockaddr *addr; - grpc_channel_args *normalized_args; +struct grpc_subchannel_key { + grpc_connector *connector; + grpc_subchannel_args args; }; GPR_TLS_DECL(subchannel_index_exec_ctx); -static subchannel_key *subchannel_key_create(struct sockaddr *sockaddr, size_t addr_len, grpc_channel_args *args) { - subchannel_key *k = gpr_malloc(sizeof(*k)); - k->addr_len = addr_len; - k->addr = gpr_malloc(addr_len); - memcpy(k->addr, addr, addr_len); - k->normalized_args = grpc_channel_args_normalize(args); - return k; +static void enter_ctx(grpc_exec_ctx *exec_ctx) { + GPR_ASSERT(gpr_tls_get(&subchannel_index_exec_ctx) == 0); + gpr_tls_set(&subchannel_index_exec_ctx, (intptr_t)exec_ctx); +} + +static void leave_ctx(grpc_exec_ctx *exec_ctx) { + GPR_ASSERT(gpr_tls_get(&subchannel_index_exec_ctx) == (intptr_t)exec_ctx); + gpr_tls_set(&subchannel_index_exec_ctx, 0); +} + +static grpc_exec_ctx *current_ctx() { + grpc_exec_ctx *c = (grpc_exec_ctx *)gpr_tls_get(&subchannel_index_exec_ctx); + GPR_ASSERT(c != NULL); + return c; } -static subchannel_key *subchannel_key_copy(subchannel_key *k) { - subchannel_key *k = gpr_malloc(sizeof(*k)); - k->addr_len = addr_len; - k->addr = gpr_malloc(addr_len); - memcpy(k->addr, addr, addr_len); - k->normalized_args = grpc_channel_args_copy(args); +static grpc_subchannel_key *create_key(grpc_connector *connector, grpc_subchannel_args *args, grpc_channel_args *(*copy_channel_args)(const grpc_channel_args *args)) { + grpc_subchannel_key *k = gpr_malloc(sizeof(*k)); + k->connector = grpc_connector_ref(connector); + k->args.filter_count = args->filter_count; + k->args.filters = gpr_malloc(sizeof(*k->args.filters) * k->args.filter_count); + memcpy(k->args.filters, args->filters, sizeof(*k->args.filters) * k->args.filter_count); + k->args.addr_len = args->addr_len; + k->args.addr = gpr_malloc(args->addr_len); + memcpy(k->args.addr, args->addr, k->args.addr_len); + k->args.args = copy_channel_args(args->args); return k; } -static int subchannel_key_compare(subchannel_key *a, subchannel_key *b) { - int c = GPR_ICMP(a->addr_len, b->addr_len); +grpc_subchannel_key *grpc_subchannel_key_create(grpc_connector *connector, grpc_subchannel_args *args) { + return create_key(connector, args, grpc_channel_args_normalize); +} + +static grpc_subchannel_key *subchannel_key_copy(grpc_subchannel_key *k) { + return create_key(k->connector, &k->args, grpc_channel_args_copy); +} + +static int subchannel_key_compare(grpc_subchannel_key *a, grpc_subchannel_key *b) { + int c = GPR_ICMP(a->connector, b->connector); + if (c != 0) return c; + c = GPR_ICMP(a->args.addr_len, b->args.addr_len); if (c != 0) return c; - c = memcmp(a->addr, b->addr, a->addr_len); + c = GPR_ICMP(a->args.filter_count, b->args.filter_count); if (c != 0) return c; - return grpc_channel_args_compare(a->normalized_args, b->normalized_args); + c = memcmp(a->args.addr, b->args.addr, a->args.addr_len); + if (c != 0) return c; + c = memcmp(a->args.filters, b->args.filters, a->args.filter_count * sizeof(*a->args.filters)); + return grpc_channel_args_compare(a->args.args, b->args.args); } -static void subchannel_key_destroy(subchannel_key *k) { - gpr_free(k->addr); - grpc_channel_args_destroy(k->normalized_args); +static void subchannel_key_destroy(grpc_subchannel_key *k) { + gpr_free(k->args.addr); + gpr_free(k->args.filters); + grpc_channel_args_destroy((grpc_channel_args*)k->args.args); gpr_free(k); } @@ -85,16 +119,17 @@ static void *sck_avl_copy(void *p) { return subchannel_key_copy(p); } -static void *sck_avl_compare(void *a, void *b) { +static long sck_avl_compare(void *a, void *b) { return subchannel_key_compare(a, b); } static void scv_avl_destroy(void *p) { - GRPC_SUBCHANNEL_UNREF(exec_ctx, p, "subchannel_index"); + grpc_exec_ctx *exec_ctx = current_ctx(); + GRPC_SUBCHANNEL_WEAK_UNREF(exec_ctx, p, "subchannel_index"); } static void *scv_avl_copy(void *p) { - GRPC_SUBCHANNEL_REF(p, "subchannel_index"); + GRPC_SUBCHANNEL_WEAK_REF(p, "subchannel_index"); return p; } @@ -107,26 +142,31 @@ static const gpr_avl_vtable subchannel_avl_vtable = { }; grpc_subchannel *grpc_subchannel_index_find( - grpc_exec_ctx *ctx, + grpc_exec_ctx *exec_ctx, grpc_connector *connector, grpc_subchannel_args *args) { + enter_ctx(ctx); + gpr_mu_lock(&g_mu); gpr_avl index = gpr_avl_ref(g_subchannel_index); gpr_mu_unlock(&g_mu); subchannel_key *key = subchannel_key_create(connector, args); - grpc_subchannel *c = grpc_subchannel_ref(gpr_avl_get(index, key)); + grpc_subchannel *c = GRPC_SUBCHANNEL_REF_FROM_WEAK_REF(gpr_avl_get(index, key)); subchannel_key_destroy(key); gpr_avl_unref(index); + leave_ctx(ctx); return c; } grpc_subchannel *grpc_subchannel_index_register( - grpc_exec_ctx *ctx, + grpc_exec_ctx *exec_ctx, grpc_connector *connector, grpc_subchannel_args *args, grpc_subchannel *constructed) { + enter_ctx(ctx); + subchannel_key *key = subchannel_key_create(connector, args); grpc_subchannel *c = NULL; @@ -137,7 +177,7 @@ grpc_subchannel *grpc_subchannel_index_register( c = gpr_avl_get(index, key); if (c != NULL) { - GRPC_SUBCHANNEL_UNREF(constructed); + GRPC_SUBCHANNEL_WEAK_UNREF(constructed); } else { gpr_avl updated = gpr_avl_add(index, key, constructed); @@ -151,5 +191,7 @@ grpc_subchannel *grpc_subchannel_index_register( gpr_avl_unref(index); } + leave_ctx(ctx); + return c; } diff --git a/src/core/client_config/subchannel_index.h b/src/core/client_config/subchannel_index.h index d501e121f1d..dfbc3228d9c 100644 --- a/src/core/client_config/subchannel_index.h +++ b/src/core/client_config/subchannel_index.h @@ -34,6 +34,28 @@ #ifndef GRPC_INTERNAL_CORE_CLIENT_CONFIG_SUBCHANNEL_INDEX_H #define GRPC_INTERNAL_CORE_CLIENT_CONFIG_SUBCHANNEL_INDEX_H +#include "src/core/client_config/connector.h" +#include "src/core/client_config/subchannel.h" +typedef struct grpc_subchannel_key grpc_subchannel_key; + +grpc_subchannel_key *grpc_subchannel_key_create( + grpc_connector *con, grpc_subchannel_args *args); + +void grpc_subchannel_key_destroy(grpc_subchannel_key *key); + +grpc_subchannel *grpc_subchannel_index_find( + grpc_exec_ctx *ctx, + grpc_subchannel_key *key); + +grpc_subchannel *grpc_subchannel_index_register( + grpc_exec_ctx *ctx, + grpc_subchannel_key *key, + grpc_subchannel *constructed); + +void grpc_subchannel_index_unregister( + grpc_exec_ctx *ctx, + grpc_subchannel_key *key, + grpc_subchannel *constructed); #endif /* GRPC_INTERNAL_CORE_CLIENT_CONFIG_SUBCHANNEL_INDEX_H */ diff --git a/src/core/surface/secure_channel_create.c b/src/core/surface/secure_channel_create.c index 552a570713d..38f3e28e3da 100644 --- a/src/core/surface/secure_channel_create.c +++ b/src/core/surface/secure_channel_create.c @@ -238,7 +238,7 @@ static grpc_subchannel *subchannel_factory_create_subchannel( gpr_mu_init(&c->mu); gpr_ref_init(&c->refs, 1); args->args = final_args; - s = grpc_subchannel_create(&c->base, args); + s = grpc_subchannel_create(exec_ctx, &c->base, args); grpc_connector_unref(exec_ctx, &c->base); grpc_channel_args_destroy(final_args); return s; From 8cdba6644a83333e53b6188ca95b5a6c88968257 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 22 Jan 2016 20:01:55 -0800 Subject: [PATCH 04/37] Subchannel index compiles --- src/core/client_config/subchannel.c | 15 +++++ src/core/client_config/subchannel.h | 2 + src/core/client_config/subchannel_index.c | 72 ++++++++++++++++++----- src/core/client_config/subchannel_index.h | 9 ++- src/core/surface/channel_create.c | 2 +- src/core/surface/init.c | 5 +- test/core/end2end/fixtures/h2_uchannel.c | 2 +- 7 files changed, 84 insertions(+), 23 deletions(-) diff --git a/src/core/client_config/subchannel.c b/src/core/client_config/subchannel.c index bf2f15a4443..c704595ec79 100644 --- a/src/core/client_config/subchannel.c +++ b/src/core/client_config/subchannel.c @@ -239,6 +239,21 @@ void grpc_subchannel_weak_ref(grpc_subchannel *c GPR_ASSERT(old_refs != 0); } +grpc_subchannel *grpc_subchannel_ref_from_weak_ref(grpc_subchannel *c GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { + if (!c) return NULL; + for (;;) { + gpr_atm old_refs = gpr_atm_acq_load(&c->ref_pair); + if (old_refs >= (1 << INTERNAL_REF_BITS)) { + gpr_atm new_refs = old_refs + (1 << INTERNAL_REF_BITS); + if (gpr_atm_rel_cas(&c->ref_pair, old_refs, new_refs)) { + return c; + } + } else { + return NULL; + } + } +} + static void disconnect(grpc_exec_ctx *exec_ctx, grpc_subchannel *c) { grpc_connected_subchannel *con; grpc_subchannel_index_unregister(exec_ctx, c->key, c); diff --git a/src/core/client_config/subchannel.h b/src/core/client_config/subchannel.h index 8fd976276b5..0d470f593ca 100644 --- a/src/core/client_config/subchannel.h +++ b/src/core/client_config/subchannel.h @@ -84,6 +84,8 @@ typedef struct grpc_subchannel_args grpc_subchannel_args; void grpc_subchannel_ref(grpc_subchannel *channel GRPC_SUBCHANNEL_REF_EXTRA_ARGS); +grpc_subchannel *grpc_subchannel_ref_from_weak_ref(grpc_subchannel *channel + GRPC_SUBCHANNEL_REF_EXTRA_ARGS); void grpc_subchannel_unref(grpc_exec_ctx *exec_ctx, grpc_subchannel *channel GRPC_SUBCHANNEL_REF_EXTRA_ARGS); diff --git a/src/core/client_config/subchannel_index.c b/src/core/client_config/subchannel_index.c index 9f6ecca2959..575ccb4aadf 100644 --- a/src/core/client_config/subchannel_index.c +++ b/src/core/client_config/subchannel_index.c @@ -33,6 +33,7 @@ #include "src/core/client_config/subchannel_index.h" +#include #include #include @@ -104,7 +105,7 @@ static int subchannel_key_compare(grpc_subchannel_key *a, grpc_subchannel_key *b return grpc_channel_args_compare(a->args.args, b->args.args); } -static void subchannel_key_destroy(grpc_subchannel_key *k) { +void grpc_subchannel_key_destroy(grpc_subchannel_key *k) { gpr_free(k->args.addr); gpr_free(k->args.filters); grpc_channel_args_destroy((grpc_channel_args*)k->args.args); @@ -112,7 +113,7 @@ static void subchannel_key_destroy(grpc_subchannel_key *k) { } static void sck_avl_destroy(void *p) { - subchannel_key_destroy(p); + grpc_subchannel_key_destroy(p); } static void *sck_avl_copy(void *p) { @@ -141,33 +142,38 @@ static const gpr_avl_vtable subchannel_avl_vtable = { .copy_value = scv_avl_copy }; +void grpc_subchannel_index_init(void) { + g_subchannel_index = gpr_avl_create(&subchannel_avl_vtable); + gpr_mu_init(&g_mu); +} + +void grpc_subchannel_index_shutdown(void) { + gpr_mu_destroy(&g_mu); + gpr_avl_unref(g_subchannel_index); +} + grpc_subchannel *grpc_subchannel_index_find( grpc_exec_ctx *exec_ctx, - grpc_connector *connector, - grpc_subchannel_args *args) { - enter_ctx(ctx); + grpc_subchannel_key *key) { + enter_ctx(exec_ctx); gpr_mu_lock(&g_mu); gpr_avl index = gpr_avl_ref(g_subchannel_index); gpr_mu_unlock(&g_mu); - subchannel_key *key = subchannel_key_create(connector, args); - grpc_subchannel *c = GRPC_SUBCHANNEL_REF_FROM_WEAK_REF(gpr_avl_get(index, key)); - subchannel_key_destroy(key); + grpc_subchannel *c = GRPC_SUBCHANNEL_REF_FROM_WEAK_REF(gpr_avl_get(index, key), "index_find"); gpr_avl_unref(index); - leave_ctx(ctx); + leave_ctx(exec_ctx); return c; } grpc_subchannel *grpc_subchannel_index_register( grpc_exec_ctx *exec_ctx, - grpc_connector *connector, - grpc_subchannel_args *args, + grpc_subchannel_key *key, grpc_subchannel *constructed) { - enter_ctx(ctx); + enter_ctx(exec_ctx); - subchannel_key *key = subchannel_key_create(connector, args); grpc_subchannel *c = NULL; while (c == NULL) { @@ -177,13 +183,13 @@ grpc_subchannel *grpc_subchannel_index_register( c = gpr_avl_get(index, key); if (c != NULL) { - GRPC_SUBCHANNEL_WEAK_UNREF(constructed); + GRPC_SUBCHANNEL_WEAK_UNREF(exec_ctx, constructed, "index_register"); } else { gpr_avl updated = gpr_avl_add(index, key, constructed); gpr_mu_lock(&g_mu); if (index.root == g_subchannel_index.root) { - GPR_SWAP(index, g_subchannel_index); + GPR_SWAP(gpr_avl, updated, g_subchannel_index); c = constructed; } gpr_mu_unlock(&g_mu); @@ -191,7 +197,41 @@ grpc_subchannel *grpc_subchannel_index_register( gpr_avl_unref(index); } - leave_ctx(ctx); + leave_ctx(exec_ctx); return c; } + +void grpc_subchannel_index_unregister( + grpc_exec_ctx *exec_ctx, + grpc_subchannel_key *key, + grpc_subchannel *constructed) { + enter_ctx(exec_ctx); + + bool done = false; + while (!done) { + gpr_mu_lock(&g_mu); + gpr_avl index = gpr_avl_ref(g_subchannel_index); + gpr_mu_unlock(&g_mu); + + grpc_subchannel *c = gpr_avl_get(index, key); + if (c != constructed) { + break; + } + + gpr_avl updated = gpr_avl_remove(index, key); + + gpr_mu_lock(&g_mu); + if (index.root == g_subchannel_index.root) { + GPR_SWAP(gpr_avl, updated, g_subchannel_index); + done = true; + } else { + GPR_SWAP(gpr_avl, updated, index); + } + gpr_mu_unlock(&g_mu); + + gpr_avl_unref(index); + } + + leave_ctx(exec_ctx); +} diff --git a/src/core/client_config/subchannel_index.h b/src/core/client_config/subchannel_index.h index dfbc3228d9c..69c19969a9b 100644 --- a/src/core/client_config/subchannel_index.h +++ b/src/core/client_config/subchannel_index.h @@ -45,17 +45,20 @@ grpc_subchannel_key *grpc_subchannel_key_create( void grpc_subchannel_key_destroy(grpc_subchannel_key *key); grpc_subchannel *grpc_subchannel_index_find( - grpc_exec_ctx *ctx, + grpc_exec_ctx *exec_ctx, grpc_subchannel_key *key); grpc_subchannel *grpc_subchannel_index_register( - grpc_exec_ctx *ctx, + grpc_exec_ctx *exec_ctx, grpc_subchannel_key *key, grpc_subchannel *constructed); void grpc_subchannel_index_unregister( - grpc_exec_ctx *ctx, + grpc_exec_ctx *exec_ctx, grpc_subchannel_key *key, grpc_subchannel *constructed); +void grpc_subchannel_index_init(void); +void grpc_subchannel_index_shutdown(void); + #endif /* GRPC_INTERNAL_CORE_CLIENT_CONFIG_SUBCHANNEL_INDEX_H */ diff --git a/src/core/surface/channel_create.c b/src/core/surface/channel_create.c index 49083f08702..031ae1b5439 100644 --- a/src/core/surface/channel_create.c +++ b/src/core/surface/channel_create.c @@ -172,7 +172,7 @@ static grpc_subchannel *subchannel_factory_create_subchannel( c->base.vtable = &connector_vtable; gpr_ref_init(&c->refs, 1); args->args = final_args; - s = grpc_subchannel_create(&c->base, args); + s = grpc_subchannel_create(exec_ctx, &c->base, args); grpc_connector_unref(exec_ctx, &c->base); grpc_channel_args_destroy(final_args); return s; diff --git a/src/core/surface/init.c b/src/core/surface/init.c index 8f1936227ed..66c5a522a40 100644 --- a/src/core/surface/init.c +++ b/src/core/surface/init.c @@ -47,6 +47,7 @@ #include "src/core/client_config/resolvers/dns_resolver.h" #include "src/core/client_config/resolvers/sockaddr_resolver.h" #include "src/core/client_config/subchannel.h" +#include "src/core/client_config/subchannel_index.h" #include "src/core/debug/trace.h" #include "src/core/iomgr/executor.h" #include "src/core/iomgr/iomgr.h" @@ -126,7 +127,7 @@ void grpc_init(void) { } gpr_timers_global_init(); grpc_cq_global_init(); - grpc_subchannel_global_init(); + grpc_subchannel_index_init(); for (i = 0; i < g_number_of_plugins; i++) { if (g_all_of_the_plugins[i].init != NULL) { g_all_of_the_plugins[i].init(); @@ -145,7 +146,7 @@ void grpc_shutdown(void) { grpc_executor_shutdown(); grpc_cq_global_shutdown(); grpc_iomgr_shutdown(); - grpc_subchannel_global_shutdown(); + grpc_subchannel_index_shutdown(); census_shutdown(); gpr_timers_global_destroy(); grpc_tracer_shutdown(); diff --git a/test/core/end2end/fixtures/h2_uchannel.c b/test/core/end2end/fixtures/h2_uchannel.c index 9b622e80d6e..ee6aac796d1 100644 --- a/test/core/end2end/fixtures/h2_uchannel.c +++ b/test/core/end2end/fixtures/h2_uchannel.c @@ -159,7 +159,7 @@ static grpc_subchannel *subchannel_factory_create_subchannel( c->base.vtable = &connector_vtable; gpr_ref_init(&c->refs, 1); args->args = final_args; - s = grpc_subchannel_create(&c->base, args); + s = grpc_subchannel_create(exec_ctx, &c->base, args); grpc_connector_unref(exec_ctx, &c->base); grpc_channel_args_destroy(final_args); if (*f->sniffed_subchannel) { From 5de79ee59f526f1eb0125d6544aaccdf5527afb4 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 25 Jan 2016 08:16:02 -0800 Subject: [PATCH 05/37] clang-format --- src/core/channel/channel_args.c | 15 +- src/core/channel/channel_args.h | 3 +- src/core/client_config/connector.c | 2 +- src/core/client_config/subchannel.c | 3 +- src/core/client_config/subchannel.h | 9 +- src/core/client_config/subchannel_index.c | 197 +++++++++++----------- src/core/client_config/subchannel_index.h | 23 ++- src/core/security/credentials.c | 6 +- src/core/security/security_connector.c | 10 +- src/core/security/security_context.c | 10 +- src/cpp/common/channel_arguments.cc | 7 +- 11 files changed, 135 insertions(+), 150 deletions(-) diff --git a/src/core/channel/channel_args.c b/src/core/channel/channel_args.c index cd35d2f701c..6ac6b994751 100644 --- a/src/core/channel/channel_args.c +++ b/src/core/channel/channel_args.c @@ -54,7 +54,8 @@ static grpc_arg copy_arg(const grpc_arg *src) { break; case GRPC_ARG_POINTER: dst.value.pointer = src->value.pointer; - dst.value.pointer.p = src->value.pointer.vtable->copy(src->value.pointer.p); + dst.value.pointer.p = + src->value.pointer.vtable->copy(src->value.pointer.p); break; } return dst; @@ -104,11 +105,9 @@ static int cmp_arg(const grpc_arg *a, const grpc_arg *b) { c = GPR_ICMP(a->value.integer, b->value.integer); break; case GRPC_ARG_POINTER: - c = GPR_ICMP(a->value.pointer.p, - b->value.pointer.p); + c = GPR_ICMP(a->value.pointer.p, b->value.pointer.p); if (c != 0) { - c = GPR_ICMP(a->value.pointer.vtable, - b->value.pointer.vtable); + c = GPR_ICMP(a->value.pointer.vtable, b->value.pointer.vtable); if (c == 0) { c = a->value.pointer.vtable->cmp(a->value.pointer.p, b->value.pointer.p); @@ -128,11 +127,11 @@ static int cmp_key_stable(const void *ap, const void *bp) { } grpc_channel_args *grpc_channel_args_normalize(const grpc_channel_args *a) { - grpc_arg **args = gpr_malloc(sizeof(grpc_arg*) * a->num_args); + grpc_arg **args = gpr_malloc(sizeof(grpc_arg *) * a->num_args); for (size_t i = 0; i < a->num_args; i++) { args[i] = &a->args[i]; } - qsort(args, a->num_args, sizeof(grpc_arg*), cmp_key_stable); + qsort(args, a->num_args, sizeof(grpc_arg *), cmp_key_stable); grpc_channel_args *b = gpr_malloc(sizeof(grpc_channel_args)); b->num_args = a->num_args; @@ -258,7 +257,7 @@ int grpc_channel_args_compression_algorithm_get_states( } } -int grpc_channel_args_compare(const grpc_channel_args *a, +int grpc_channel_args_compare(const grpc_channel_args *a, const grpc_channel_args *b) { int c = GPR_ICMP(a->num_args, b->num_args); if (c != 0) return c; diff --git a/src/core/channel/channel_args.h b/src/core/channel/channel_args.h index ce848782e3f..c8eaacb87b3 100644 --- a/src/core/channel/channel_args.h +++ b/src/core/channel/channel_args.h @@ -88,6 +88,7 @@ grpc_channel_args *grpc_channel_args_compression_algorithm_set_state( int grpc_channel_args_compression_algorithm_get_states( const grpc_channel_args *a); -int grpc_channel_args_compare(const grpc_channel_args *a, const grpc_channel_args *b); +int grpc_channel_args_compare(const grpc_channel_args *a, + const grpc_channel_args *b); #endif /* GRPC_INTERNAL_CORE_CHANNEL_CHANNEL_ARGS_H */ diff --git a/src/core/client_config/connector.c b/src/core/client_config/connector.c index eaa215fe8f6..9aac02ae1cc 100644 --- a/src/core/client_config/connector.c +++ b/src/core/client_config/connector.c @@ -33,7 +33,7 @@ #include "src/core/client_config/connector.h" -grpc_connector *grpc_connector_ref(grpc_connector* connector) { +grpc_connector* grpc_connector_ref(grpc_connector* connector) { connector->vtable->ref(connector); return connector; } diff --git a/src/core/client_config/subchannel.c b/src/core/client_config/subchannel.c index c704595ec79..32c718553df 100644 --- a/src/core/client_config/subchannel.c +++ b/src/core/client_config/subchannel.c @@ -239,7 +239,8 @@ void grpc_subchannel_weak_ref(grpc_subchannel *c GPR_ASSERT(old_refs != 0); } -grpc_subchannel *grpc_subchannel_ref_from_weak_ref(grpc_subchannel *c GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { +grpc_subchannel *grpc_subchannel_ref_from_weak_ref( + grpc_subchannel *c GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { if (!c) return NULL; for (;;) { gpr_atm old_refs = gpr_atm_acq_load(&c->ref_pair); diff --git a/src/core/client_config/subchannel.h b/src/core/client_config/subchannel.h index 0d470f593ca..c48d7a3a8b4 100644 --- a/src/core/client_config/subchannel.h +++ b/src/core/client_config/subchannel.h @@ -68,7 +68,8 @@ typedef struct grpc_subchannel_args grpc_subchannel_args; , const char *file, int line, const char *reason #else #define GRPC_SUBCHANNEL_REF(p, r) grpc_subchannel_ref((p)) -#define GRPC_SUBCHANNEL_REF_FROM_WEAK_REF(p, r) grpc_subchannel_ref_from_weak_ref((p)) +#define GRPC_SUBCHANNEL_REF_FROM_WEAK_REF(p, r) \ + grpc_subchannel_ref_from_weak_ref((p)) #define GRPC_SUBCHANNEL_UNREF(cl, p, r) grpc_subchannel_unref((cl), (p)) #define GRPC_SUBCHANNEL_WEAK_REF(p, r) grpc_subchannel_weak_ref((p)) #define GRPC_SUBCHANNEL_WEAK_UNREF(cl, p, r) \ @@ -84,8 +85,8 @@ typedef struct grpc_subchannel_args grpc_subchannel_args; void grpc_subchannel_ref(grpc_subchannel *channel GRPC_SUBCHANNEL_REF_EXTRA_ARGS); -grpc_subchannel *grpc_subchannel_ref_from_weak_ref(grpc_subchannel *channel - GRPC_SUBCHANNEL_REF_EXTRA_ARGS); +grpc_subchannel *grpc_subchannel_ref_from_weak_ref( + grpc_subchannel *channel GRPC_SUBCHANNEL_REF_EXTRA_ARGS); void grpc_subchannel_unref(grpc_exec_ctx *exec_ctx, grpc_subchannel *channel GRPC_SUBCHANNEL_REF_EXTRA_ARGS); @@ -152,7 +153,7 @@ grpc_call_stack *grpc_subchannel_call_get_call_stack( struct grpc_subchannel_args { /* When updating this struct, also update subchannel_index.c */ - + /** Channel filters for this channel - wrapped factories will likely want to mutate this */ const grpc_channel_filter **filters; diff --git a/src/core/client_config/subchannel_index.c b/src/core/client_config/subchannel_index.c index 575ccb4aadf..418ebd135f4 100644 --- a/src/core/client_config/subchannel_index.c +++ b/src/core/client_config/subchannel_index.c @@ -49,34 +49,37 @@ static gpr_avl g_subchannel_index; static gpr_mu g_mu; struct grpc_subchannel_key { - grpc_connector *connector; - grpc_subchannel_args args; + grpc_connector *connector; + grpc_subchannel_args args; }; GPR_TLS_DECL(subchannel_index_exec_ctx); static void enter_ctx(grpc_exec_ctx *exec_ctx) { - GPR_ASSERT(gpr_tls_get(&subchannel_index_exec_ctx) == 0); - gpr_tls_set(&subchannel_index_exec_ctx, (intptr_t)exec_ctx); + GPR_ASSERT(gpr_tls_get(&subchannel_index_exec_ctx) == 0); + gpr_tls_set(&subchannel_index_exec_ctx, (intptr_t)exec_ctx); } static void leave_ctx(grpc_exec_ctx *exec_ctx) { - GPR_ASSERT(gpr_tls_get(&subchannel_index_exec_ctx) == (intptr_t)exec_ctx); - gpr_tls_set(&subchannel_index_exec_ctx, 0); + GPR_ASSERT(gpr_tls_get(&subchannel_index_exec_ctx) == (intptr_t)exec_ctx); + gpr_tls_set(&subchannel_index_exec_ctx, 0); } static grpc_exec_ctx *current_ctx() { - grpc_exec_ctx *c = (grpc_exec_ctx *)gpr_tls_get(&subchannel_index_exec_ctx); - GPR_ASSERT(c != NULL); - return c; + grpc_exec_ctx *c = (grpc_exec_ctx *)gpr_tls_get(&subchannel_index_exec_ctx); + GPR_ASSERT(c != NULL); + return c; } -static grpc_subchannel_key *create_key(grpc_connector *connector, grpc_subchannel_args *args, grpc_channel_args *(*copy_channel_args)(const grpc_channel_args *args)) { +static grpc_subchannel_key *create_key( + grpc_connector *connector, grpc_subchannel_args *args, + grpc_channel_args *(*copy_channel_args)(const grpc_channel_args *args)) { grpc_subchannel_key *k = gpr_malloc(sizeof(*k)); k->connector = grpc_connector_ref(connector); k->args.filter_count = args->filter_count; k->args.filters = gpr_malloc(sizeof(*k->args.filters) * k->args.filter_count); - memcpy(k->args.filters, args->filters, sizeof(*k->args.filters) * k->args.filter_count); + memcpy(k->args.filters, args->filters, + sizeof(*k->args.filters) * k->args.filter_count); k->args.addr_len = args->addr_len; k->args.addr = gpr_malloc(args->addr_len); memcpy(k->args.addr, args->addr, k->args.addr_len); @@ -84,15 +87,17 @@ static grpc_subchannel_key *create_key(grpc_connector *connector, grpc_subchanne return k; } -grpc_subchannel_key *grpc_subchannel_key_create(grpc_connector *connector, grpc_subchannel_args *args) { - return create_key(connector, args, grpc_channel_args_normalize); +grpc_subchannel_key *grpc_subchannel_key_create(grpc_connector *connector, + grpc_subchannel_args *args) { + return create_key(connector, args, grpc_channel_args_normalize); } static grpc_subchannel_key *subchannel_key_copy(grpc_subchannel_key *k) { - return create_key(k->connector, &k->args, grpc_channel_args_copy); + return create_key(k->connector, &k->args, grpc_channel_args_copy); } -static int subchannel_key_compare(grpc_subchannel_key *a, grpc_subchannel_key *b) { +static int subchannel_key_compare(grpc_subchannel_key *a, + grpc_subchannel_key *b) { int c = GPR_ICMP(a->connector, b->connector); if (c != 0) return c; c = GPR_ICMP(a->args.addr_len, b->args.addr_len); @@ -101,137 +106,131 @@ static int subchannel_key_compare(grpc_subchannel_key *a, grpc_subchannel_key *b if (c != 0) return c; c = memcmp(a->args.addr, b->args.addr, a->args.addr_len); if (c != 0) return c; - c = memcmp(a->args.filters, b->args.filters, a->args.filter_count * sizeof(*a->args.filters)); + c = memcmp(a->args.filters, b->args.filters, + a->args.filter_count * sizeof(*a->args.filters)); return grpc_channel_args_compare(a->args.args, b->args.args); } void grpc_subchannel_key_destroy(grpc_subchannel_key *k) { gpr_free(k->args.addr); gpr_free(k->args.filters); - grpc_channel_args_destroy((grpc_channel_args*)k->args.args); + grpc_channel_args_destroy((grpc_channel_args *)k->args.args); gpr_free(k); } -static void sck_avl_destroy(void *p) { - grpc_subchannel_key_destroy(p); -} +static void sck_avl_destroy(void *p) { grpc_subchannel_key_destroy(p); } -static void *sck_avl_copy(void *p) { - return subchannel_key_copy(p); -} +static void *sck_avl_copy(void *p) { return subchannel_key_copy(p); } static long sck_avl_compare(void *a, void *b) { return subchannel_key_compare(a, b); } static void scv_avl_destroy(void *p) { - grpc_exec_ctx *exec_ctx = current_ctx(); + grpc_exec_ctx *exec_ctx = current_ctx(); GRPC_SUBCHANNEL_WEAK_UNREF(exec_ctx, p, "subchannel_index"); } -static void *scv_avl_copy(void *p) { +static void *scv_avl_copy(void *p) { GRPC_SUBCHANNEL_WEAK_REF(p, "subchannel_index"); - return p; + return p; } static const gpr_avl_vtable subchannel_avl_vtable = { - .destroy_key = sck_avl_destroy, - .copy_key = sck_avl_copy, - .compare_keys = sck_avl_compare, - .destroy_value = scv_avl_destroy, - .copy_value = scv_avl_copy -}; + .destroy_key = sck_avl_destroy, + .copy_key = sck_avl_copy, + .compare_keys = sck_avl_compare, + .destroy_value = scv_avl_destroy, + .copy_value = scv_avl_copy}; void grpc_subchannel_index_init(void) { - g_subchannel_index = gpr_avl_create(&subchannel_avl_vtable); - gpr_mu_init(&g_mu); + g_subchannel_index = gpr_avl_create(&subchannel_avl_vtable); + gpr_mu_init(&g_mu); } void grpc_subchannel_index_shutdown(void) { - gpr_mu_destroy(&g_mu); - gpr_avl_unref(g_subchannel_index); + gpr_mu_destroy(&g_mu); + gpr_avl_unref(g_subchannel_index); } -grpc_subchannel *grpc_subchannel_index_find( - grpc_exec_ctx *exec_ctx, - grpc_subchannel_key *key) { - enter_ctx(exec_ctx); +grpc_subchannel *grpc_subchannel_index_find(grpc_exec_ctx *exec_ctx, + grpc_subchannel_key *key) { + enter_ctx(exec_ctx); - gpr_mu_lock(&g_mu); - gpr_avl index = gpr_avl_ref(g_subchannel_index); - gpr_mu_unlock(&g_mu); + gpr_mu_lock(&g_mu); + gpr_avl index = gpr_avl_ref(g_subchannel_index); + gpr_mu_unlock(&g_mu); - grpc_subchannel *c = GRPC_SUBCHANNEL_REF_FROM_WEAK_REF(gpr_avl_get(index, key), "index_find"); - gpr_avl_unref(index); + grpc_subchannel *c = + GRPC_SUBCHANNEL_REF_FROM_WEAK_REF(gpr_avl_get(index, key), "index_find"); + gpr_avl_unref(index); - leave_ctx(exec_ctx); - return c; + leave_ctx(exec_ctx); + return c; } -grpc_subchannel *grpc_subchannel_index_register( - grpc_exec_ctx *exec_ctx, - grpc_subchannel_key *key, - grpc_subchannel *constructed) { - enter_ctx(exec_ctx); +grpc_subchannel *grpc_subchannel_index_register(grpc_exec_ctx *exec_ctx, + grpc_subchannel_key *key, + grpc_subchannel *constructed) { + enter_ctx(exec_ctx); - grpc_subchannel *c = NULL; + grpc_subchannel *c = NULL; - while (c == NULL) { - gpr_mu_lock(&g_mu); - gpr_avl index = gpr_avl_ref(g_subchannel_index); - gpr_mu_unlock(&g_mu); + while (c == NULL) { + gpr_mu_lock(&g_mu); + gpr_avl index = gpr_avl_ref(g_subchannel_index); + gpr_mu_unlock(&g_mu); - c = gpr_avl_get(index, key); - if (c != NULL) { - GRPC_SUBCHANNEL_WEAK_UNREF(exec_ctx, constructed, "index_register"); - } else { - gpr_avl updated = gpr_avl_add(index, key, constructed); + c = gpr_avl_get(index, key); + if (c != NULL) { + GRPC_SUBCHANNEL_WEAK_UNREF(exec_ctx, constructed, "index_register"); + } else { + gpr_avl updated = gpr_avl_add(index, key, constructed); - gpr_mu_lock(&g_mu); - if (index.root == g_subchannel_index.root) { - GPR_SWAP(gpr_avl, updated, g_subchannel_index); - c = constructed; - } - gpr_mu_unlock(&g_mu); - } - gpr_avl_unref(index); - } + gpr_mu_lock(&g_mu); + if (index.root == g_subchannel_index.root) { + GPR_SWAP(gpr_avl, updated, g_subchannel_index); + c = constructed; + } + gpr_mu_unlock(&g_mu); + } + gpr_avl_unref(index); + } - leave_ctx(exec_ctx); + leave_ctx(exec_ctx); - return c; + return c; } -void grpc_subchannel_index_unregister( - grpc_exec_ctx *exec_ctx, - grpc_subchannel_key *key, - grpc_subchannel *constructed) { - enter_ctx(exec_ctx); +void grpc_subchannel_index_unregister(grpc_exec_ctx *exec_ctx, + grpc_subchannel_key *key, + grpc_subchannel *constructed) { + enter_ctx(exec_ctx); - bool done = false; - while (!done) { - gpr_mu_lock(&g_mu); - gpr_avl index = gpr_avl_ref(g_subchannel_index); - gpr_mu_unlock(&g_mu); + bool done = false; + while (!done) { + gpr_mu_lock(&g_mu); + gpr_avl index = gpr_avl_ref(g_subchannel_index); + gpr_mu_unlock(&g_mu); - grpc_subchannel *c = gpr_avl_get(index, key); - if (c != constructed) { - break; - } + grpc_subchannel *c = gpr_avl_get(index, key); + if (c != constructed) { + break; + } - gpr_avl updated = gpr_avl_remove(index, key); + gpr_avl updated = gpr_avl_remove(index, key); - gpr_mu_lock(&g_mu); - if (index.root == g_subchannel_index.root) { - GPR_SWAP(gpr_avl, updated, g_subchannel_index); - done = true; - } else { - GPR_SWAP(gpr_avl, updated, index); - } - gpr_mu_unlock(&g_mu); + gpr_mu_lock(&g_mu); + if (index.root == g_subchannel_index.root) { + GPR_SWAP(gpr_avl, updated, g_subchannel_index); + done = true; + } else { + GPR_SWAP(gpr_avl, updated, index); + } + gpr_mu_unlock(&g_mu); - gpr_avl_unref(index); - } + gpr_avl_unref(index); + } - leave_ctx(exec_ctx); + leave_ctx(exec_ctx); } diff --git a/src/core/client_config/subchannel_index.h b/src/core/client_config/subchannel_index.h index 69c19969a9b..6c870811707 100644 --- a/src/core/client_config/subchannel_index.h +++ b/src/core/client_config/subchannel_index.h @@ -39,24 +39,21 @@ typedef struct grpc_subchannel_key grpc_subchannel_key; -grpc_subchannel_key *grpc_subchannel_key_create( - grpc_connector *con, grpc_subchannel_args *args); +grpc_subchannel_key *grpc_subchannel_key_create(grpc_connector *con, + grpc_subchannel_args *args); void grpc_subchannel_key_destroy(grpc_subchannel_key *key); -grpc_subchannel *grpc_subchannel_index_find( - grpc_exec_ctx *exec_ctx, - grpc_subchannel_key *key); +grpc_subchannel *grpc_subchannel_index_find(grpc_exec_ctx *exec_ctx, + grpc_subchannel_key *key); -grpc_subchannel *grpc_subchannel_index_register( - grpc_exec_ctx *exec_ctx, - grpc_subchannel_key *key, - grpc_subchannel *constructed); +grpc_subchannel *grpc_subchannel_index_register(grpc_exec_ctx *exec_ctx, + grpc_subchannel_key *key, + grpc_subchannel *constructed); -void grpc_subchannel_index_unregister( - grpc_exec_ctx *exec_ctx, - grpc_subchannel_key *key, - grpc_subchannel *constructed); +void grpc_subchannel_index_unregister(grpc_exec_ctx *exec_ctx, + grpc_subchannel_key *key, + grpc_subchannel *constructed); void grpc_subchannel_index_init(void); void grpc_subchannel_index_shutdown(void); diff --git a/src/core/security/credentials.c b/src/core/security/credentials.c index 28f41b2041f..86a8e130de0 100644 --- a/src/core/security/credentials.c +++ b/src/core/security/credentials.c @@ -201,10 +201,8 @@ static int server_credentials_pointer_cmp(void *a, void *b) { } static const grpc_arg_pointer_vtable cred_ptr_vtable = { - server_credentials_pointer_arg_copy, - server_credentials_pointer_arg_destroy, - server_credentials_pointer_cmp -}; + server_credentials_pointer_arg_copy, server_credentials_pointer_arg_destroy, + server_credentials_pointer_cmp}; grpc_arg grpc_server_credentials_to_arg(grpc_server_credentials *p) { grpc_arg arg; diff --git a/src/core/security/security_connector.c b/src/core/security/security_connector.c index 40f486128b7..cf50e9a91d3 100644 --- a/src/core/security/security_connector.c +++ b/src/core/security/security_connector.c @@ -194,15 +194,11 @@ static void *connector_pointer_arg_copy(void *p) { return GRPC_SECURITY_CONNECTOR_REF(p, "connector_pointer_arg"); } -static int connector_pointer_cmp(void *a, void *b) { - return GPR_ICMP(a, b); -} +static int connector_pointer_cmp(void *a, void *b) { return GPR_ICMP(a, b); } static const grpc_arg_pointer_vtable connector_pointer_vtable = { - connector_pointer_arg_copy, - connector_pointer_arg_destroy, - connector_pointer_cmp -}; + connector_pointer_arg_copy, connector_pointer_arg_destroy, + connector_pointer_cmp}; grpc_arg grpc_security_connector_to_arg(grpc_security_connector *sc) { grpc_arg result; diff --git a/src/core/security/security_context.c b/src/core/security/security_context.c index 6e948f61bc3..d28664cad39 100644 --- a/src/core/security/security_context.c +++ b/src/core/security/security_context.c @@ -309,15 +309,11 @@ static void *auth_context_pointer_arg_copy(void *p) { return GRPC_AUTH_CONTEXT_REF(p, "auth_context_pointer_arg"); } -static int auth_context_pointer_cmp(void *a, void *b) { - return GPR_ICMP(a, b); -} +static int auth_context_pointer_cmp(void *a, void *b) { return GPR_ICMP(a, b); } static const grpc_arg_pointer_vtable auth_context_pointer_vtable = { - auth_context_pointer_arg_copy, - auth_context_pointer_arg_destroy, - auth_context_pointer_cmp -}; + auth_context_pointer_arg_copy, auth_context_pointer_arg_destroy, + auth_context_pointer_cmp}; grpc_arg grpc_auth_context_to_arg(grpc_auth_context *p) { grpc_arg arg; diff --git a/src/cpp/common/channel_arguments.cc b/src/cpp/common/channel_arguments.cc index 3536e354287..f1583de5a14 100644 --- a/src/cpp/common/channel_arguments.cc +++ b/src/cpp/common/channel_arguments.cc @@ -93,17 +93,14 @@ void ChannelArguments::SetPointer(const grpc::string& key, void* value) { struct VtableMembers { static void* Copy(void* in) { return in; } static void Destroy(void* in) {} - static int Compare(void* a, void *b) { + static int Compare(void* a, void* b) { if (a < b) return -1; if (a > b) return 1; return 0; } }; static const grpc_arg_pointer_vtable vtable = { - &VtableMembers::Copy, - &VtableMembers::Destroy, - &VtableMembers::Compare - }; + &VtableMembers::Copy, &VtableMembers::Destroy, &VtableMembers::Compare}; grpc_arg arg; arg.type = GRPC_ARG_POINTER; From 194824467c8d014123a8a3cc8f4ad65b6f9a9b85 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 25 Jan 2016 09:59:20 -0800 Subject: [PATCH 06/37] Get subchannel index working --- build.yaml | 4 +- grpc.gemspec | 2 + include/grpc/support/avl.h | 3 +- include/grpc/support/useful.h | 2 +- package.json | 2 + src/core/channel/channel_args.c | 4 +- src/core/channel/channel_args.h | 2 +- src/core/client_config/connector.c | 2 +- src/core/client_config/connector.h | 2 +- src/core/client_config/subchannel.c | 14 ++- src/core/client_config/subchannel.h | 10 +- src/core/client_config/subchannel_index.c | 109 +++++++++++------- src/core/client_config/subchannel_index.h | 20 +++- src/core/security/security_context.c | 2 +- src/core/surface/channel_create.c | 2 +- src/core/surface/secure_channel_create.c | 2 +- src/cpp/common/channel_arguments.cc | 2 +- test/core/end2end/fixtures/h2_uchannel.c | 2 +- tools/distrib/check_copyright.py | 6 +- ...suppressions.txt => lsan_suppressions.txt} | 0 tools/run_tests/configs.json | 4 +- 21 files changed, 122 insertions(+), 74 deletions(-) rename tools/{asan_suppressions.txt => lsan_suppressions.txt} (100%) diff --git a/build.yaml b/build.yaml index 525035397ce..bbc36a8b12f 100644 --- a/build.yaml +++ b/build.yaml @@ -2505,8 +2505,8 @@ configs: LDXX: clang++ compile_the_world: true test_environ: - ASAN_OPTIONS: suppressions=tools/asan_suppressions.txt:detect_leaks=1:color=always - LSAN_OPTIONS: suppressions=tools/asan_suppressions.txt:report_objects=1 + ASAN_OPTIONS: detect_leaks=1:color=always + LSAN_OPTIONS: suppressions=tools/lsan_suppressions.txt:report_objects=1 timeout_multiplier: 1.5 asan-noleaks: CC: clang diff --git a/grpc.gemspec b/grpc.gemspec index 47b66ae535d..afc4a890f4e 100755 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -172,6 +172,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/client_config/resolvers/sockaddr_resolver.h ) s.files += %w( src/core/client_config/subchannel.h ) s.files += %w( src/core/client_config/subchannel_factory.h ) + s.files += %w( src/core/client_config/subchannel_index.h ) s.files += %w( src/core/client_config/uri_parser.h ) s.files += %w( src/core/compression/algorithm_metadata.h ) s.files += %w( src/core/compression/message_compress.h ) @@ -310,6 +311,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/client_config/resolvers/sockaddr_resolver.c ) s.files += %w( src/core/client_config/subchannel.c ) s.files += %w( src/core/client_config/subchannel_factory.c ) + s.files += %w( src/core/client_config/subchannel_index.c ) s.files += %w( src/core/client_config/uri_parser.c ) s.files += %w( src/core/compression/algorithm.c ) s.files += %w( src/core/compression/message_compress.c ) diff --git a/include/grpc/support/avl.h b/include/grpc/support/avl.h index d462f52bfe0..54605ceb7cb 100644 --- a/include/grpc/support/avl.h +++ b/include/grpc/support/avl.h @@ -81,7 +81,8 @@ void gpr_avl_unref(gpr_avl avl); if key exists in avl, the new tree's key entry updated (i.e. a duplicate is not created) */ gpr_avl gpr_avl_add(gpr_avl avl, void *key, void *value); -/** return a new tree with key deleted */ +/** return a new tree with key deleted + implicitly unrefs avl to allow easy chaining. */ gpr_avl gpr_avl_remove(gpr_avl avl, void *key); /** lookup key, and return the associated value. does not mutate avl. diff --git a/include/grpc/support/useful.h b/include/grpc/support/useful.h index 003e096cf9a..a0e76da29ef 100644 --- a/include/grpc/support/useful.h +++ b/include/grpc/support/useful.h @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/package.json b/package.json index 0b87f6a59d8..74de532bc2e 100644 --- a/package.json +++ b/package.json @@ -123,6 +123,7 @@ "src/core/client_config/resolvers/sockaddr_resolver.h", "src/core/client_config/subchannel.h", "src/core/client_config/subchannel_factory.h", + "src/core/client_config/subchannel_index.h", "src/core/client_config/uri_parser.h", "src/core/compression/algorithm_metadata.h", "src/core/compression/message_compress.h", @@ -261,6 +262,7 @@ "src/core/client_config/resolvers/sockaddr_resolver.c", "src/core/client_config/subchannel.c", "src/core/client_config/subchannel_factory.c", + "src/core/client_config/subchannel_index.c", "src/core/client_config/uri_parser.c", "src/core/compression/algorithm.c", "src/core/compression/message_compress.c", diff --git a/src/core/channel/channel_args.c b/src/core/channel/channel_args.c index 6ac6b994751..63e440f97be 100644 --- a/src/core/channel/channel_args.c +++ b/src/core/channel/channel_args.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -93,7 +93,7 @@ grpc_channel_args *grpc_channel_args_merge(const grpc_channel_args *a, } static int cmp_arg(const grpc_arg *a, const grpc_arg *b) { - int c = a->type - b->type; + int c = GPR_ICMP(a->type, b->type); if (c != 0) return c; c = strcmp(a->key, b->key); if (c != 0) return c; diff --git a/src/core/channel/channel_args.h b/src/core/channel/channel_args.h index c8eaacb87b3..b3a7c9f4349 100644 --- a/src/core/channel/channel_args.h +++ b/src/core/channel/channel_args.h @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/src/core/client_config/connector.c b/src/core/client_config/connector.c index 9aac02ae1cc..aa34aa7fab6 100644 --- a/src/core/client_config/connector.c +++ b/src/core/client_config/connector.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/src/core/client_config/connector.h b/src/core/client_config/connector.h index b301e1bb197..b91eb512c30 100644 --- a/src/core/client_config/connector.h +++ b/src/core/client_config/connector.h @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/src/core/client_config/subchannel.c b/src/core/client_config/subchannel.c index 32c718553df..145e146862e 100644 --- a/src/core/client_config/subchannel.c +++ b/src/core/client_config/subchannel.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -210,6 +210,7 @@ static void subchannel_destroy(grpc_exec_ctx *exec_ctx, void *arg, grpc_connectivity_state_destroy(exec_ctx, &c->state_tracker); grpc_connector_unref(exec_ctx, c->connector); grpc_pollset_set_destroy(&c->pollset_set); + grpc_subchannel_key_destroy(exec_ctx, c->key); gpr_free(c); } @@ -225,18 +226,21 @@ static gpr_atm ref_mutate(grpc_subchannel *c, gpr_atm delta, return old_val; } -void grpc_subchannel_ref(grpc_subchannel *c GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { +grpc_subchannel *grpc_subchannel_ref(grpc_subchannel *c + GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { gpr_atm old_refs; old_refs = ref_mutate(c, (1 << INTERNAL_REF_BITS), 0 REF_MUTATE_PURPOSE("STRONG_REF")); GPR_ASSERT((old_refs & STRONG_REF_MASK) != 0); + return c; } -void grpc_subchannel_weak_ref(grpc_subchannel *c - GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { +grpc_subchannel *grpc_subchannel_weak_ref(grpc_subchannel *c + GRPC_SUBCHANNEL_REF_EXTRA_ARGS) { gpr_atm old_refs; old_refs = ref_mutate(c, 1, 0 REF_MUTATE_PURPOSE("WEAK_REF")); GPR_ASSERT(old_refs != 0); + return c; } grpc_subchannel *grpc_subchannel_ref_from_weak_ref( @@ -302,7 +306,7 @@ grpc_subchannel *grpc_subchannel_create(grpc_exec_ctx *exec_ctx, grpc_subchannel_key *key = grpc_subchannel_key_create(connector, args); grpc_subchannel *c = grpc_subchannel_index_find(exec_ctx, key); if (c) { - grpc_subchannel_key_destroy(key); + grpc_subchannel_key_destroy(exec_ctx, key); return c; } diff --git a/src/core/client_config/subchannel.h b/src/core/client_config/subchannel.h index c48d7a3a8b4..313e63c75c8 100644 --- a/src/core/client_config/subchannel.h +++ b/src/core/client_config/subchannel.h @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -83,15 +83,15 @@ typedef struct grpc_subchannel_args grpc_subchannel_args; #define GRPC_SUBCHANNEL_REF_EXTRA_ARGS #endif -void grpc_subchannel_ref(grpc_subchannel *channel - GRPC_SUBCHANNEL_REF_EXTRA_ARGS); +grpc_subchannel *grpc_subchannel_ref(grpc_subchannel *channel + GRPC_SUBCHANNEL_REF_EXTRA_ARGS); grpc_subchannel *grpc_subchannel_ref_from_weak_ref( grpc_subchannel *channel GRPC_SUBCHANNEL_REF_EXTRA_ARGS); void grpc_subchannel_unref(grpc_exec_ctx *exec_ctx, grpc_subchannel *channel GRPC_SUBCHANNEL_REF_EXTRA_ARGS); -void grpc_subchannel_weak_ref(grpc_subchannel *channel - GRPC_SUBCHANNEL_REF_EXTRA_ARGS); +grpc_subchannel *grpc_subchannel_weak_ref(grpc_subchannel *channel + GRPC_SUBCHANNEL_REF_EXTRA_ARGS); void grpc_subchannel_weak_unref(grpc_exec_ctx *exec_ctx, grpc_subchannel *channel GRPC_SUBCHANNEL_REF_EXTRA_ARGS); diff --git a/src/core/client_config/subchannel_index.c b/src/core/client_config/subchannel_index.c index 418ebd135f4..c4b97301612 100644 --- a/src/core/client_config/subchannel_index.c +++ b/src/core/client_config/subchannel_index.c @@ -1,35 +1,35 @@ -/* - * - * 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. - * - */ +// +// +// Copyright 2016, 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 "src/core/client_config/subchannel_index.h" @@ -42,8 +42,8 @@ #include "src/core/channel/channel_args.h" -/* a map of subchannel_key --> subchannel, used for detecting connections - to the same destination in order to share them */ +// a map of subchannel_key --> subchannel, used for detecting connections +// to the same destination in order to share them static gpr_avl g_subchannel_index; static gpr_mu g_mu; @@ -111,14 +111,18 @@ static int subchannel_key_compare(grpc_subchannel_key *a, return grpc_channel_args_compare(a->args.args, b->args.args); } -void grpc_subchannel_key_destroy(grpc_subchannel_key *k) { +void grpc_subchannel_key_destroy(grpc_exec_ctx *exec_ctx, + grpc_subchannel_key *k) { + grpc_connector_unref(exec_ctx, k->connector); gpr_free(k->args.addr); gpr_free(k->args.filters); grpc_channel_args_destroy((grpc_channel_args *)k->args.args); gpr_free(k); } -static void sck_avl_destroy(void *p) { grpc_subchannel_key_destroy(p); } +static void sck_avl_destroy(void *p) { + grpc_subchannel_key_destroy(current_ctx(), p); +} static void *sck_avl_copy(void *p) { return subchannel_key_copy(p); } @@ -127,8 +131,7 @@ static long sck_avl_compare(void *a, void *b) { } static void scv_avl_destroy(void *p) { - grpc_exec_ctx *exec_ctx = current_ctx(); - GRPC_SUBCHANNEL_WEAK_UNREF(exec_ctx, p, "subchannel_index"); + GRPC_SUBCHANNEL_WEAK_UNREF(current_ctx(), p, "subchannel_index"); } static void *scv_avl_copy(void *p) { @@ -157,6 +160,8 @@ grpc_subchannel *grpc_subchannel_index_find(grpc_exec_ctx *exec_ctx, grpc_subchannel_key *key) { enter_ctx(exec_ctx); + // Lock, and take a reference to the subchannel index. + // We don't need to do the search under a lock as avl's are immutable. gpr_mu_lock(&g_mu); gpr_avl index = gpr_avl_ref(g_subchannel_index); gpr_mu_unlock(&g_mu); @@ -177,22 +182,34 @@ grpc_subchannel *grpc_subchannel_index_register(grpc_exec_ctx *exec_ctx, grpc_subchannel *c = NULL; while (c == NULL) { + // Compare and swap loop: + // - take a reference to the current index gpr_mu_lock(&g_mu); gpr_avl index = gpr_avl_ref(g_subchannel_index); gpr_mu_unlock(&g_mu); + // - Check to see if a subchannel already exists c = gpr_avl_get(index, key); if (c != NULL) { + // yes -> we're done GRPC_SUBCHANNEL_WEAK_UNREF(exec_ctx, constructed, "index_register"); } else { - gpr_avl updated = gpr_avl_add(index, key, constructed); - + // no -> update the avl and compare/swap + gpr_avl updated = + gpr_avl_add(gpr_avl_ref(index), subchannel_key_copy(key), + GRPC_SUBCHANNEL_WEAK_REF(constructed, "index_register")); + + // it may happen (but it's expected to be unlikely) + // that some other thread has changed the index: + // compare/swap here to check that, and retry as necessary gpr_mu_lock(&g_mu); if (index.root == g_subchannel_index.root) { GPR_SWAP(gpr_avl, updated, g_subchannel_index); c = constructed; } gpr_mu_unlock(&g_mu); + + gpr_avl_unref(updated); } gpr_avl_unref(index); } @@ -209,26 +226,32 @@ void grpc_subchannel_index_unregister(grpc_exec_ctx *exec_ctx, bool done = false; while (!done) { + // Compare and swap loop: + // - take a reference to the current index gpr_mu_lock(&g_mu); gpr_avl index = gpr_avl_ref(g_subchannel_index); gpr_mu_unlock(&g_mu); + // Check to see if this key still refers to the previously + // registered subchannel grpc_subchannel *c = gpr_avl_get(index, key); if (c != constructed) { + gpr_avl_unref(index); break; } - gpr_avl updated = gpr_avl_remove(index, key); + // compare and swap the update (some other thread may have + // mutated the index behind us) + gpr_avl updated = gpr_avl_remove(gpr_avl_ref(index), key); gpr_mu_lock(&g_mu); if (index.root == g_subchannel_index.root) { GPR_SWAP(gpr_avl, updated, g_subchannel_index); done = true; - } else { - GPR_SWAP(gpr_avl, updated, index); } gpr_mu_unlock(&g_mu); + gpr_avl_unref(updated); gpr_avl_unref(index); } diff --git a/src/core/client_config/subchannel_index.h b/src/core/client_config/subchannel_index.h index 6c870811707..fc3187a758e 100644 --- a/src/core/client_config/subchannel_index.h +++ b/src/core/client_config/subchannel_index.h @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -37,25 +37,41 @@ #include "src/core/client_config/connector.h" #include "src/core/client_config/subchannel.h" +/** \file Provides an index of active subchannels so that they can be + shared amongst channels */ + typedef struct grpc_subchannel_key grpc_subchannel_key; +/** Create a key that can be used to uniquely identify a subchannel */ grpc_subchannel_key *grpc_subchannel_key_create(grpc_connector *con, grpc_subchannel_args *args); -void grpc_subchannel_key_destroy(grpc_subchannel_key *key); +/** Destroy a subchannel key */ +void grpc_subchannel_key_destroy(grpc_exec_ctx *exec_ctx, + grpc_subchannel_key *key); +/** Given a subchannel key, find the subchannel registered for it. + Returns NULL if no such channel exists. + Thread-safe. */ grpc_subchannel *grpc_subchannel_index_find(grpc_exec_ctx *exec_ctx, grpc_subchannel_key *key); +/** Register a subchannel against a key. + Takes ownership of \a constructed. + Returns the registered subchannel. This may be different from + \a constructed in the case of a registration race. */ grpc_subchannel *grpc_subchannel_index_register(grpc_exec_ctx *exec_ctx, grpc_subchannel_key *key, grpc_subchannel *constructed); +/** Remove \a constructed as the registered subchannel for \a key. */ void grpc_subchannel_index_unregister(grpc_exec_ctx *exec_ctx, grpc_subchannel_key *key, grpc_subchannel *constructed); +/** Initialize the subchannel index (global) */ void grpc_subchannel_index_init(void); +/** Shutdown the subchannel index (global) */ void grpc_subchannel_index_shutdown(void); #endif /* GRPC_INTERNAL_CORE_CLIENT_CONFIG_SUBCHANNEL_INDEX_H */ diff --git a/src/core/security/security_context.c b/src/core/security/security_context.c index d28664cad39..a71b3bc9153 100644 --- a/src/core/security/security_context.c +++ b/src/core/security/security_context.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/src/core/surface/channel_create.c b/src/core/surface/channel_create.c index 031ae1b5439..7d067b58633 100644 --- a/src/core/surface/channel_create.c +++ b/src/core/surface/channel_create.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/src/core/surface/secure_channel_create.c b/src/core/surface/secure_channel_create.c index 38f3e28e3da..6cbd5985ab1 100644 --- a/src/core/surface/secure_channel_create.c +++ b/src/core/surface/secure_channel_create.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/src/cpp/common/channel_arguments.cc b/src/cpp/common/channel_arguments.cc index f1583de5a14..a495118ffce 100644 --- a/src/cpp/common/channel_arguments.cc +++ b/src/cpp/common/channel_arguments.cc @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/test/core/end2end/fixtures/h2_uchannel.c b/test/core/end2end/fixtures/h2_uchannel.c index ee6aac796d1..24079c0b4b4 100644 --- a/test/core/end2end/fixtures/h2_uchannel.c +++ b/test/core/end2end/fixtures/h2_uchannel.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/tools/distrib/check_copyright.py b/tools/distrib/check_copyright.py index 935acf525ea..6123218c553 100755 --- a/tools/distrib/check_copyright.py +++ b/tools/distrib/check_copyright.py @@ -68,9 +68,9 @@ with open('LICENSE') as f: # that given a line of license text, returns what should # be in the file LICENSE_PREFIX = { - '.c': r'\s*\*\s*', - '.cc': r'\s*\*\s*', - '.h': r'\s*\*\s*', + '.c': r'\s*(//|\*)\s*', + '.cc': r'\s*(//|\*)\s*', + '.h': r'\s*(//|\*)\s*', '.m': r'\s*\*\s*', '.php': r'\s*\*\s*', '.js': r'\s*\*\s*', diff --git a/tools/asan_suppressions.txt b/tools/lsan_suppressions.txt similarity index 100% rename from tools/asan_suppressions.txt rename to tools/lsan_suppressions.txt diff --git a/tools/run_tests/configs.json b/tools/run_tests/configs.json index 769942df996..b21793d9cc4 100644 --- a/tools/run_tests/configs.json +++ b/tools/run_tests/configs.json @@ -45,8 +45,8 @@ { "config": "asan", "environ": { - "ASAN_OPTIONS": "suppressions=tools/asan_suppressions.txt:detect_leaks=1:color=always", - "LSAN_OPTIONS": "suppressions=tools/asan_suppressions.txt:report_objects=1" + "ASAN_OPTIONS": "detect_leaks=1:color=always", + "LSAN_OPTIONS": "suppressions=tools/lsan_suppressions.txt:report_objects=1" }, "timeout_multiplier": 1.5 }, From f9ceb1d105d906e393db2520eec759ed6576424e Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 25 Jan 2016 10:55:20 -0800 Subject: [PATCH 07/37] Fix *SAN compilation --- Makefile | 8 +++++++- templates/Makefile.template | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index e1837ef26e0..4f9d9c228a6 100644 --- a/Makefile +++ b/Makefile @@ -261,6 +261,12 @@ endif CXX11_CHECK_CMD = $(CXX) -std=c++11 -o $(TMPOUT) -c test/build/c++11.cc HAS_CXX11 = $(shell $(CXX11_CHECK_CMD) 2> /dev/null && echo true || echo false) +CHECK_NO_SHIFT_NEGATIVE_VALUE_CMD = $(CC) -std=c99 -Wno-shift-negative-value -o $(TMPOUT) -c test/build/empty.c +HAS_NO_SHIFT_NEGATIVE_VALUE = $(shell $(CHECK_NO_SHIFT_NEGATIVE_VALUE_CMD) 2> /dev/null && echo true || echo false) +ifeq ($(HAS_NO_SHIFT_NEGATIVE_VALUE),true) +W_NO_SHIFT_NEGATIVE_VALUE=-Wno-shift-negative-value +endif + # The HOST compiler settings are used to compile the protoc plugins. # In most cases, you won't have to change anything, but if you are # cross-compiling, you can override these variables from GNU make's @@ -5488,7 +5494,7 @@ LIBZ_SRC = \ LIBZ_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(LIBZ_SRC)))) -$(LIBZ_OBJS): CFLAGS := $(CFLAGS) -Wno-sign-conversion -Wno-conversion -Wno-unused-value -Wno-implicit-function-declaration -Wno-shift-negative-value -fvisibility=hidden +$(LIBZ_OBJS): CFLAGS := $(CFLAGS) -Wno-sign-conversion -Wno-conversion -Wno-unused-value -Wno-implicit-function-declaration $(W_NO_SHIFT_NEGATIVE_VALUE) -fvisibility=hidden $(LIBDIR)/$(CONFIG)/libz.a: $(ZLIB_DEP) $(OPENSSL_DEP) $(LIBZ_OBJS) $(E) "[AR] Creating $@" diff --git a/templates/Makefile.template b/templates/Makefile.template index 5db2c4cbd8c..b52354c1a00 100644 --- a/templates/Makefile.template +++ b/templates/Makefile.template @@ -165,6 +165,12 @@ CXX11_CHECK_CMD = $(CXX) -std=c++11 -o $(TMPOUT) -c test/build/c++11.cc HAS_CXX11 = $(shell $(CXX11_CHECK_CMD) 2> /dev/null && echo true || echo false) + CHECK_NO_SHIFT_NEGATIVE_VALUE_CMD = $(CC) -std=c99 -Wno-shift-negative-value -o $(TMPOUT) -c test/build/empty.c + HAS_NO_SHIFT_NEGATIVE_VALUE = $(shell $(CHECK_NO_SHIFT_NEGATIVE_VALUE_CMD) 2> /dev/null && echo true || echo false) + ifeq ($(HAS_NO_SHIFT_NEGATIVE_VALUE),true) + W_NO_SHIFT_NEGATIVE_VALUE=-Wno-shift-negative-value + endif + # The HOST compiler settings are used to compile the protoc plugins. # In most cases, you won't have to change anything, but if you are # cross-compiling, you can override these variables from GNU make's @@ -1438,7 +1444,7 @@ $(LIB${lib.name.upper()}_OBJS): CXXFLAGS := -Ithird_party/boringssl/include $(CXXFLAGS) -fvisibility=hidden $(LIB${lib.name.upper()}_OBJS): CPPFLAGS += -DOPENSSL_NO_ASM -D_GNU_SOURCE % elif lib.zlib: - $(LIB${lib.name.upper()}_OBJS): CFLAGS := $(CFLAGS) -Wno-sign-conversion -Wno-conversion -Wno-unused-value -Wno-implicit-function-declaration -Wno-shift-negative-value -fvisibility=hidden + $(LIB${lib.name.upper()}_OBJS): CFLAGS := $(CFLAGS) -Wno-sign-conversion -Wno-conversion -Wno-unused-value -Wno-implicit-function-declaration $(W_NO_SHIFT_NEGATIVE_VALUE) -fvisibility=hidden % else: % endif From ba7d4dff76908e370d334331d4cd95401561eb11 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 25 Jan 2016 11:47:58 -0800 Subject: [PATCH 08/37] Fix avl copying --- src/core/support/avl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/support/avl.c b/src/core/support/avl.c index 9734c9987fe..8d3ce23e6c3 100644 --- a/src/core/support/avl.c +++ b/src/core/support/avl.c @@ -167,7 +167,7 @@ static gpr_avl_node *rotate_right_left(const gpr_avl_vtable *vtable, void *key, vtable->copy_key(right->left->key), vtable->copy_value(right->left->value), new_node(key, value, left, ref_node(right->left->left)), - new_node(vtable->copy_key(right->key), vtable->copy_key(right->value), + new_node(vtable->copy_key(right->key), vtable->copy_value(right->value), ref_node(right->left->right), ref_node(right->right))); unref_node(vtable, right); return n; From fdb603f5985b3ae604e1799b5bdbf52cda6bfd0a Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 25 Jan 2016 11:56:24 -0800 Subject: [PATCH 09/37] Fix Windows --- src/core/client_config/subchannel_index.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/client_config/subchannel_index.c b/src/core/client_config/subchannel_index.c index c4b97301612..6a712bd2a98 100644 --- a/src/core/client_config/subchannel_index.c +++ b/src/core/client_config/subchannel_index.c @@ -115,7 +115,7 @@ void grpc_subchannel_key_destroy(grpc_exec_ctx *exec_ctx, grpc_subchannel_key *k) { grpc_connector_unref(exec_ctx, k->connector); gpr_free(k->args.addr); - gpr_free(k->args.filters); + gpr_free((grpc_channel_args *)k->args.filters); grpc_channel_args_destroy((grpc_channel_args *)k->args.args); gpr_free(k); } From 2d50f37047561bdabed6ddda68baa14e691a26ea Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 28 Jan 2016 11:48:18 -0800 Subject: [PATCH 10/37] Fix copyrights, formatting --- include/grpc/support/avl.h | 4 ++-- src/core/client_config/subchannel_index.h | 2 +- src/core/support/avl.c | 2 +- tools/distrib/check_copyright.py | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/grpc/support/avl.h b/include/grpc/support/avl.h index 54605ceb7cb..23c3e466a62 100644 --- a/include/grpc/support/avl.h +++ b/include/grpc/support/avl.h @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -89,4 +89,4 @@ gpr_avl gpr_avl_remove(gpr_avl avl, void *key); returns NULL if key is not found. */ void *gpr_avl_get(gpr_avl avl, void *key); -#endif +#endif /* GRPC_SUPPORT_AVL_H */ diff --git a/src/core/client_config/subchannel_index.h b/src/core/client_config/subchannel_index.h index fc3187a758e..095ef178194 100644 --- a/src/core/client_config/subchannel_index.h +++ b/src/core/client_config/subchannel_index.h @@ -56,7 +56,7 @@ void grpc_subchannel_key_destroy(grpc_exec_ctx *exec_ctx, grpc_subchannel *grpc_subchannel_index_find(grpc_exec_ctx *exec_ctx, grpc_subchannel_key *key); -/** Register a subchannel against a key. +/** Register a subchannel against a key. Takes ownership of \a constructed. Returns the registered subchannel. This may be different from \a constructed in the case of a registration race. */ diff --git a/src/core/support/avl.c b/src/core/support/avl.c index 8d3ce23e6c3..f378b3ee17a 100644 --- a/src/core/support/avl.c +++ b/src/core/support/avl.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/tools/distrib/check_copyright.py b/tools/distrib/check_copyright.py index 6123218c553..174781e6f6d 100755 --- a/tools/distrib/check_copyright.py +++ b/tools/distrib/check_copyright.py @@ -68,9 +68,9 @@ with open('LICENSE') as f: # that given a line of license text, returns what should # be in the file LICENSE_PREFIX = { - '.c': r'\s*(//|\*)\s*', - '.cc': r'\s*(//|\*)\s*', - '.h': r'\s*(//|\*)\s*', + '.c': r'\s*(?://|\*)\s*', + '.cc': r'\s*(?://|\*)\s*', + '.h': r'\s*(?://|\*)\s*', '.m': r'\s*\*\s*', '.php': r'\s*\*\s*', '.js': r'\s*\*\s*', From 515b27149ec2e002d6282f99354be573dcf10d01 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 28 Jan 2016 11:57:09 -0800 Subject: [PATCH 11/37] Fix windows --- src/core/client_config/subchannel_index.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/client_config/subchannel_index.c b/src/core/client_config/subchannel_index.c index 6a712bd2a98..7bdec21e1cc 100644 --- a/src/core/client_config/subchannel_index.c +++ b/src/core/client_config/subchannel_index.c @@ -78,7 +78,7 @@ static grpc_subchannel_key *create_key( k->connector = grpc_connector_ref(connector); k->args.filter_count = args->filter_count; k->args.filters = gpr_malloc(sizeof(*k->args.filters) * k->args.filter_count); - memcpy(k->args.filters, args->filters, + memcpy((grpc_channel_filter*)k->args.filters, args->filters, sizeof(*k->args.filters) * k->args.filter_count); k->args.addr_len = args->addr_len; k->args.addr = gpr_malloc(args->addr_len); From f1b0849bb024fcd0794b2b0eca8af8a949f9fc70 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 28 Jan 2016 11:57:51 -0800 Subject: [PATCH 12/37] clang-format --- src/core/client_config/subchannel_index.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/client_config/subchannel_index.c b/src/core/client_config/subchannel_index.c index 7bdec21e1cc..f78a7fd588c 100644 --- a/src/core/client_config/subchannel_index.c +++ b/src/core/client_config/subchannel_index.c @@ -78,7 +78,7 @@ static grpc_subchannel_key *create_key( k->connector = grpc_connector_ref(connector); k->args.filter_count = args->filter_count; k->args.filters = gpr_malloc(sizeof(*k->args.filters) * k->args.filter_count); - memcpy((grpc_channel_filter*)k->args.filters, args->filters, + memcpy((grpc_channel_filter *)k->args.filters, args->filters, sizeof(*k->args.filters) * k->args.filter_count); k->args.addr_len = args->addr_len; k->args.addr = gpr_malloc(args->addr_len); From e1a10316522f32a18f148d8318e701adc359b948 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 2 Feb 2016 09:23:47 -0800 Subject: [PATCH 13/37] clang-format --- include/grpc/impl/codegen/grpc_types.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h index a73937ac013..b11f6ffec41 100644 --- a/include/grpc/impl/codegen/grpc_types.h +++ b/include/grpc/impl/codegen/grpc_types.h @@ -69,10 +69,10 @@ typedef enum { } grpc_arg_type; typedef struct grpc_arg_pointer_vtable { - void *(*copy)(void *p); - void (*destroy)(void *p); - int (*cmp)(void *p, void *q); - } grpc_arg_pointer_vtable; + void *(*copy)(void *p); + void (*destroy)(void *p); + int (*cmp)(void *p, void *q); +} grpc_arg_pointer_vtable; /** A single argument... each argument has a key and a value From 7e5a9cf32addfaf8e6b73fda27fa1a36d5a3821e Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 2 Feb 2016 09:28:22 -0800 Subject: [PATCH 14/37] Handle unhandled cases --- src/core/channel/channel_args.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/core/channel/channel_args.c b/src/core/channel/channel_args.c index 055b020652a..619bdd7e99a 100644 --- a/src/core/channel/channel_args.c +++ b/src/core/channel/channel_args.c @@ -37,6 +37,7 @@ #include #include +#include #include #include @@ -100,11 +101,9 @@ static int cmp_arg(const grpc_arg *a, const grpc_arg *b) { if (c != 0) return c; switch (a->type) { case GRPC_ARG_STRING: - c = strcmp(a->value.string, b->value.string); - break; + return strcmp(a->value.string, b->value.string); case GRPC_ARG_INTEGER: - c = GPR_ICMP(a->value.integer, b->value.integer); - break; + return GPR_ICMP(a->value.integer, b->value.integer); case GRPC_ARG_POINTER: c = GPR_ICMP(a->value.pointer.p, b->value.pointer.p); if (c != 0) { @@ -114,9 +113,9 @@ static int cmp_arg(const grpc_arg *a, const grpc_arg *b) { b->value.pointer.p); } } - break; + return c; } - return c; + GPR_UNREACHABLE_CODE(return 0); } static int cmp_key_stable(const void *ap, const void *bp) { From 79e2b47bf194733c48491bfb2b1bacb597d2df0c Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Tue, 9 Feb 2016 09:38:29 -0800 Subject: [PATCH 15/37] Add sanitize script --- tools/distrib/sanitize.sh | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 tools/distrib/sanitize.sh diff --git a/tools/distrib/sanitize.sh b/tools/distrib/sanitize.sh new file mode 100644 index 00000000000..56644b1d65b --- /dev/null +++ b/tools/distrib/sanitize.sh @@ -0,0 +1,38 @@ +#!/bin/bash +# Copyright 2016, 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. + +set -ex + +cd $(dirname $0)/../.. + +./tools/buildgen/generate_projects.sh +./tools/distrib/clang_format_code.sh +./tools/distrib/check_copyright.py --fix +./tools/distrib/check_trailing_newlines.sh From a72a49044beed3e7bb0b7943fdd47afe5d2e9cd3 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Tue, 9 Feb 2016 09:41:21 -0800 Subject: [PATCH 16/37] Set sanitize script executable --- tools/distrib/sanitize.sh | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 tools/distrib/sanitize.sh diff --git a/tools/distrib/sanitize.sh b/tools/distrib/sanitize.sh old mode 100644 new mode 100755 From f656f189e453c87370caa1cdffa6eed6e71b0f04 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Tue, 9 Feb 2016 10:56:25 -0800 Subject: [PATCH 17/37] Make sanitize script a suitable pre-commit hook --- tools/distrib/check_copyright.py | 9 ++++++- tools/distrib/clang_format_code.sh | 2 +- tools/distrib/sanitize.sh | 24 +++++++++++++++---- .../clang_format_all_the_things.sh | 6 +++++ 4 files changed, 35 insertions(+), 6 deletions(-) diff --git a/tools/distrib/check_copyright.py b/tools/distrib/check_copyright.py index a7efdc85cc1..ef836d6e2da 100755 --- a/tools/distrib/check_copyright.py +++ b/tools/distrib/check_copyright.py @@ -57,6 +57,9 @@ argp.add_argument('-a', '--ancient', argp.add_argument('-f', '--fix', default=False, action='store_true'); +argp.add_argument('--precommit', + default=False, + action='store_true') args = argp.parse_args() # open the license text @@ -101,6 +104,10 @@ RE_LICENSE = dict( for line in LICENSE)) for k, v in LICENSE_PREFIX.iteritems()) +if args.precommit: + FILE_LIST_COMMAND = 'git diff --name-only HEAD | grep -v ^third_party/' +else: + FILE_LIST_COMMAND = 'git ls-tree -r --name-only -r HEAD | grep -v ^third_party/' def load(name): with open(name) as f: @@ -124,7 +131,7 @@ def log(cond, why, filename): # scan files, validate the text ok = True -for filename in subprocess.check_output('git ls-tree -r --name-only -r HEAD | grep -v ^third_party/', +for filename in subprocess.check_output(FILE_LIST_COMMAND, shell=True).splitlines(): if filename in KNOWN_BAD: continue ext = os.path.splitext(filename)[1] diff --git a/tools/distrib/clang_format_code.sh b/tools/distrib/clang_format_code.sh index 6bfa278cae6..d904a841d4e 100755 --- a/tools/distrib/clang_format_code.sh +++ b/tools/distrib/clang_format_code.sh @@ -37,4 +37,4 @@ cd $(dirname $0)/../.. docker build -t grpc_clang_format tools/dockerfile/grpc_clang_format # run clang-format against the checked out codebase -docker run -e TEST=$TEST --rm=true -v ${HOST_GIT_ROOT:-`pwd`}:/local-code -t grpc_clang_format /clang_format_all_the_things.sh +docker run -e TEST=$TEST -e CHANGED_FILES="$CHANGED_FILES" --rm=true -v ${HOST_GIT_ROOT:-`pwd`}:/local-code -t grpc_clang_format /clang_format_all_the_things.sh diff --git a/tools/distrib/sanitize.sh b/tools/distrib/sanitize.sh index 56644b1d65b..3b7ca6fd88d 100755 --- a/tools/distrib/sanitize.sh +++ b/tools/distrib/sanitize.sh @@ -32,7 +32,23 @@ set -ex cd $(dirname $0)/../.. -./tools/buildgen/generate_projects.sh -./tools/distrib/clang_format_code.sh -./tools/distrib/check_copyright.py --fix -./tools/distrib/check_trailing_newlines.sh +DIFF_COMMAND="git diff --name-only HEAD | grep -v ^third_party/" + +if [ "x$1" == 'x--pre-commit' ]; then + if eval $DIFF_COMMAND | grep '^build.yaml$'; then + ./tools/buildgen/generate_projects.sh + else + templates=$(eval $DIFF_COMMAND | grep '\.template$' || true) + if [ -n "$templates" ]; then + ./tools/buildgen/generate_projects.sh --templates $templates + fi + fi + CHANGED_FILES=$(eval $DIFF_COMMAND) ./tools/distrib/clang_format_code.sh + ./tools/distrib/check_copyright.py --fix --precommit + ./tools/distrib/check_trailing_newlines.sh +else + ./tools/buildgen/generate_projects.sh + ./tools/distrib/clang_format_code.sh + ./tools/distrib/check_copyright.py --fix + ./tools/distrib/check_trailing_newlines.sh +fi diff --git a/tools/dockerfile/grpc_clang_format/clang_format_all_the_things.sh b/tools/dockerfile/grpc_clang_format/clang_format_all_the_things.sh index 86ba8b2e90b..dd8ea1ac305 100755 --- a/tools/dockerfile/grpc_clang_format/clang_format_all_the_things.sh +++ b/tools/dockerfile/grpc_clang_format/clang_format_all_the_things.sh @@ -48,6 +48,12 @@ do done done +# The CHANGED_FILES variable is used to restrict the set of files to check. +# Here we set files to the intersection of files and CHANGED_FILES +if [ -n "$CHANGED_FILES" ]; then + files=$(comm -12 <(echo $files | tr ' ' '\n' | sort -u) <(echo $CHANGED_FILES | tr ' ' '\n' | sort -u)) +fi + if [ "x$TEST" = "x" ] then echo $files | xargs $CLANG_FORMAT -i From c58a781ba2c923d379d1650ead40e5368e8a828a Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Tue, 9 Feb 2016 16:33:17 -0800 Subject: [PATCH 18/37] Handle failing git command in check_copyright.py --- tools/distrib/check_copyright.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tools/distrib/check_copyright.py b/tools/distrib/check_copyright.py index ef836d6e2da..d1a34a920dc 100755 --- a/tools/distrib/check_copyright.py +++ b/tools/distrib/check_copyright.py @@ -131,8 +131,14 @@ def log(cond, why, filename): # scan files, validate the text ok = True -for filename in subprocess.check_output(FILE_LIST_COMMAND, - shell=True).splitlines(): +filename_list = [] +try: + filename_list = subprocess.check_output(FILE_LIST_COMMAND, + shell=True).splitlines() +except subprocess.CalledProcessError: + sys.exit(0) + +for filename in filename_list: if filename in KNOWN_BAD: continue ext = os.path.splitext(filename)[1] base = os.path.basename(filename) From d59ad7ef393c7624c7035a09b488f630cbd96730 Mon Sep 17 00:00:00 2001 From: yang-g Date: Wed, 10 Feb 2016 12:42:53 -0800 Subject: [PATCH 19/37] Provide explicit API for user to set user agent string prefix --- include/grpc++/support/channel_arguments.h | 9 +- src/cpp/client/create_channel.cc | 8 +- src/cpp/common/channel_arguments.cc | 33 +++- test/cpp/common/channel_arguments_test.cc | 183 +++++++++++++-------- test/cpp/end2end/end2end_test.cc | 23 +++ 5 files changed, 174 insertions(+), 82 deletions(-) diff --git a/include/grpc++/support/channel_arguments.h b/include/grpc++/support/channel_arguments.h index a2960a7ecce..72f52657cd7 100644 --- a/include/grpc++/support/channel_arguments.h +++ b/include/grpc++/support/channel_arguments.h @@ -51,7 +51,7 @@ class ChannelArgumentsTest; /// concrete setters are provided. class ChannelArguments { public: - ChannelArguments() {} + ChannelArguments(); ~ChannelArguments() {} ChannelArguments(const ChannelArguments& other); @@ -62,8 +62,8 @@ class ChannelArguments { void Swap(ChannelArguments& other); - /// Populates this instance with the arguments from \a channel_args. Does not - /// take ownership of \a channel_args. + /// Dump arguments in this instance to \a channel_args. Does not take + /// ownership of \a channel_args. /// /// Note that the underlying arguments are shared. Changes made to either \a /// channel_args or this instance would be reflected on both. @@ -77,6 +77,9 @@ class ChannelArguments { /// Set the compression algorithm for the channel. void SetCompressionAlgorithm(grpc_compression_algorithm algorithm); + /// The given string will be sent at the front of the user agent string. + void SetUserAgentPrefix(const grpc::string& user_agent_prefix); + // Generic channel argument setters. Only for advanced use cases. /// Set an integer argument \a value under \a key. void SetInt(const grpc::string& key, int value); diff --git a/src/cpp/client/create_channel.cc b/src/cpp/client/create_channel.cc index fdaa28ffef2..76a1b31e2f7 100644 --- a/src/cpp/client/create_channel.cc +++ b/src/cpp/client/create_channel.cc @@ -32,7 +32,6 @@ */ #include -#include #include #include @@ -56,13 +55,8 @@ std::shared_ptr CreateCustomChannel( const ChannelArguments& args) { internal::GrpcLibrary init_lib; // We need to call init in case of a bad creds. - ChannelArguments cp_args = args; - std::ostringstream user_agent_prefix; - user_agent_prefix << "grpc-c++/" << grpc_version_string(); - cp_args.SetString(GRPC_ARG_PRIMARY_USER_AGENT_STRING, - user_agent_prefix.str()); return creds - ? creds->CreateChannel(target, cp_args) + ? creds->CreateChannel(target, args) : CreateChannelInternal("", grpc_lame_client_channel_create( NULL, GRPC_STATUS_INVALID_ARGUMENT, "Invalid credentials.")); diff --git a/src/cpp/common/channel_arguments.cc b/src/cpp/common/channel_arguments.cc index 90cd5136af3..e23c964797e 100644 --- a/src/cpp/common/channel_arguments.cc +++ b/src/cpp/common/channel_arguments.cc @@ -30,14 +30,23 @@ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. * */ - #include +#include + +#include #include #include "src/core/channel/channel_args.h" namespace grpc { +ChannelArguments::ChannelArguments() { + std::ostringstream user_agent_prefix; + user_agent_prefix << "grpc-c++/" << grpc_version_string(); + // This will be ignored if used on the server side. + SetString(GRPC_ARG_PRIMARY_USER_AGENT_STRING, user_agent_prefix.str()); +} + ChannelArguments::ChannelArguments(const ChannelArguments& other) : strings_(other.strings_) { args_.reserve(other.args_.size()); @@ -81,6 +90,28 @@ void ChannelArguments::SetCompressionAlgorithm( SetInt(GRPC_COMPRESSION_ALGORITHM_ARG, algorithm); } +// Note: a second call to this will add in front the result of the first call. +void ChannelArguments::SetUserAgentPrefix( + const grpc::string& user_agent_prefix) { + if (user_agent_prefix.empty()) { + return; + } + bool replaced = false; + for (auto it = args_.begin(); it != args_.end(); ++it) { + const grpc_arg& arg = *it; + if (arg.type == GRPC_ARG_STRING && + grpc::string(arg.key) == GRPC_ARG_PRIMARY_USER_AGENT_STRING) { + strings_.push_back(user_agent_prefix + " " + arg.value.string); + it->value.string = const_cast(strings_.back().c_str()); + replaced = true; + break; + } + } + if (!replaced) { + SetString(GRPC_ARG_PRIMARY_USER_AGENT_STRING, user_agent_prefix); + } +} + void ChannelArguments::SetInt(const grpc::string& key, int value) { grpc_arg arg; arg.type = GRPC_ARG_INTEGER; diff --git a/test/cpp/common/channel_arguments_test.cc b/test/cpp/common/channel_arguments_test.cc index e010d375cf9..8a6e9b2222e 100644 --- a/test/cpp/common/channel_arguments_test.cc +++ b/test/cpp/common/channel_arguments_test.cc @@ -45,90 +45,131 @@ class ChannelArgumentsTest : public ::testing::Test { grpc_channel_args* args) { channel_args.SetChannelArgs(args); } + + grpc::string GetDefaultUserAgentPrefix() { + std::ostringstream user_agent_prefix; + user_agent_prefix << "grpc-c++/" << grpc_version_string(); + return user_agent_prefix.str(); + } + + void VerifyDefaultChannelArgs() { + grpc_channel_args args; + SetChannelArgs(channel_args_, &args); + EXPECT_EQ(static_cast(1), args.num_args); + EXPECT_STREQ(GRPC_ARG_PRIMARY_USER_AGENT_STRING, args.args[0].key); + EXPECT_EQ(GetDefaultUserAgentPrefix(), + grpc::string(args.args[0].value.string)); + } + + bool HasArg(grpc_arg expected_arg) { + grpc_channel_args args; + SetChannelArgs(channel_args_, &args); + for (size_t i = 0; i < args.num_args; i++) { + const grpc_arg& arg = args.args[i]; + if (arg.type == expected_arg.type && + grpc::string(arg.key) == expected_arg.key) { + if (arg.type == GRPC_ARG_INTEGER) { + return arg.value.integer == expected_arg.value.integer; + } else if (arg.type == GRPC_ARG_STRING) { + return grpc::string(arg.value.string) == expected_arg.value.string; + } else if (arg.type == GRPC_ARG_POINTER) { + return arg.value.pointer.p == expected_arg.value.pointer.p && + arg.value.pointer.copy == expected_arg.value.pointer.copy && + arg.value.pointer.destroy == + expected_arg.value.pointer.destroy; + } + } + } + return false; + } + ChannelArguments channel_args_; }; TEST_F(ChannelArgumentsTest, SetInt) { - grpc_channel_args args; - ChannelArguments channel_args; - // Empty arguments. - SetChannelArgs(channel_args, &args); - EXPECT_EQ(static_cast(0), args.num_args); - - grpc::string key("key0"); - channel_args.SetInt(key, 0); + VerifyDefaultChannelArgs(); + grpc::string key0("key0"); + grpc_arg arg0; + arg0.type = GRPC_ARG_INTEGER; + arg0.key = const_cast(key0.c_str()); + arg0.value.integer = 0; + grpc::string key1("key1"); + grpc_arg arg1; + arg1.type = GRPC_ARG_INTEGER; + arg1.key = const_cast(key1.c_str()); + arg1.value.integer = 1; + + grpc::string arg_key0(key0); + channel_args_.SetInt(arg_key0, arg0.value.integer); // Clear key early to make sure channel_args takes a copy - key = ""; - SetChannelArgs(channel_args, &args); - EXPECT_EQ(static_cast(1), args.num_args); - EXPECT_EQ(GRPC_ARG_INTEGER, args.args[0].type); - EXPECT_STREQ("key0", args.args[0].key); - EXPECT_EQ(0, args.args[0].value.integer); - - key = "key1"; - channel_args.SetInt(key, 1); - key = ""; - SetChannelArgs(channel_args, &args); - EXPECT_EQ(static_cast(2), args.num_args); - // We do not enforce order on the arguments. - for (size_t i = 0; i < args.num_args; i++) { - EXPECT_EQ(GRPC_ARG_INTEGER, args.args[i].type); - if (grpc::string(args.args[i].key) == "key0") { - EXPECT_EQ(0, args.args[i].value.integer); - } else if (grpc::string(args.args[i].key) == "key1") { - EXPECT_EQ(1, args.args[i].value.integer); - } - } + arg_key0.clear(); + EXPECT_TRUE(HasArg(arg0)); + + grpc::string arg_key1(key1); + channel_args_.SetInt(arg_key1, arg1.value.integer); + arg_key1.clear(); + EXPECT_TRUE(HasArg(arg0)); + EXPECT_TRUE(HasArg(arg1)); } TEST_F(ChannelArgumentsTest, SetString) { - grpc_channel_args args; - ChannelArguments channel_args; - // Empty arguments. - SetChannelArgs(channel_args, &args); - EXPECT_EQ(static_cast(0), args.num_args); - - grpc::string key("key0"); - grpc::string val("val0"); - channel_args.SetString(key, val); + VerifyDefaultChannelArgs(); + grpc::string key0("key0"); + grpc::string val0("val0"); + grpc_arg arg0; + arg0.type = GRPC_ARG_STRING; + arg0.key = const_cast(key0.c_str()); + arg0.value.string = const_cast(val0.c_str()); + grpc::string key1("key1"); + grpc::string val1("val1"); + grpc_arg arg1; + arg1.type = GRPC_ARG_STRING; + arg1.key = const_cast(key1.c_str()); + arg1.value.string = const_cast(val1.c_str()); + + grpc::string key(key0); + grpc::string val(val0); + channel_args_.SetString(key, val); // Clear key/val early to make sure channel_args takes a copy key = ""; val = ""; - SetChannelArgs(channel_args, &args); - EXPECT_EQ(static_cast(1), args.num_args); - EXPECT_EQ(GRPC_ARG_STRING, args.args[0].type); - EXPECT_STREQ("key0", args.args[0].key); - EXPECT_STREQ("val0", args.args[0].value.string); - - key = "key1"; - val = "val1"; - channel_args.SetString(key, val); - SetChannelArgs(channel_args, &args); - EXPECT_EQ(static_cast(2), args.num_args); - // We do not enforce order on the arguments. - for (size_t i = 0; i < args.num_args; i++) { - EXPECT_EQ(GRPC_ARG_STRING, args.args[i].type); - if (grpc::string(args.args[i].key) == "key0") { - EXPECT_STREQ("val0", args.args[i].value.string); - } else if (grpc::string(args.args[i].key) == "key1") { - EXPECT_STREQ("val1", args.args[i].value.string); - } - } + EXPECT_TRUE(HasArg(arg0)); + + key = key1; + val = val1; + channel_args_.SetString(key, val); + // Clear key/val early to make sure channel_args takes a copy + key = ""; + val = ""; + EXPECT_TRUE(HasArg(arg0)); + EXPECT_TRUE(HasArg(arg1)); } TEST_F(ChannelArgumentsTest, SetPointer) { - grpc_channel_args args; - ChannelArguments channel_args; - // Empty arguments. - SetChannelArgs(channel_args, &args); - EXPECT_EQ(static_cast(0), args.num_args); - - grpc::string key("key0"); - channel_args.SetPointer(key, &key); - SetChannelArgs(channel_args, &args); - EXPECT_EQ(static_cast(1), args.num_args); - EXPECT_EQ(GRPC_ARG_POINTER, args.args[0].type); - EXPECT_STREQ("key0", args.args[0].key); - EXPECT_EQ(&key, args.args[0].value.pointer.p); + VerifyDefaultChannelArgs(); + grpc::string key0("key0"); + grpc_arg arg0; + arg0.type = GRPC_ARG_POINTER; + arg0.key = const_cast(key0.c_str()); + arg0.value.pointer.p = &key0; + arg0.value.pointer.copy = nullptr; + arg0.value.pointer.destroy = nullptr; + + grpc::string key(key0); + channel_args_.SetPointer(key, arg0.value.pointer.p); + EXPECT_TRUE(HasArg(arg0)); +} + +TEST_F(ChannelArgumentsTest, SetUserAgentPrefix) { + VerifyDefaultChannelArgs(); + grpc::string prefix("prefix"); + grpc::string whole_prefix = prefix + " " + GetDefaultUserAgentPrefix(); + grpc_arg arg0; + arg0.type = GRPC_ARG_STRING; + arg0.key = const_cast(GRPC_ARG_PRIMARY_USER_AGENT_STRING); + arg0.value.string = const_cast(whole_prefix.c_str()); + + channel_args_.SetUserAgentPrefix(prefix); + EXPECT_TRUE(HasArg(arg0)); } } // namespace testing diff --git a/test/cpp/end2end/end2end_test.cc b/test/cpp/end2end/end2end_test.cc index 65da71b3915..c8523847ab2 100644 --- a/test/cpp/end2end/end2end_test.cc +++ b/test/cpp/end2end/end2end_test.cc @@ -252,6 +252,9 @@ class End2endTest : public ::testing::TestWithParam { args.SetSslTargetNameOverride("foo.test.google.fr"); channel_creds = SslCredentials(ssl_opts); } + if (!user_agent_prefix_.empty()) { + args.SetUserAgentPrefix(user_agent_prefix_); + } args.SetString(GRPC_ARG_SECONDARY_USER_AGENT_STRING, "end2end_test"); channel_ = CreateCustomChannel(server_address_.str(), channel_creds, args); } @@ -285,6 +288,7 @@ class End2endTest : public ::testing::TestWithParam { TestServiceImpl service_; TestServiceImpl special_service_; TestServiceImplDupPkg dup_pkg_service_; + grpc::string user_agent_prefix_; }; static void SendRpc(grpc::testing::EchoTestService::Stub* stub, int num_rpcs, @@ -601,6 +605,25 @@ TEST_P(End2endServerTryCancelTest, BidiStreamServerCancelAfter) { TestBidiStreamServerCancel(CANCEL_AFTER_PROCESSING, 5); } +TEST_P(End2endTest, SimpleRpcWithCustomeUserAgentPrefix) { + user_agent_prefix_ = "custom_prefix"; + ResetStub(); + EchoRequest request; + EchoResponse response; + request.set_message("Hello hello hello hello"); + request.mutable_param()->set_echo_metadata(true); + + ClientContext context; + Status s = stub_->Echo(&context, request, &response); + EXPECT_EQ(response.message(), request.message()); + EXPECT_TRUE(s.ok()); + const auto& trailing_metadata = context.GetServerTrailingMetadata(); + auto iter = trailing_metadata.find("user-agent"); + EXPECT_TRUE(iter != trailing_metadata.end()); + grpc::string expected_prefix = user_agent_prefix_ + " grpc-c++/"; + EXPECT_TRUE(iter->second.starts_with(expected_prefix)); +} + TEST_P(End2endTest, MultipleRpcsWithVariedBinaryMetadataValue) { ResetStub(); std::vector threads; From 1945ae42854689afa398e46d3086e55706fee655 Mon Sep 17 00:00:00 2001 From: yang-g Date: Wed, 10 Feb 2016 12:48:32 -0800 Subject: [PATCH 20/37] Fix copyright --- include/grpc++/support/channel_arguments.h | 2 +- src/cpp/common/channel_arguments.cc | 2 +- src/csharp/Grpc.Core.Tests/GrpcEnvironmentTest.cs | 2 +- src/csharp/Grpc.Core/Version.cs | 2 +- test/cpp/common/channel_arguments_test.cc | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/grpc++/support/channel_arguments.h b/include/grpc++/support/channel_arguments.h index 72f52657cd7..4e530d4b89c 100644 --- a/include/grpc++/support/channel_arguments.h +++ b/include/grpc++/support/channel_arguments.h @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/src/cpp/common/channel_arguments.cc b/src/cpp/common/channel_arguments.cc index e23c964797e..72e7b2b4cf5 100644 --- a/src/cpp/common/channel_arguments.cc +++ b/src/cpp/common/channel_arguments.cc @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/src/csharp/Grpc.Core.Tests/GrpcEnvironmentTest.cs b/src/csharp/Grpc.Core.Tests/GrpcEnvironmentTest.cs index ab12c120cba..fac93fcc5cc 100644 --- a/src/csharp/Grpc.Core.Tests/GrpcEnvironmentTest.cs +++ b/src/csharp/Grpc.Core.Tests/GrpcEnvironmentTest.cs @@ -1,6 +1,6 @@ #region Copyright notice and license -// Copyright 2015, Google Inc. +// Copyright 2015-2016, Google Inc. // All rights reserved. // // Redistribution and use in source and binary forms, with or without diff --git a/src/csharp/Grpc.Core/Version.cs b/src/csharp/Grpc.Core/Version.cs index 8a26bd83623..6d88438a07b 100644 --- a/src/csharp/Grpc.Core/Version.cs +++ b/src/csharp/Grpc.Core/Version.cs @@ -1,6 +1,6 @@ #region Copyright notice and license -// Copyright 2015, Google Inc. +// Copyright 2015-2016, Google Inc. // All rights reserved. // // Redistribution and use in source and binary forms, with or without diff --git a/test/cpp/common/channel_arguments_test.cc b/test/cpp/common/channel_arguments_test.cc index 8a6e9b2222e..5ee63bf6885 100644 --- a/test/cpp/common/channel_arguments_test.cc +++ b/test/cpp/common/channel_arguments_test.cc @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without From 24e274b8d9c9617bc10b9dcde66864fb2884b58c Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 11 Feb 2016 08:54:49 -0800 Subject: [PATCH 21/37] Add comment --- src/core/channel/channel_args.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/channel/channel_args.c b/src/core/channel/channel_args.c index 619bdd7e99a..bae7a90a015 100644 --- a/src/core/channel/channel_args.c +++ b/src/core/channel/channel_args.c @@ -118,6 +118,8 @@ static int cmp_arg(const grpc_arg *a, const grpc_arg *b) { GPR_UNREACHABLE_CODE(return 0); } +/* stabilizing comparison function: since channel_args ordering matters for + * keys with the same name, we need to preserve that ordering */ static int cmp_key_stable(const void *ap, const void *bp) { const grpc_arg *const *a = ap; const grpc_arg *const *b = bp; From 1a06082265d0b8a7567ab5403294252b6be610c5 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Fri, 12 Feb 2016 17:34:08 -0800 Subject: [PATCH 22/37] Add 70% load scenario for testing with open-loop --- test/cpp/qps/qps-sweep.sh | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/cpp/qps/qps-sweep.sh b/test/cpp/qps/qps-sweep.sh index 539da1d8930..5287930590a 100755 --- a/test/cpp/qps/qps-sweep.sh +++ b/test/cpp/qps/qps-sweep.sh @@ -55,7 +55,7 @@ for secure in true false; do --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=100 \ --client_channels=64 --bbuf_req_size=0 --bbuf_resp_size=0 \ --async_client_threads=0 --async_server_threads=0 --secure_test=$secure \ - --num_servers=1 --num_clients=0 + --num_servers=1 --num_clients=0 |& tee /tmp/qps-test.$$ # Scenario 2b: QPS with a single server core "$bins"/opt/qps_driver --rpc_type=STREAMING --client_type=ASYNC_CLIENT \ @@ -71,7 +71,15 @@ for secure in true false; do --async_client_threads=0 --async_server_threads=0 --secure_test=$secure \ --num_servers=1 --num_clients=0 - # Scenario 3: Latency at near-peak load (TBD) + # Scenario 3: Latency at near-peak load (all clients equally loaded) + "$bins"/opt/qps_driver --rpc_type=STREAMING --client_type=ASYNC_CLIENT \ + --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=100 \ + --client_channels=64 --bbuf_req_size=0 --bbuf_resp_size=0 \ + --async_client_threads=0 --async_server_threads=0 --secure_test=$secure \ + --num_servers=1 --num_clients=0 --poisson_load=`awk '$5 == "QPS:" \ + {print int(0.7 * $6); exit}' /tmp/qps-test.$$` + + rm /tmp/qps-test.$$ # Scenario 4: Single-channel bidirectional throughput test (like TCP_STREAM). "$bins"/opt/qps_driver --rpc_type=STREAMING --client_type=ASYNC_CLIENT \ From fcc8f89eea3b9cd2aacef2e6789de7ac03417793 Mon Sep 17 00:00:00 2001 From: vjpai Date: Tue, 16 Feb 2016 17:30:12 -0800 Subject: [PATCH 23/37] Fix copyright --- test/core/iomgr/udp_server_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/iomgr/udp_server_test.c b/test/core/iomgr/udp_server_test.c index 0bb69a2a764..2e253d8a8a1 100644 --- a/test/core/iomgr/udp_server_test.c +++ b/test/core/iomgr/udp_server_test.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without From 399d34bef4c7cf85de7970a6d0f654c3af933fbd Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Tue, 16 Feb 2016 19:37:15 -0800 Subject: [PATCH 24/37] Fix pygrpc_load_core in pxi to match with the C definition --- src/python/grpcio/grpc/_cython/_cygrpc/grpc.pxi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/grpc.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/grpc.pxi index bbeed9ad401..800d0ea2f6f 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/grpc.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/grpc.pxi @@ -36,7 +36,7 @@ cdef extern from "grpc/_cython/loader.h": ctypedef unsigned uint32_t ctypedef long int64_t - int pygrpc_load_core(const char*) + int pygrpc_load_core(char*) void *gpr_malloc(size_t size) void gpr_free(void *ptr) From e813bc760e72ed07aa31858e869f8f56f7adeed0 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Wed, 17 Feb 2016 10:34:42 +0000 Subject: [PATCH 25/37] Add a NullLogger in the C# implementation I'd argue that at this stage, this should probably be the default. Further options to consider: - Make it a singleton, or at least expose a static property with a single instance? - Allow SetLogger(null) to use the NullLogger? --- src/csharp/Grpc.Core/Grpc.Core.csproj | 1 + src/csharp/Grpc.Core/Logging/NullLogger.cs | 122 +++++++++++++++++++++ 2 files changed, 123 insertions(+) create mode 100644 src/csharp/Grpc.Core/Logging/NullLogger.cs diff --git a/src/csharp/Grpc.Core/Grpc.Core.csproj b/src/csharp/Grpc.Core/Grpc.Core.csproj index 9587503e4b3..8d7d2cae0de 100644 --- a/src/csharp/Grpc.Core/Grpc.Core.csproj +++ b/src/csharp/Grpc.Core/Grpc.Core.csproj @@ -59,6 +59,7 @@ + diff --git a/src/csharp/Grpc.Core/Logging/NullLogger.cs b/src/csharp/Grpc.Core/Logging/NullLogger.cs new file mode 100644 index 00000000000..58679a0ff9f --- /dev/null +++ b/src/csharp/Grpc.Core/Logging/NullLogger.cs @@ -0,0 +1,122 @@ +#region Copyright notice and license + +// Copyright 2016, 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. + +#endregion + +using System; + +namespace Grpc.Core.Logging +{ + /// + /// Logger which doesn't log any information anywhere. + /// + public sealed class NullLogger : ILogger + { + /// + /// As with all logging calls on this logger, this method is a no-op. + /// + public void Debug(string message) + { + } + + /// + /// As with all logging calls on this logger, this method is a no-op. + /// + public void Debug(string format, params object[] formatArgs) + { + } + + /// + /// As with all logging calls on this logger, this method is a no-op. + /// + public void Error(string message) + { + } + + /// + /// As with all logging calls on this logger, this method is a no-op. + /// + public void Error(Exception exception, string message) + { + } + + /// + /// As with all logging calls on this logger, this method is a no-op. + /// + public void Error(string format, params object[] formatArgs) + { + } + + /// + /// Returns a reference to the instance on which the method is called, as + /// instances aren't associated with specific types. + /// + public ILogger ForType() + { + return this; + } + + /// + /// As with all logging calls on this logger, this method is a no-op. + /// + public void Info(string message) + { + } + + /// + /// As with all logging calls on this logger, this method is a no-op. + /// + public void Info(string format, params object[] formatArgs) + { + } + + /// + /// As with all logging calls on this logger, this method is a no-op. + /// + public void Warning(string message) + { + } + + /// + /// As with all logging calls on this logger, this method is a no-op. + /// + public void Warning(Exception exception, string message) + { + } + + /// + /// As with all logging calls on this logger, this method is a no-op. + /// + public void Warning(string format, params object[] formatArgs) + { + } + } +} From ed3cd423b69318044feadd1a8a7238b9451e9810 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Wed, 17 Feb 2016 08:49:03 -0800 Subject: [PATCH 26/37] Use three different load-factors and document load-factor variable --- test/cpp/qps/qps-sweep.sh | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/test/cpp/qps/qps-sweep.sh b/test/cpp/qps/qps-sweep.sh index 5287930590a..540e4e66087 100755 --- a/test/cpp/qps/qps-sweep.sh +++ b/test/cpp/qps/qps-sweep.sh @@ -71,13 +71,15 @@ for secure in true false; do --async_client_threads=0 --async_server_threads=0 --secure_test=$secure \ --num_servers=1 --num_clients=0 - # Scenario 3: Latency at near-peak load (all clients equally loaded) - "$bins"/opt/qps_driver --rpc_type=STREAMING --client_type=ASYNC_CLIENT \ - --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=100 \ - --client_channels=64 --bbuf_req_size=0 --bbuf_resp_size=0 \ - --async_client_threads=0 --async_server_threads=0 --secure_test=$secure \ - --num_servers=1 --num_clients=0 --poisson_load=`awk '$5 == "QPS:" \ - {print int(0.7 * $6); exit}' /tmp/qps-test.$$` + # Scenario 3: Latency at sub-peak load (all clients equally loaded) + for loadfactor in 0.2 0.5 0.7; do + "$bins"/opt/qps_driver --rpc_type=STREAMING --client_type=ASYNC_CLIENT \ + --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=100 \ + --client_channels=64 --bbuf_req_size=0 --bbuf_resp_size=0 \ + --async_client_threads=0 --async_server_threads=0 --secure_test=$secure \ + --num_servers=1 --num_clients=0 --poisson_load=`awk -v lf=$loadfactor \ + '$5 == "QPS:" {print int(lf * $6); exit}' /tmp/qps-test.$$` + done rm /tmp/qps-test.$$ From 8a2ff73640fc106e59253cb837bfa2bdcf243fc8 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Wed, 17 Feb 2016 09:21:10 -0800 Subject: [PATCH 27/37] Use more variables for better explanations --- test/cpp/qps/qps-sweep.sh | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/test/cpp/qps/qps-sweep.sh b/test/cpp/qps/qps-sweep.sh index 540e4e66087..89d373279f6 100755 --- a/test/cpp/qps/qps-sweep.sh +++ b/test/cpp/qps/qps-sweep.sh @@ -40,6 +40,8 @@ bins=`find . .. ../.. ../../.. -name bins | head -1` set -x big=65536 +wide=64 +deep=100 half=`echo $QPS_WORKERS | awk -F, '{print int(NF/2)}'` for secure in true false; do @@ -52,30 +54,30 @@ for secure in true false; do # Scenario 2: generic async streaming "unconstrained" (QPS) "$bins"/opt/qps_driver --rpc_type=STREAMING --client_type=ASYNC_CLIENT \ - --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=100 \ - --client_channels=64 --bbuf_req_size=0 --bbuf_resp_size=0 \ + --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=$deep \ + --client_channels=$wide --bbuf_req_size=0 --bbuf_resp_size=0 \ --async_client_threads=0 --async_server_threads=0 --secure_test=$secure \ --num_servers=1 --num_clients=0 |& tee /tmp/qps-test.$$ # Scenario 2b: QPS with a single server core "$bins"/opt/qps_driver --rpc_type=STREAMING --client_type=ASYNC_CLIENT \ - --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=100 \ - --client_channels=64 --bbuf_req_size=0 --bbuf_resp_size=0 \ + --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=$deep \ + --client_channels=$wide --bbuf_req_size=0 --bbuf_resp_size=0 \ --async_client_threads=0 --async_server_threads=0 --secure_test=$secure \ --num_servers=1 --num_clients=0 --server_core_limit=1 # Scenario 2c: protobuf-based QPS "$bins"/opt/qps_driver --rpc_type=STREAMING --client_type=ASYNC_CLIENT \ - --server_type=ASYNC_SERVER --outstanding_rpcs_per_channel=100 \ - --client_channels=64 --simple_req_size=0 --simple_resp_size=0 \ + --server_type=ASYNC_SERVER --outstanding_rpcs_per_channel=$deep \ + --client_channels=$wide --simple_req_size=0 --simple_resp_size=0 \ --async_client_threads=0 --async_server_threads=0 --secure_test=$secure \ --num_servers=1 --num_clients=0 # Scenario 3: Latency at sub-peak load (all clients equally loaded) for loadfactor in 0.2 0.5 0.7; do "$bins"/opt/qps_driver --rpc_type=STREAMING --client_type=ASYNC_CLIENT \ - --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=100 \ - --client_channels=64 --bbuf_req_size=0 --bbuf_resp_size=0 \ + --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=$deep \ + --client_channels=$wide --bbuf_req_size=0 --bbuf_resp_size=0 \ --async_client_threads=0 --async_server_threads=0 --secure_test=$secure \ --num_servers=1 --num_clients=0 --poisson_load=`awk -v lf=$loadfactor \ '$5 == "QPS:" {print int(lf * $6); exit}' /tmp/qps-test.$$` @@ -85,7 +87,7 @@ for secure in true false; do # Scenario 4: Single-channel bidirectional throughput test (like TCP_STREAM). "$bins"/opt/qps_driver --rpc_type=STREAMING --client_type=ASYNC_CLIENT \ - --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=100 \ + --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=$deep \ --client_channels=1 --bbuf_req_size=$big --bbuf_resp_size=$big \ --async_client_threads=1 --async_server_threads=1 --secure_test=$secure \ --num_servers=1 --num_clients=1 @@ -118,35 +120,35 @@ for secure in true false; do # Scenario 9: Crossbar QPS test "$bins"/opt/qps_driver --rpc_type=STREAMING --client_type=ASYNC_CLIENT \ - --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=100 \ - --client_channels=64 --bbuf_req_size=0 --bbuf_resp_size=0 \ + --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=$deep \ + --client_channels=$wide --bbuf_req_size=0 --bbuf_resp_size=0 \ --async_client_threads=0 --async_server_threads=0 --secure_test=$secure \ --num_servers=$half --num_clients=0 # Scenario 10: Multi-channel bidir throughput test "$bins"/opt/qps_driver --rpc_type=STREAMING --client_type=ASYNC_CLIENT \ - --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=100 \ - --client_channels=64 --bbuf_req_size=$big --bbuf_resp_size=$big \ + --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=1 \ + --client_channels=$wide --bbuf_req_size=$big --bbuf_resp_size=$big \ --async_client_threads=0 --async_server_threads=0 --secure_test=$secure \ --num_servers=1 --num_clients=1 # Scenario 11: Single-channel request throughput test "$bins"/opt/qps_driver --rpc_type=STREAMING --client_type=ASYNC_CLIENT \ - --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=100 \ + --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=$deep \ --client_channels=1 --bbuf_req_size=$big --bbuf_resp_size=0 \ --async_client_threads=1 --async_server_threads=1 --secure_test=$secure \ --num_servers=1 --num_clients=1 # Scenario 12: Single-channel response throughput test "$bins"/opt/qps_driver --rpc_type=STREAMING --client_type=ASYNC_CLIENT \ - --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=100 \ + --server_type=ASYNC_GENERIC_SERVER --outstanding_rpcs_per_channel=$deep \ --client_channels=1 --bbuf_req_size=0 --bbuf_resp_size=$big \ --async_client_threads=1 --async_server_threads=1 --secure_test=$secure \ --num_servers=1 --num_clients=1 # Scenario 13: Single-channel bidirectional protobuf throughput test "$bins"/opt/qps_driver --rpc_type=STREAMING --client_type=ASYNC_CLIENT \ - --server_type=ASYNC_SERVER --outstanding_rpcs_per_channel=100 \ + --server_type=ASYNC_SERVER --outstanding_rpcs_per_channel=$deep \ --client_channels=1 --simple_req_size=$big --simple_resp_size=$big \ --async_client_threads=1 --async_server_threads=1 --secure_test=$secure \ --num_servers=1 --num_clients=1 From d01391233ad11cfd77b909e22cfd7bc6de62c2af Mon Sep 17 00:00:00 2001 From: Masood Malekghassemi Date: Tue, 16 Feb 2016 21:08:49 -0800 Subject: [PATCH 28/37] Extern "C" Python DLL support --- src/python/grpcio/grpc/_cython/imports.generated.c | 8 ++++++++ src/python/grpcio/grpc/_cython/imports.generated.h | 8 ++++++++ src/python/grpcio/grpc/_cython/loader.c | 11 ++++++++++- src/python/grpcio/grpc/_cython/loader.h | 9 +++++++++ .../grpcio/grpc/_cython/imports.generated.c.template | 9 +++++++++ .../grpcio/grpc/_cython/imports.generated.h.template | 8 ++++++++ 6 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/python/grpcio/grpc/_cython/imports.generated.c b/src/python/grpcio/grpc/_cython/imports.generated.c index 817303c8a4a..4aa41dbcd73 100644 --- a/src/python/grpcio/grpc/_cython/imports.generated.c +++ b/src/python/grpcio/grpc/_cython/imports.generated.c @@ -296,6 +296,10 @@ gpr_thd_options_is_joinable_type gpr_thd_options_is_joinable_import; gpr_thd_currentid_type gpr_thd_currentid_import; gpr_thd_join_type gpr_thd_join_import; +#ifdef __cplusplus +extern "C" { +#endif /* __cpluslus */ + void pygrpc_load_imports(HMODULE library) { census_initialize_import = (census_initialize_type) GetProcAddress(library, "census_initialize"); census_shutdown_import = (census_shutdown_type) GetProcAddress(library, "census_shutdown"); @@ -557,4 +561,8 @@ void pygrpc_load_imports(HMODULE library) { gpr_thd_join_import = (gpr_thd_join_type) GetProcAddress(library, "gpr_thd_join"); } +#ifdef __cplusplus +} +#endif /* __cpluslus */ + #endif /* !GPR_WIN32 */ diff --git a/src/python/grpcio/grpc/_cython/imports.generated.h b/src/python/grpcio/grpc/_cython/imports.generated.h index 6d0a6e06c00..f5329f33782 100644 --- a/src/python/grpcio/grpc/_cython/imports.generated.h +++ b/src/python/grpcio/grpc/_cython/imports.generated.h @@ -836,8 +836,16 @@ typedef void(*gpr_thd_join_type)(gpr_thd_id t); extern gpr_thd_join_type gpr_thd_join_import; #define gpr_thd_join gpr_thd_join_import +#ifdef __cplusplus +extern "C" { +#endif /* __cpluslus */ + void pygrpc_load_imports(HMODULE library); +#ifdef __cplusplus +} +#endif /* __cpluslus */ + #else /* !GPR_WIN32 */ #include diff --git a/src/python/grpcio/grpc/_cython/loader.c b/src/python/grpcio/grpc/_cython/loader.c index cdd47deed30..3b72806ea18 100644 --- a/src/python/grpcio/grpc/_cython/loader.c +++ b/src/python/grpcio/grpc/_cython/loader.c @@ -33,6 +33,10 @@ #include "loader.h" +#ifdef __cplusplus +extern "C" { +#endif /* __cpluslus */ + #if GPR_WIN32 int pygrpc_load_core(char *path) { @@ -56,4 +60,9 @@ int pygrpc_load_core(char *path) { int pygrpc_load_core(char *path) { return 1; } -#endif +#endif /* !GPR_WIN32 */ + +#ifdef __cplusplus +} +#endif /* __cpluslus */ + diff --git a/src/python/grpcio/grpc/_cython/loader.h b/src/python/grpcio/grpc/_cython/loader.h index dd31e1561b5..3b8796d39f7 100644 --- a/src/python/grpcio/grpc/_cython/loader.h +++ b/src/python/grpcio/grpc/_cython/loader.h @@ -39,7 +39,16 @@ /* Additional inclusions not covered by "imports.generated.h" */ #include +#ifdef __cplusplus +extern "C" { +#endif /* __cpluslus */ + /* Attempts to load the core if necessary, and return non-zero upon succes. */ int pygrpc_load_core(char *path); +#ifdef __cplusplus +} +#endif /* __cpluslus */ + #endif /* GRPC_RB_BYTE_BUFFER_H_ */ + diff --git a/templates/src/python/grpcio/grpc/_cython/imports.generated.c.template b/templates/src/python/grpcio/grpc/_cython/imports.generated.c.template index be33280c0ce..62fe0947d25 100644 --- a/templates/src/python/grpcio/grpc/_cython/imports.generated.c.template +++ b/templates/src/python/grpcio/grpc/_cython/imports.generated.c.template @@ -43,10 +43,19 @@ ${api.name}_type ${api.name}_import; %endfor + #ifdef __cplusplus + extern "C" { + #endif /* __cpluslus */ + void pygrpc_load_imports(HMODULE library) { %for api in c_apis: ${api.name}_import = (${api.name}_type) GetProcAddress(library, "${api.name}"); %endfor } + #ifdef __cplusplus + } + #endif /* __cpluslus */ + #endif /* !GPR_WIN32 */ + diff --git a/templates/src/python/grpcio/grpc/_cython/imports.generated.h.template b/templates/src/python/grpcio/grpc/_cython/imports.generated.h.template index 6866a61caef..8e7c1831800 100644 --- a/templates/src/python/grpcio/grpc/_cython/imports.generated.h.template +++ b/templates/src/python/grpcio/grpc/_cython/imports.generated.h.template @@ -52,8 +52,16 @@ #define ${api.name} ${api.name}_import %endfor + #ifdef __cplusplus + extern "C" { + #endif /* __cpluslus */ + void pygrpc_load_imports(HMODULE library); + #ifdef __cplusplus + } + #endif /* __cpluslus */ + #else /* !GPR_WIN32 */ #include From c02910b07ae492098d7d0c1bca747fbcad742393 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 17 Feb 2016 12:59:26 -0800 Subject: [PATCH 29/37] Node: add options to modify ProtoBuf behavior --- src/node/index.js | 40 +++++++++++++++++++++++++--------------- src/node/src/client.js | 6 ++++-- src/node/src/common.js | 29 ++++++++++++++++++++++++----- src/node/src/server.js | 7 ++++++- 4 files changed, 59 insertions(+), 23 deletions(-) diff --git a/src/node/index.js b/src/node/index.js index 7eacdc67b1d..4e4d12d9e45 100644 --- a/src/node/index.js +++ b/src/node/index.js @@ -56,17 +56,18 @@ var grpc = require('./src/grpc_extension'); /** * Load a gRPC object from an existing ProtoBuf.Reflect object. * @param {ProtoBuf.Reflect.Namespace} value The ProtoBuf object to load. + * @param {Object=} options Options to apply to the loaded object * @return {Object} The resulting gRPC object */ -exports.loadObject = function loadObject(value) { +exports.loadObject = function loadObject(value, options) { var result = {}; if (value.className === 'Namespace') { _.each(value.children, function(child) { - result[child.name] = loadObject(child); + result[child.name] = loadObject(child, options); }); return result; } else if (value.className === 'Service') { - return client.makeProtobufClientConstructor(value); + return client.makeProtobufClientConstructor(value, options); } else if (value.className === 'Message' || value.className === 'Enum') { return value.build(); } else { @@ -78,27 +79,36 @@ var loadObject = exports.loadObject; /** * Load a gRPC object from a .proto file. - * @param {string} filename The file to load + * @param {string|{root: string, file: string}} filename The file to load * @param {string=} format The file format to expect. Must be either 'proto' or * 'json'. Defaults to 'proto' + * @param {Object=} options Options to apply to the loaded file * @return {Object} The resulting gRPC object */ -exports.load = function load(filename, format) { +exports.load = function load(filename, format, options) { if (!format) { format = 'proto'; } + var convertFieldsToCamelCaseOriginal = ProtoBuf.convertFieldsToCamelCase; + if(options && options.hasOwnProperty('convertFieldsToCamelCase')) { + ProtoBuf.convertFieldsToCamelCase = options.convertFieldsToCamelCase; + } var builder; - switch(format) { - case 'proto': - builder = ProtoBuf.loadProtoFile(filename); - break; - case 'json': - builder = ProtoBuf.loadJsonFile(filename); - break; - default: - throw new Error('Unrecognized format "' + format + '"'); + try { + switch(format) { + case 'proto': + builder = ProtoBuf.loadProtoFile(filename); + break; + case 'json': + builder = ProtoBuf.loadJsonFile(filename); + break; + default: + throw new Error('Unrecognized format "' + format + '"'); + } + } finally { + ProtoBuf.convertFieldsToCamelCase = convertFieldsToCamelCaseOriginal; } - return loadObject(builder.ns); + return loadObject(builder.ns, options); }; /** diff --git a/src/node/src/client.js b/src/node/src/client.js index b5247a69ee0..5523d6b9a47 100644 --- a/src/node/src/client.js +++ b/src/node/src/client.js @@ -698,13 +698,15 @@ exports.waitForClientReady = function(client, deadline, callback) { * Creates a constructor for clients for the given service * @param {ProtoBuf.Reflect.Service} service The service to generate a client * for + * @param {Object=} options Options to apply to the client * @return {function(string, Object)} New client constructor */ -exports.makeProtobufClientConstructor = function(service) { - var method_attrs = common.getProtobufServiceAttrs(service, service.name); +exports.makeProtobufClientConstructor = function(service, options) { + var method_attrs = common.getProtobufServiceAttrs(service, service.name, options); var Client = exports.makeClientConstructor( method_attrs, common.fullyQualifiedName(service)); Client.service = service; + Client.service.grpc_options = options; return Client; }; diff --git a/src/node/src/common.js b/src/node/src/common.js index 2e6c01c4d74..3e6609ab107 100644 --- a/src/node/src/common.js +++ b/src/node/src/common.js @@ -44,9 +44,20 @@ var _ = require('lodash'); /** * Get a function that deserializes a specific type of protobuf. * @param {function()} cls The constructor of the message type to deserialize + * @param {bool=} binaryAsBase64 Deserialize bytes fields as base64 instead of binary. + * Defaults to false + * @param {bool=} longsAsStrings Deserialize long values as strings instead of doubles. + * Defaults to true * @return {function(Buffer):cls} The deserialization function */ -exports.deserializeCls = function deserializeCls(cls) { +exports.deserializeCls = function deserializeCls(cls, binaryAsBase64, + longsAsStrings) { + if (binaryAsBase64 === undefined || binaryAsBase64 === null) { + binaryAsBase64 = false; + } + if (longsAsStrings === undefined || longsAsStrings === null) { + longsAsStrings = true; + } /** * Deserialize a buffer to a message object * @param {Buffer} arg_buf The buffer to deserialize @@ -55,7 +66,7 @@ exports.deserializeCls = function deserializeCls(cls) { return function deserialize(arg_buf) { // Convert to a native object with binary fields as Buffers (first argument) // and longs as strings (second argument) - return cls.decode(arg_buf).toRaw(false, true); + return cls.decode(arg_buf).toRaw(binaryAsBase64, longsAsStrings); }; }; @@ -119,19 +130,27 @@ exports.wrapIgnoreNull = function wrapIgnoreNull(func) { /** * Return a map from method names to method attributes for the service. * @param {ProtoBuf.Reflect.Service} service The service to get attributes for + * @param {Object=} options Options to apply to these attributes * @return {Object} The attributes map */ -exports.getProtobufServiceAttrs = function getProtobufServiceAttrs(service) { +exports.getProtobufServiceAttrs = function getProtobufServiceAttrs(service, options) { var prefix = '/' + fullyQualifiedName(service) + '/'; + var binaryAsBase64, longsAsStrings; + if (options) { + binaryAsBase64 = options.binaryAsBase64; + longsAsStrings = options.longsAsStrings; + } return _.object(_.map(service.children, function(method) { return [_.camelCase(method.name), { path: prefix + method.name, requestStream: method.requestStream, responseStream: method.responseStream, requestSerialize: serializeCls(method.resolvedRequestType.build()), - requestDeserialize: deserializeCls(method.resolvedRequestType.build()), + requestDeserialize: deserializeCls(method.resolvedRequestType.build(), + binaryAsBase64, longsAsStrings), responseSerialize: serializeCls(method.resolvedResponseType.build()), - responseDeserialize: deserializeCls(method.resolvedResponseType.build()) + responseDeserialize: deserializeCls(method.resolvedResponseType.build(), + binaryAsBase64, longsAsStrings) }]; })); }; diff --git a/src/node/src/server.js b/src/node/src/server.js index e5aadcd5658..0cf7ba34246 100644 --- a/src/node/src/server.js +++ b/src/node/src/server.js @@ -737,7 +737,12 @@ Server.prototype.addService = function(service, implementation) { * method implementation for the provided service. */ Server.prototype.addProtoService = function(service, implementation) { - this.addService(common.getProtobufServiceAttrs(service), implementation); + var options; + if (service.grpc_options) { + options = service.grpc_options; + } + this.addService(common.getProtobufServiceAttrs(service, options), + implementation); }; /** From 2815f184a3feee59d243a740ace1eb8dbc632c75 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Wed, 17 Feb 2016 13:55:03 -0800 Subject: [PATCH 30/37] Document variable names --- test/cpp/qps/qps-sweep.sh | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/cpp/qps/qps-sweep.sh b/test/cpp/qps/qps-sweep.sh index 89d373279f6..7a357888497 100755 --- a/test/cpp/qps/qps-sweep.sh +++ b/test/cpp/qps/qps-sweep.sh @@ -37,11 +37,26 @@ fi bins=`find . .. ../.. ../../.. -name bins | head -1` +# Print out each command that gets executed set -x +# +# Specify parameters used in some of the tests +# + +# big is the size in bytes of large messages (0 is the size otherwise) big=65536 + +# wide is the number of client channels in multi-channel tests (1 otherwise) wide=64 + +# deep is the number of RPCs outstanding on a channel in non-ping-pong tests +# (the value used is 1 otherwise) deep=100 + +# half is half the count of worker processes, used in the crossbar scenario +# that uses equal clients and servers. The other scenarios use only 1 server +# and either 1 client or N-1 clients as appropriate half=`echo $QPS_WORKERS | awk -F, '{print int(NF/2)}'` for secure in true false; do From 9384dd971abe8754dc47498a10ee2554d8d762dd Mon Sep 17 00:00:00 2001 From: yang-g Date: Wed, 17 Feb 2016 14:25:23 -0800 Subject: [PATCH 31/37] add comments --- src/cpp/common/channel_arguments.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/cpp/common/channel_arguments.cc b/src/cpp/common/channel_arguments.cc index 72e7b2b4cf5..3e581b4a9c9 100644 --- a/src/cpp/common/channel_arguments.cc +++ b/src/cpp/common/channel_arguments.cc @@ -91,6 +91,9 @@ void ChannelArguments::SetCompressionAlgorithm( } // Note: a second call to this will add in front the result of the first call. +// An example is calling this on a copy of ChannelArguments which already has a +// prefix. The user can build up a prefix string by calling this multiple times, +// each with more significant identifier. void ChannelArguments::SetUserAgentPrefix( const grpc::string& user_agent_prefix) { if (user_agent_prefix.empty()) { From 639766023e33235c571497f28e90494ee60840d0 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Wed, 17 Feb 2016 23:43:55 +0100 Subject: [PATCH 32/37] Flagging the missing API entries. --- grpc.def | 1 + include/grpc/grpc_security.h | 3 ++- src/python/grpcio/grpc/_cython/imports.generated.c | 2 ++ src/python/grpcio/grpc/_cython/imports.generated.h | 3 +++ src/ruby/ext/grpc/rb_grpc_imports.generated.c | 2 ++ src/ruby/ext/grpc/rb_grpc_imports.generated.h | 3 +++ 6 files changed, 13 insertions(+), 1 deletion(-) diff --git a/grpc.def b/grpc.def index d37e6879c57..bd0bc85a7c3 100644 --- a/grpc.def +++ b/grpc.def @@ -99,6 +99,7 @@ EXPORTS grpc_auth_context_set_peer_identity_property_name grpc_channel_credentials_release grpc_google_default_credentials_create + grpc_set_ssl_roots_override_callback grpc_ssl_credentials_create grpc_call_credentials_release grpc_composite_channel_credentials_create diff --git a/include/grpc/grpc_security.h b/include/grpc/grpc_security.h index 3de5cae8be2..ef7205ded8b 100644 --- a/include/grpc/grpc_security.h +++ b/include/grpc/grpc_security.h @@ -167,7 +167,8 @@ typedef grpc_ssl_roots_override_result (*grpc_ssl_roots_override_callback)( before any ssl credentials are created to have the desired side effect. If GRPC_DEFAULT_SSL_ROOTS_FILE_PATH environment is set to a valid path, the callback will not be called. */ -void grpc_set_ssl_roots_override_callback(grpc_ssl_roots_override_callback cb); +GRPCAPI void grpc_set_ssl_roots_override_callback( + grpc_ssl_roots_override_callback cb); /* Object that holds a private key / certificate chain pair in PEM format. */ typedef struct { diff --git a/src/python/grpcio/grpc/_cython/imports.generated.c b/src/python/grpcio/grpc/_cython/imports.generated.c index 4aa41dbcd73..4b1860ce8c9 100644 --- a/src/python/grpcio/grpc/_cython/imports.generated.c +++ b/src/python/grpcio/grpc/_cython/imports.generated.c @@ -137,6 +137,7 @@ grpc_auth_context_add_cstring_property_type grpc_auth_context_add_cstring_proper grpc_auth_context_set_peer_identity_property_name_type grpc_auth_context_set_peer_identity_property_name_import; grpc_channel_credentials_release_type grpc_channel_credentials_release_import; grpc_google_default_credentials_create_type grpc_google_default_credentials_create_import; +grpc_set_ssl_roots_override_callback_type grpc_set_ssl_roots_override_callback_import; grpc_ssl_credentials_create_type grpc_ssl_credentials_create_import; grpc_call_credentials_release_type grpc_call_credentials_release_import; grpc_composite_channel_credentials_create_type grpc_composite_channel_credentials_create_import; @@ -401,6 +402,7 @@ void pygrpc_load_imports(HMODULE library) { grpc_auth_context_set_peer_identity_property_name_import = (grpc_auth_context_set_peer_identity_property_name_type) GetProcAddress(library, "grpc_auth_context_set_peer_identity_property_name"); grpc_channel_credentials_release_import = (grpc_channel_credentials_release_type) GetProcAddress(library, "grpc_channel_credentials_release"); grpc_google_default_credentials_create_import = (grpc_google_default_credentials_create_type) GetProcAddress(library, "grpc_google_default_credentials_create"); + grpc_set_ssl_roots_override_callback_import = (grpc_set_ssl_roots_override_callback_type) GetProcAddress(library, "grpc_set_ssl_roots_override_callback"); grpc_ssl_credentials_create_import = (grpc_ssl_credentials_create_type) GetProcAddress(library, "grpc_ssl_credentials_create"); grpc_call_credentials_release_import = (grpc_call_credentials_release_type) GetProcAddress(library, "grpc_call_credentials_release"); grpc_composite_channel_credentials_create_import = (grpc_composite_channel_credentials_create_type) GetProcAddress(library, "grpc_composite_channel_credentials_create"); diff --git a/src/python/grpcio/grpc/_cython/imports.generated.h b/src/python/grpcio/grpc/_cython/imports.generated.h index f5329f33782..ca30742abcb 100644 --- a/src/python/grpcio/grpc/_cython/imports.generated.h +++ b/src/python/grpcio/grpc/_cython/imports.generated.h @@ -361,6 +361,9 @@ extern grpc_channel_credentials_release_type grpc_channel_credentials_release_im typedef grpc_channel_credentials *(*grpc_google_default_credentials_create_type)(void); extern grpc_google_default_credentials_create_type grpc_google_default_credentials_create_import; #define grpc_google_default_credentials_create grpc_google_default_credentials_create_import +typedef void(*grpc_set_ssl_roots_override_callback_type)(grpc_ssl_roots_override_callback cb); +extern grpc_set_ssl_roots_override_callback_type grpc_set_ssl_roots_override_callback_import; +#define grpc_set_ssl_roots_override_callback grpc_set_ssl_roots_override_callback_import typedef grpc_channel_credentials *(*grpc_ssl_credentials_create_type)(const char *pem_root_certs, grpc_ssl_pem_key_cert_pair *pem_key_cert_pair, void *reserved); extern grpc_ssl_credentials_create_type grpc_ssl_credentials_create_import; #define grpc_ssl_credentials_create grpc_ssl_credentials_create_import diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.c b/src/ruby/ext/grpc/rb_grpc_imports.generated.c index d4ddb734c0d..1af34d97fbc 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.c +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.c @@ -137,6 +137,7 @@ grpc_auth_context_add_cstring_property_type grpc_auth_context_add_cstring_proper grpc_auth_context_set_peer_identity_property_name_type grpc_auth_context_set_peer_identity_property_name_import; grpc_channel_credentials_release_type grpc_channel_credentials_release_import; grpc_google_default_credentials_create_type grpc_google_default_credentials_create_import; +grpc_set_ssl_roots_override_callback_type grpc_set_ssl_roots_override_callback_import; grpc_ssl_credentials_create_type grpc_ssl_credentials_create_import; grpc_call_credentials_release_type grpc_call_credentials_release_import; grpc_composite_channel_credentials_create_type grpc_composite_channel_credentials_create_import; @@ -397,6 +398,7 @@ void grpc_rb_load_imports(HMODULE library) { grpc_auth_context_set_peer_identity_property_name_import = (grpc_auth_context_set_peer_identity_property_name_type) GetProcAddress(library, "grpc_auth_context_set_peer_identity_property_name"); grpc_channel_credentials_release_import = (grpc_channel_credentials_release_type) GetProcAddress(library, "grpc_channel_credentials_release"); grpc_google_default_credentials_create_import = (grpc_google_default_credentials_create_type) GetProcAddress(library, "grpc_google_default_credentials_create"); + grpc_set_ssl_roots_override_callback_import = (grpc_set_ssl_roots_override_callback_type) GetProcAddress(library, "grpc_set_ssl_roots_override_callback"); grpc_ssl_credentials_create_import = (grpc_ssl_credentials_create_type) GetProcAddress(library, "grpc_ssl_credentials_create"); grpc_call_credentials_release_import = (grpc_call_credentials_release_type) GetProcAddress(library, "grpc_call_credentials_release"); grpc_composite_channel_credentials_create_import = (grpc_composite_channel_credentials_create_type) GetProcAddress(library, "grpc_composite_channel_credentials_create"); diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.h b/src/ruby/ext/grpc/rb_grpc_imports.generated.h index 618ae5e7fcf..b61c5282b6d 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.h +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.h @@ -361,6 +361,9 @@ extern grpc_channel_credentials_release_type grpc_channel_credentials_release_im typedef grpc_channel_credentials *(*grpc_google_default_credentials_create_type)(void); extern grpc_google_default_credentials_create_type grpc_google_default_credentials_create_import; #define grpc_google_default_credentials_create grpc_google_default_credentials_create_import +typedef void(*grpc_set_ssl_roots_override_callback_type)(grpc_ssl_roots_override_callback cb); +extern grpc_set_ssl_roots_override_callback_type grpc_set_ssl_roots_override_callback_import; +#define grpc_set_ssl_roots_override_callback grpc_set_ssl_roots_override_callback_import typedef grpc_channel_credentials *(*grpc_ssl_credentials_create_type)(const char *pem_root_certs, grpc_ssl_pem_key_cert_pair *pem_key_cert_pair, void *reserved); extern grpc_ssl_credentials_create_type grpc_ssl_credentials_create_import; #define grpc_ssl_credentials_create grpc_ssl_credentials_create_import From 654d2549b752dfd74500f0b5882abaf306cd62c1 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 17 Feb 2016 15:36:28 -0800 Subject: [PATCH 33/37] Add tests and documentation for new options --- src/node/index.js | 10 ++++++- src/node/src/client.js | 3 +- src/node/src/common.js | 11 +++---- src/node/test/common_test.js | 50 ++++++++++++++++++++++++++++++- src/node/test/test_messages.proto | 5 ++++ 5 files changed, 71 insertions(+), 8 deletions(-) diff --git a/src/node/index.js b/src/node/index.js index 4e4d12d9e45..1c197729d77 100644 --- a/src/node/index.js +++ b/src/node/index.js @@ -78,7 +78,15 @@ exports.loadObject = function loadObject(value, options) { var loadObject = exports.loadObject; /** - * Load a gRPC object from a .proto file. + * Load a gRPC object from a .proto file. The options object can provide the + * following options: + * - convertFieldsToCamelCase: Loads this file with that option on protobuf.js + * set as specified. See + * https://github.com/dcodeIO/protobuf.js/wiki/Advanced-options for details + * - binaryAsBase64: deserialize bytes values as base64 strings instead of + * Buffers. Defaults to false + * - longsAsStrings: deserialize long values as strings instead of objects. + * Defaults to true * @param {string|{root: string, file: string}} filename The file to load * @param {string=} format The file format to expect. Must be either 'proto' or * 'json'. Defaults to 'proto' diff --git a/src/node/src/client.js b/src/node/src/client.js index 5523d6b9a47..c02c44730e5 100644 --- a/src/node/src/client.js +++ b/src/node/src/client.js @@ -702,7 +702,8 @@ exports.waitForClientReady = function(client, deadline, callback) { * @return {function(string, Object)} New client constructor */ exports.makeProtobufClientConstructor = function(service, options) { - var method_attrs = common.getProtobufServiceAttrs(service, service.name, options); + var method_attrs = common.getProtobufServiceAttrs(service, service.name, + options); var Client = exports.makeClientConstructor( method_attrs, common.fullyQualifiedName(service)); Client.service = service; diff --git a/src/node/src/common.js b/src/node/src/common.js index 3e6609ab107..e5217608ecd 100644 --- a/src/node/src/common.js +++ b/src/node/src/common.js @@ -44,10 +44,10 @@ var _ = require('lodash'); /** * Get a function that deserializes a specific type of protobuf. * @param {function()} cls The constructor of the message type to deserialize - * @param {bool=} binaryAsBase64 Deserialize bytes fields as base64 instead of binary. - * Defaults to false - * @param {bool=} longsAsStrings Deserialize long values as strings instead of doubles. - * Defaults to true + * @param {bool=} binaryAsBase64 Deserialize bytes fields as base64 strings + * instead of Buffers. Defaults to false + * @param {bool=} longsAsStrings Deserialize long values as strings instead of + * objects. Defaults to true * @return {function(Buffer):cls} The deserialization function */ exports.deserializeCls = function deserializeCls(cls, binaryAsBase64, @@ -133,7 +133,8 @@ exports.wrapIgnoreNull = function wrapIgnoreNull(func) { * @param {Object=} options Options to apply to these attributes * @return {Object} The attributes map */ -exports.getProtobufServiceAttrs = function getProtobufServiceAttrs(service, options) { +exports.getProtobufServiceAttrs = function getProtobufServiceAttrs(service, + options) { var prefix = '/' + fullyQualifiedName(service) + '/'; var binaryAsBase64, longsAsStrings; if (options) { diff --git a/src/node/test/common_test.js b/src/node/test/common_test.js index 08ba429ed7f..c57b7388f67 100644 --- a/src/node/test/common_test.js +++ b/src/node/test/common_test.js @@ -42,7 +42,7 @@ var ProtoBuf = require('protobufjs'); var messages_proto = ProtoBuf.loadProtoFile( __dirname + '/test_messages.proto').build(); -describe('Proto message serialize and deserialize', function() { +describe('Proto message long int serialize and deserialize', function() { var longSerialize = common.serializeCls(messages_proto.LongValues); var longDeserialize = common.deserializeCls(messages_proto.LongValues); var pos_value = '314159265358979'; @@ -87,4 +87,52 @@ describe('Proto message serialize and deserialize', function() { assert.strictEqual(longDeserialize(serialized).sfixed_64.toString(), neg_value); }); + it('should deserialize as a number with the right option set', function() { + var longNumDeserialize = common.deserializeCls(messages_proto.LongValues, + false, false); + var serialized = longSerialize({int_64: pos_value}); + assert.strictEqual(typeof longDeserialize(serialized).int_64, 'string'); + /* With the longsAsStrings option disabled, long values are represented as + * objects with 3 keys: low, high, and unsigned */ + assert.strictEqual(typeof longNumDeserialize(serialized).int_64, 'object'); + }); +}); +describe('Proto message bytes serialize and deserialize', function() { + var sequenceSerialize = common.serializeCls(messages_proto.SequenceValues); + var sequenceDeserialize = common.deserializeCls( + messages_proto.SequenceValues); + var sequenceBase64Deserialize = common.deserializeCls( + messages_proto.SequenceValues, true); + var buffer_val = new Buffer([0x69, 0xb7]); + var base64_val = 'abc='; + it('should preserve a buffer', function() { + var serialized = sequenceSerialize({bytes_field: buffer_val}); + var deserialized = sequenceDeserialize(serialized); + assert.strictEqual(deserialized.bytes_field.compare(buffer_val), 0); + }); + it('should accept base64 encoded strings', function() { + var serialized = sequenceSerialize({bytes_field: base64_val}); + var deserialized = sequenceDeserialize(serialized); + assert.strictEqual(deserialized.bytes_field.compare(buffer_val), 0); + }); + it('should output base64 encoded strings with an option set', function() { + var serialized = sequenceSerialize({bytes_field: base64_val}); + var deserialized = sequenceBase64Deserialize(serialized); + assert.strictEqual(deserialized.bytes_field, base64_val); + }); + /* The next two tests are specific tests to verify that issue + * https://github.com/grpc/grpc/issues/5174 has been fixed. They are skipped + * because they will not pass until a protobuf.js release has been published + * with a fix for https://github.com/dcodeIO/protobuf.js/issues/390 */ + it.skip('should serialize a repeated field as packed by default', function() { + var expected_serialize = new Buffer([0x12, 0x01, 0x01, 0x0a]); + var serialized = sequenceSerialize({repeated_field: [10]}); + assert.strictEqual(expected_serialize.compare(serialized), 0); + }); + it.skip('should deserialize packed or unpacked repeated', function() { + var serialized = new Buffer([0x12, 0x01, 0x01, 0x0a]); + assert.doesNotThrow(function() { + sequenceDeserialize(serialized); + }); + }); }); diff --git a/src/node/test/test_messages.proto b/src/node/test/test_messages.proto index c77a937d3f4..d1ffcf996df 100644 --- a/src/node/test/test_messages.proto +++ b/src/node/test/test_messages.proto @@ -36,3 +36,8 @@ message LongValues { fixed64 fixed_64 = 4; sfixed64 sfixed_64 = 5; } + +message SequenceValues { + bytes bytes_field = 1; + repeated int32 repeated_field = 2; +} \ No newline at end of file From 5acbb9c1e9e93b3fc085cb52e5e6207470bd938a Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 17 Feb 2016 15:42:01 -0800 Subject: [PATCH 34/37] Sanitize files --- src/node/test/common_test.js | 2 +- src/node/test/test_messages.proto | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node/test/common_test.js b/src/node/test/common_test.js index c57b7388f67..66a4205f82c 100644 --- a/src/node/test/common_test.js +++ b/src/node/test/common_test.js @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/src/node/test/test_messages.proto b/src/node/test/test_messages.proto index d1ffcf996df..9b8cb875eeb 100644 --- a/src/node/test/test_messages.proto +++ b/src/node/test/test_messages.proto @@ -40,4 +40,4 @@ message LongValues { message SequenceValues { bytes bytes_field = 1; repeated int32 repeated_field = 2; -} \ No newline at end of file +} From 4105505b334013c638b486bb34f26f7b090b46ef Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Wed, 17 Feb 2016 18:01:18 -0800 Subject: [PATCH 35/37] Fixed refactoring of grpc_arg pointer vtable --- include/grpc++/support/channel_arguments.h | 11 +++++++++++ src/cpp/common/channel_arguments.cc | 13 ++----------- test/cpp/common/channel_arguments_test.cc | 15 ++++++++++----- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/include/grpc++/support/channel_arguments.h b/include/grpc++/support/channel_arguments.h index 4e530d4b89c..a9ede35f903 100644 --- a/include/grpc++/support/channel_arguments.h +++ b/include/grpc++/support/channel_arguments.h @@ -95,6 +95,17 @@ class ChannelArguments { friend class SecureChannelCredentials; friend class testing::ChannelArgumentsTest; + /// Default pointer argument operations. + struct PointerVtableMembers { + static void* Copy(void* in) { return in; } + static void Destroy(void* in) {} + static int Compare(void* a, void* b) { + if (a < b) return -1; + if (a > b) return 1; + return 0; + } + }; + // Returns empty string when it is not set. grpc::string GetSslTargetNameOverride() const; diff --git a/src/cpp/common/channel_arguments.cc b/src/cpp/common/channel_arguments.cc index 989c1058324..d7faa5e173d 100644 --- a/src/cpp/common/channel_arguments.cc +++ b/src/cpp/common/channel_arguments.cc @@ -124,18 +124,9 @@ void ChannelArguments::SetInt(const grpc::string& key, int value) { } void ChannelArguments::SetPointer(const grpc::string& key, void* value) { - struct VtableMembers { - static void* Copy(void* in) { return in; } - static void Destroy(void* in) {} - static int Compare(void* a, void* b) { - if (a < b) return -1; - if (a > b) return 1; - return 0; - } - }; static const grpc_arg_pointer_vtable vtable = { - &VtableMembers::Copy, &VtableMembers::Destroy, &VtableMembers::Compare}; - + &PointerVtableMembers::Copy, &PointerVtableMembers::Destroy, + &PointerVtableMembers::Compare}; grpc_arg arg; arg.type = GRPC_ARG_POINTER; strings_.push_back(key); diff --git a/test/cpp/common/channel_arguments_test.cc b/test/cpp/common/channel_arguments_test.cc index 5ee63bf6885..06e41e29ba7 100644 --- a/test/cpp/common/channel_arguments_test.cc +++ b/test/cpp/common/channel_arguments_test.cc @@ -41,6 +41,11 @@ namespace testing { class ChannelArgumentsTest : public ::testing::Test { protected: + ChannelArgumentsTest() + : pointer_vtable_({&ChannelArguments::PointerVtableMembers::Copy, + &ChannelArguments::PointerVtableMembers::Destroy, + &ChannelArguments::PointerVtableMembers::Compare}) {} + void SetChannelArgs(const ChannelArguments& channel_args, grpc_channel_args* args) { channel_args.SetChannelArgs(args); @@ -74,14 +79,15 @@ class ChannelArgumentsTest : public ::testing::Test { return grpc::string(arg.value.string) == expected_arg.value.string; } else if (arg.type == GRPC_ARG_POINTER) { return arg.value.pointer.p == expected_arg.value.pointer.p && - arg.value.pointer.copy == expected_arg.value.pointer.copy && - arg.value.pointer.destroy == - expected_arg.value.pointer.destroy; + arg.value.pointer.vtable->copy == expected_arg.value.pointer.vtable->copy && + arg.value.pointer.vtable->destroy == + expected_arg.value.pointer.vtable->destroy; } } } return false; } + grpc_arg_pointer_vtable pointer_vtable_; ChannelArguments channel_args_; }; @@ -151,8 +157,7 @@ TEST_F(ChannelArgumentsTest, SetPointer) { arg0.type = GRPC_ARG_POINTER; arg0.key = const_cast(key0.c_str()); arg0.value.pointer.p = &key0; - arg0.value.pointer.copy = nullptr; - arg0.value.pointer.destroy = nullptr; + arg0.value.pointer.vtable = &pointer_vtable_; grpc::string key(key0); channel_args_.SetPointer(key, arg0.value.pointer.p); From 1689990ff391b12f7bf4d6ce68867928d81e24de Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Wed, 17 Feb 2016 18:32:17 -0800 Subject: [PATCH 36/37] clang format --- test/cpp/common/channel_arguments_test.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/cpp/common/channel_arguments_test.cc b/test/cpp/common/channel_arguments_test.cc index 06e41e29ba7..a4821b4d0ba 100644 --- a/test/cpp/common/channel_arguments_test.cc +++ b/test/cpp/common/channel_arguments_test.cc @@ -79,7 +79,8 @@ class ChannelArgumentsTest : public ::testing::Test { return grpc::string(arg.value.string) == expected_arg.value.string; } else if (arg.type == GRPC_ARG_POINTER) { return arg.value.pointer.p == expected_arg.value.pointer.p && - arg.value.pointer.vtable->copy == expected_arg.value.pointer.vtable->copy && + arg.value.pointer.vtable->copy == + expected_arg.value.pointer.vtable->copy && arg.value.pointer.vtable->destroy == expected_arg.value.pointer.vtable->destroy; } From 7d243df88f359263f4c1ca82a472e49199bd52fe Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Thu, 18 Feb 2016 09:58:05 -0800 Subject: [PATCH 37/37] Increase Node's per-test timeout and Ruby's overall test timeout --- tools/run_tests/run_node.bat | 2 +- tools/run_tests/run_node.sh | 9 +++++++-- tools/run_tests/run_tests.py | 4 ++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/tools/run_tests/run_node.bat b/tools/run_tests/run_node.bat index f5cf01f0959..ad9ca14b8b9 100644 --- a/tools/run_tests/run_node.bat +++ b/tools/run_tests/run_node.bat @@ -29,4 +29,4 @@ set JUNIT_REPORT_PATH=src\node\reports.xml set JUNIT_REPORT_STACK=1 -.\node_modules\.bin\mocha.cmd --reporter mocha-jenkins-reporter src\node\test \ No newline at end of file +.\node_modules\.bin\mocha.cmd --reporter mocha-jenkins-reporter --timeout 8000 src\node\test \ No newline at end of file diff --git a/tools/run_tests/run_node.sh b/tools/run_tests/run_node.sh index 40f61d77cc9..178584ae8ed 100755 --- a/tools/run_tests/run_node.sh +++ b/tools/run_tests/run_node.sh @@ -41,10 +41,13 @@ cd $(dirname $0)/../.. root=`pwd` +test_directory='src/node/test' +timeout=8000 + if [ "$CONFIG" = "gcov" ] then ./node_modules/.bin/istanbul cover --dir reports/node_coverage \ - -x **/interop/* ./node_modules/.bin/_mocha -- --timeout 8000 src/node/test + -x **/interop/* ./node_modules/.bin/_mocha -- --timeout $timeout $test_directory cd build gcov Release/obj.target/grpc/ext/*.o lcov --base-directory . --directory . -c -o coverage.info @@ -55,5 +58,7 @@ then echo '' > \ ../reports/node_coverage/index.html else - JUNIT_REPORT_PATH=src/node/reports.xml JUNIT_REPORT_STACK=1 ./node_modules/.bin/mocha --reporter mocha-jenkins-reporter src/node/test + JUNIT_REPORT_PATH=src/node/reports.xml JUNIT_REPORT_STACK=1 \ + ./node_modules/.bin/mocha --timeout $timeout \ + --reporter mocha-jenkins-reporter $test_directory fi diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index de3716bc887..0b3efa29e3a 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -334,13 +334,14 @@ class RubyLanguage(object): def test_specs(self, config, args): return [config.job_spec(['tools/run_tests/run_ruby.sh'], None, + timeout_seconds=10*60, environ=_FORCE_ENVIRON_FOR_WRAPPERS)] def pre_build_steps(self): return [['tools/run_tests/pre_build_ruby.sh']] def make_targets(self, test_regex): - return ['static_c'] + return [] def make_options(self): return [] @@ -1197,4 +1198,3 @@ else: if BuildAndRunError.POST_TEST in errors: exit_code |= 4 sys.exit(exit_code) -