From 5ef31ab9c92429e64a43501a4bd440760cca399b Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 10 Nov 2016 16:27:48 -0800 Subject: Progress towards mdstr elimination --- src/core/ext/client_channel/client_channel.c | 24 ++++++++++++------------ src/core/ext/client_channel/subchannel.c | 2 +- src/core/ext/client_channel/subchannel.h | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) (limited to 'src/core/ext/client_channel') diff --git a/src/core/ext/client_channel/client_channel.c b/src/core/ext/client_channel/client_channel.c index 7954fcfb8b..94ca656b2a 100644 --- a/src/core/ext/client_channel/client_channel.c +++ b/src/core/ext/client_channel/client_channel.c @@ -96,7 +96,7 @@ static void method_parameters_del(grpc_exec_ctx *exec_ctx, void *p) { gpr_free(p); } -static const grpc_mdstr_hash_table_vtable method_parameters_vtable = { +static const grpc_slice_hash_table_vtable method_parameters_vtable = { method_parameters_del, method_parameters_copy, method_parameters_cmp}; static void *method_config_convert_value( @@ -131,7 +131,7 @@ typedef struct client_channel_channel_data { char *lb_policy_name; grpc_lb_policy *lb_policy; /** maps method names to method_parameters structs */ - grpc_mdstr_hash_table *method_params_table; + grpc_slice_hash_table *method_params_table; /** incoming resolver result - set by resolver.next() */ grpc_channel_args *resolver_result; /** a list of closures that are all waiting for config to come in */ @@ -232,7 +232,7 @@ static void on_resolver_result_changed(grpc_exec_ctx *exec_ctx, void *arg, char *lb_policy_name = NULL; grpc_lb_policy *lb_policy = NULL; grpc_lb_policy *old_lb_policy; - grpc_mdstr_hash_table *method_params_table = NULL; + grpc_slice_hash_table *method_params_table = NULL; grpc_connectivity_state state = GRPC_CHANNEL_TRANSIENT_FAILURE; bool exit_idle = false; grpc_error *state_error = GRPC_ERROR_CREATE("No load balancing policy"); @@ -316,7 +316,7 @@ static void on_resolver_result_changed(grpc_exec_ctx *exec_ctx, void *arg, old_lb_policy = chand->lb_policy; chand->lb_policy = lb_policy; if (chand->method_params_table != NULL) { - grpc_mdstr_hash_table_unref(exec_ctx, chand->method_params_table); + grpc_slice_hash_table_unref(exec_ctx, chand->method_params_table); } chand->method_params_table = method_params_table; if (lb_policy != NULL) { @@ -494,7 +494,7 @@ static void cc_destroy_channel_elem(grpc_exec_ctx *exec_ctx, } gpr_free(chand->lb_policy_name); if (chand->method_params_table != NULL) { - grpc_mdstr_hash_table_unref(exec_ctx, chand->method_params_table); + grpc_slice_hash_table_unref(exec_ctx, chand->method_params_table); } grpc_connectivity_state_destroy(exec_ctx, &chand->state_tracker); grpc_pollset_set_destroy(chand->interested_parties); @@ -529,7 +529,7 @@ typedef struct client_channel_call_data { // to avoid this without breaking the grpc_deadline_state abstraction. grpc_deadline_state deadline_state; - grpc_mdstr *path; // Request path. + grpc_slice path; // Request path. gpr_timespec call_start_time; gpr_timespec deadline; wait_for_ready_value wait_for_ready_from_service_config; @@ -926,10 +926,10 @@ static void read_service_config(grpc_exec_ctx *exec_ctx, void *arg, if (error == GRPC_ERROR_NONE) { // Get the method config table from channel data. gpr_mu_lock(&chand->mu); - grpc_mdstr_hash_table *method_params_table = NULL; + grpc_slice_hash_table *method_params_table = NULL; if (chand->method_params_table != NULL) { method_params_table = - grpc_mdstr_hash_table_ref(chand->method_params_table); + grpc_slice_hash_table_ref(chand->method_params_table); } gpr_mu_unlock(&chand->mu); // If the method config table was present, use it. @@ -958,7 +958,7 @@ static void read_service_config(grpc_exec_ctx *exec_ctx, void *arg, gpr_mu_unlock(&calld->mu); } } - grpc_mdstr_hash_table_unref(exec_ctx, method_params_table); + grpc_slice_hash_table_unref(exec_ctx, method_params_table); } } GRPC_CALL_STACK_UNREF(exec_ctx, calld->owning_call, "read_service_config"); @@ -996,8 +996,8 @@ static grpc_error *cc_init_call_elem(grpc_exec_ctx *exec_ctx, if (chand->lb_policy != NULL) { // We already have a resolver result, so check for service config. if (chand->method_params_table != NULL) { - grpc_mdstr_hash_table *method_params_table = - grpc_mdstr_hash_table_ref(chand->method_params_table); + grpc_slice_hash_table *method_params_table = + grpc_slice_hash_table_ref(chand->method_params_table); gpr_mu_unlock(&chand->mu); method_parameters *method_params = grpc_method_config_table_get( exec_ctx, method_params_table, args->path); @@ -1013,7 +1013,7 @@ static grpc_error *cc_init_call_elem(grpc_exec_ctx *exec_ctx, method_params->wait_for_ready; } } - grpc_mdstr_hash_table_unref(exec_ctx, method_params_table); + grpc_slice_hash_table_unref(exec_ctx, method_params_table); } else { gpr_mu_unlock(&chand->mu); } diff --git a/src/core/ext/client_channel/subchannel.c b/src/core/ext/client_channel/subchannel.c index bcb3082267..7b64a76f67 100644 --- a/src/core/ext/client_channel/subchannel.c +++ b/src/core/ext/client_channel/subchannel.c @@ -703,7 +703,7 @@ grpc_connected_subchannel *grpc_subchannel_get_connected_subchannel( grpc_error *grpc_connected_subchannel_create_call( grpc_exec_ctx *exec_ctx, grpc_connected_subchannel *con, - grpc_polling_entity *pollent, grpc_mdstr *path, gpr_timespec deadline, + grpc_polling_entity *pollent, grpc_slice path, gpr_timespec deadline, grpc_subchannel_call **call) { grpc_channel_stack *chanstk = CHANNEL_STACK_FROM_CONNECTION(con); *call = gpr_malloc(sizeof(grpc_subchannel_call) + chanstk->call_stack_size); diff --git a/src/core/ext/client_channel/subchannel.h b/src/core/ext/client_channel/subchannel.h index 93bd72d20d..17208cb853 100644 --- a/src/core/ext/client_channel/subchannel.h +++ b/src/core/ext/client_channel/subchannel.h @@ -111,7 +111,7 @@ void grpc_subchannel_call_unref(grpc_exec_ctx *exec_ctx, /** construct a subchannel call */ grpc_error *grpc_connected_subchannel_create_call( grpc_exec_ctx *exec_ctx, grpc_connected_subchannel *connected_subchannel, - grpc_polling_entity *pollent, grpc_mdstr *path, gpr_timespec deadline, + grpc_polling_entity *pollent, grpc_slice path, gpr_timespec deadline, grpc_subchannel_call **subchannel_call); /** process a transport level op */ -- cgit v1.2.3 From d7f15831c6a4dcd71fe10babde7bb0b380fb1eff Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 16 Nov 2016 17:12:55 -0800 Subject: Tests are starting to compile --- src/core/ext/census/grpc_filter.c | 4 +- src/core/ext/client_channel/client_channel.c | 5 +- src/core/ext/lb_policy/grpclb/grpclb.c | 35 ++++---- .../ext/load_reporting/load_reporting_filter.c | 31 ++++--- .../lib/security/transport/server_auth_filter.c | 28 ++---- src/core/lib/transport/static_metadata.c | 100 +++++++++++---------- src/core/lib/transport/static_metadata.h | 25 +++--- test/core/end2end/fuzzers/hpack.dictionary | 1 + tools/codegen/core/gen_static_metadata.py | 48 ++++++---- tools/codegen/core/perfect/build.sh | 4 + tools/codegen/core/perfect/run.sh | 3 +- 11 files changed, 154 insertions(+), 130 deletions(-) create mode 100755 tools/codegen/core/perfect/build.sh (limited to 'src/core/ext/client_channel') diff --git a/src/core/ext/census/grpc_filter.c b/src/core/ext/census/grpc_filter.c index 397dbc40a8..5008879512 100644 --- a/src/core/ext/census/grpc_filter.c +++ b/src/core/ext/census/grpc_filter.c @@ -67,9 +67,7 @@ static void extract_and_annotate_method_tag(grpc_metadata_batch *md, channel_data *chand) { grpc_linked_mdelem *m; for (m = md->list.head; m != NULL; m = m->next) { - if (m->md->key == GRPC_MDSTR_PATH) { - gpr_log(GPR_DEBUG, "%s", - (const char *)GRPC_SLICE_START_PTR(m->md->value->slice)); + if (grpc_slice_cmp(m->md->key, GRPC_MDSTR_PATH) == 0) { /* Add method tag here */ } } diff --git a/src/core/ext/client_channel/client_channel.c b/src/core/ext/client_channel/client_channel.c index 011a8411aa..5faad33a1d 100644 --- a/src/core/ext/client_channel/client_channel.c +++ b/src/core/ext/client_channel/client_channel.c @@ -51,6 +51,7 @@ #include "src/core/lib/iomgr/iomgr.h" #include "src/core/lib/iomgr/polling_entity.h" #include "src/core/lib/profiling/timers.h" +#include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/support/string.h" #include "src/core/lib/surface/channel.h" #include "src/core/lib/transport/connectivity_state.h" @@ -972,7 +973,7 @@ static grpc_error *cc_init_call_elem(grpc_exec_ctx *exec_ctx, call_data *calld = elem->call_data; // Initialize data members. grpc_deadline_state_init(exec_ctx, elem, args->call_stack); - calld->path = GRPC_MDSTR_REF(args->path); + calld->path = grpc_slice_ref_internal(args->path); calld->call_start_time = args->start_time; calld->deadline = gpr_convert_clock_type(args->deadline, GPR_CLOCK_MONOTONIC); calld->wait_for_ready_from_service_config = WAIT_FOR_READY_UNSET; @@ -1041,7 +1042,7 @@ static void cc_destroy_call_elem(grpc_exec_ctx *exec_ctx, void *and_free_memory) { call_data *calld = elem->call_data; grpc_deadline_state_destroy(exec_ctx, elem); - GRPC_MDSTR_UNREF(exec_ctx, calld->path); + grpc_slice_unref_internal(exec_ctx, calld->path); GRPC_ERROR_UNREF(calld->cancel_error); grpc_subchannel_call *call = GET_CALL(calld); if (call != NULL && call != CANCELLED_CALL) { diff --git a/src/core/ext/lb_policy/grpclb/grpclb.c b/src/core/ext/lb_policy/grpclb/grpclb.c index 3e5c039fdd..1a673e95af 100644 --- a/src/core/ext/lb_policy/grpclb/grpclb.c +++ b/src/core/ext/lb_policy/grpclb/grpclb.c @@ -333,8 +333,7 @@ typedef struct glb_lb_policy { /* call status code and details, set in lb_on_server_status_received() */ grpc_status_code lb_call_status; - char *lb_call_status_details; - size_t lb_call_status_details_capacity; + grpc_slice lb_call_status_details; /** LB call retry backoff state */ gpr_backoff lb_call_backoff_state; @@ -447,10 +446,10 @@ static grpc_lb_addresses *process_serverlist( GPR_ARRAY_SIZE(server->load_balance_token); const size_t lb_token_length = strnlen(server->load_balance_token, lb_token_max_length); - grpc_slice lb_token_mdstr = grpc_mdstr_from_buffer( - (uint8_t *)server->load_balance_token, lb_token_length); - user_data = grpc_mdelem_from_metadata_strings( - exec_ctx, GRPC_MDSTR_LB_TOKEN, lb_token_mdstr); + grpc_slice lb_token_mdstr = grpc_slice_from_copied_buffer( + server->load_balance_token, lb_token_length); + user_data = grpc_mdelem_from_slices(exec_ctx, GRPC_MDSTR_LB_TOKEN, + lb_token_mdstr); } else { gpr_log(GPR_ERROR, "Missing LB token for backend address '%s'. The empty token will " @@ -972,11 +971,12 @@ static void lb_call_init_locked(grpc_exec_ctx *exec_ctx, /* Note the following LB call progresses every time there's activity in \a * glb_policy->base.interested_parties, which is comprised of the polling * entities from \a client_channel. */ + grpc_slice host = grpc_slice_from_copied_string(glb_policy->server_name); glb_policy->lb_call = grpc_channel_create_pollset_set_call( exec_ctx, glb_policy->lb_channel, NULL, GRPC_PROPAGATE_DEFAULTS, glb_policy->base.interested_parties, - "/grpc.lb.v1.LoadBalancer/BalanceLoad", glb_policy->server_name, - glb_policy->deadline, NULL); + GRPC_MDSTR_SLASH_GRPC_DOT_LB_DOT_V1_DOT_LOADBALANCER_SLASH_BALANCELOAD, + &host, glb_policy->deadline, NULL); grpc_metadata_array_init(&glb_policy->lb_initial_metadata_recv); grpc_metadata_array_init(&glb_policy->lb_trailing_metadata_recv); @@ -989,9 +989,6 @@ static void lb_call_init_locked(grpc_exec_ctx *exec_ctx, grpc_slice_unref_internal(exec_ctx, request_payload_slice); grpc_grpclb_request_destroy(request); - glb_policy->lb_call_status_details = NULL; - glb_policy->lb_call_status_details_capacity = 0; - grpc_closure_init(&glb_policy->lb_on_server_status_received, lb_on_server_status_received, glb_policy); grpc_closure_init(&glb_policy->lb_on_response_received, @@ -1002,7 +999,8 @@ static void lb_call_init_locked(grpc_exec_ctx *exec_ctx, BACKOFF_MAX_SECONDS * 1000); } -static void lb_call_destroy_locked(glb_lb_policy *glb_policy) { +static void lb_call_destroy_locked(grpc_exec_ctx *exec_ctx, + glb_lb_policy *glb_policy) { GPR_ASSERT(glb_policy->lb_call != NULL); grpc_call_destroy(glb_policy->lb_call); glb_policy->lb_call = NULL; @@ -1011,7 +1009,7 @@ static void lb_call_destroy_locked(glb_lb_policy *glb_policy) { grpc_metadata_array_destroy(&glb_policy->lb_trailing_metadata_recv); grpc_byte_buffer_destroy(glb_policy->lb_request_payload); - gpr_free(glb_policy->lb_call_status_details); + grpc_slice_unref_internal(exec_ctx, glb_policy->lb_call_status_details); } /* @@ -1060,8 +1058,6 @@ static void query_for_backends_locked(grpc_exec_ctx *exec_ctx, op->data.recv_status_on_client.status = &glb_policy->lb_call_status; op->data.recv_status_on_client.status_details = &glb_policy->lb_call_status_details; - op->data.recv_status_on_client.status_details_capacity = - &glb_policy->lb_call_status_details_capacity; op->flags = 0; op->reserved = NULL; op++; @@ -1201,15 +1197,18 @@ static void lb_on_server_status_received(grpc_exec_ctx *exec_ctx, void *arg, GPR_ASSERT(glb_policy->lb_call != NULL); if (grpc_lb_glb_trace) { + char *status_details = + grpc_dump_slice(glb_policy->lb_call_status_details, GPR_DUMP_ASCII); gpr_log(GPR_DEBUG, "Status from LB server received. Status = %d, Details = '%s', " "(call: %p)", - glb_policy->lb_call_status, glb_policy->lb_call_status_details, + glb_policy->lb_call_status, status_details, (void *)glb_policy->lb_call); + gpr_free(status_details); } - /* We need to performe cleanups no matter what. */ - lb_call_destroy_locked(glb_policy); + /* We need to perform cleanups no matter what. */ + lb_call_destroy_locked(exec_ctx, glb_policy); if (!glb_policy->shutting_down) { /* if we aren't shutting down, restart the LB client call after some time */ diff --git a/src/core/ext/load_reporting/load_reporting_filter.c b/src/core/ext/load_reporting/load_reporting_filter.c index f50e6520cd..c152e3d3b8 100644 --- a/src/core/ext/load_reporting/load_reporting_filter.c +++ b/src/core/ext/load_reporting/load_reporting_filter.c @@ -41,13 +41,17 @@ #include "src/core/ext/load_reporting/load_reporting_filter.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/profiling/timers.h" +#include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/transport/static_metadata.h" typedef struct call_data { intptr_t id; /**< an id unique to the call */ - char *trailing_md_string; - char *initial_md_string; - const char *service_method; + bool have_trailing_md_string; + grpc_slice trailing_md_string; + bool have_initial_md_string; + grpc_slice initial_md_string; + bool have_service_method; + grpc_slice service_method; /* stores the recv_initial_metadata op's ready closure, which we wrap with our * own (on_initial_md_ready) in order to capture the incoming initial metadata @@ -74,10 +78,12 @@ static grpc_mdelem *recv_md_filter(grpc_exec_ctx *exec_ctx, void *user_data, grpc_call_element *elem = a->elem; call_data *calld = elem->call_data; - if (md->key == GRPC_MDSTR_PATH) { - calld->service_method = grpc_mdstr_as_c_string(md->value); - } else if (md->key == GRPC_MDSTR_LB_TOKEN) { - calld->initial_md_string = gpr_strdup(grpc_mdstr_as_c_string(md->value)); + if (grpc_slice_cmp(md->key, GRPC_MDSTR_PATH) == 0) { + calld->service_method = grpc_slice_ref_internal(md->value); + calld->have_service_method = true; + } else if (grpc_slice_cmp(md->key, GRPC_MDSTR_LB_TOKEN) == 0) { + calld->initial_md_string = grpc_slice_ref_internal(md->value); + calld->have_initial_md_string = true; return NULL; } @@ -95,7 +101,7 @@ static void on_initial_md_ready(grpc_exec_ctx *exec_ctx, void *user_data, a.exec_ctx = exec_ctx; grpc_metadata_batch_filter(exec_ctx, calld->recv_initial_metadata, recv_md_filter, &a); - if (calld->service_method == NULL) { + if (!calld->have_service_method) { err = grpc_error_add_child(err, GRPC_ERROR_CREATE("Missing :path header")); } @@ -148,8 +154,8 @@ static void destroy_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, calld->service_method}; */ - gpr_free(calld->initial_md_string); - gpr_free(calld->trailing_md_string); + grpc_slice_unref_internal(exec_ctx, calld->initial_md_string); + grpc_slice_unref_internal(exec_ctx, calld->trailing_md_string); } /* Constructor for channel_data */ @@ -195,8 +201,9 @@ static grpc_mdelem *lr_trailing_md_filter(grpc_exec_ctx *exec_ctx, grpc_call_element *elem = user_data; call_data *calld = elem->call_data; - if (md->key == GRPC_MDSTR_LB_COST_BIN) { - calld->trailing_md_string = gpr_strdup(grpc_mdstr_as_c_string(md->value)); + if (grpc_slice_cmp(md->key, GRPC_MDSTR_LB_COST_BIN) == 0) { + calld->trailing_md_string = grpc_slice_ref_internal(md->value); + calld->have_trailing_md_string = true; return NULL; } diff --git a/src/core/lib/security/transport/server_auth_filter.c b/src/core/lib/security/transport/server_auth_filter.c index db0d0d6907..1824cf37b6 100644 --- a/src/core/lib/security/transport/server_auth_filter.c +++ b/src/core/lib/security/transport/server_auth_filter.c @@ -33,12 +33,13 @@ #include +#include +#include + #include "src/core/lib/security/context/security_context.h" #include "src/core/lib/security/credentials/credentials.h" #include "src/core/lib/security/transport/auth_filters.h" - -#include -#include +#include "src/core/lib/slice/slice_internal.h" typedef struct call_data { grpc_metadata_batch *recv_initial_metadata; @@ -76,9 +77,8 @@ static grpc_metadata_array metadata_batch_to_md_array( gpr_realloc(result.metadata, result.capacity * sizeof(grpc_metadata)); } usr_md = &result.metadata[result.count++]; - usr_md->key = grpc_mdstr_as_c_string(key); - usr_md->value = grpc_mdstr_as_c_string(value); - usr_md->value_length = GRPC_SLICE_LENGTH(value->slice); + usr_md->key = grpc_slice_ref_internal(key); + usr_md->value = grpc_slice_ref_internal(value); } return result; } @@ -90,19 +90,9 @@ static grpc_mdelem *remove_consumed_md(grpc_exec_ctx *exec_ctx, void *user_data, size_t i; for (i = 0; i < calld->num_consumed_md; i++) { const grpc_metadata *consumed_md = &calld->consumed_md[i]; - /* Maybe we could do a pointer comparison but we do not have any guarantee - that the metadata processor used the same pointers for consumed_md in the - callback. */ - if (GRPC_SLICE_LENGTH(md->key->slice) != strlen(consumed_md->key) || - GRPC_SLICE_LENGTH(md->value->slice) != consumed_md->value_length) { - continue; - } - if (memcmp(GRPC_SLICE_START_PTR(md->key->slice), consumed_md->key, - GRPC_SLICE_LENGTH(md->key->slice)) == 0 && - memcmp(GRPC_SLICE_START_PTR(md->value->slice), consumed_md->value, - GRPC_SLICE_LENGTH(md->value->slice)) == 0) { - return NULL; /* Delete. */ - } + if (grpc_slice_cmp(md->key, consumed_md->key) == 0 && + grpc_slice_cmp(md->value, consumed_md->value) == 0) + return NULL; } return md; } diff --git a/src/core/lib/transport/static_metadata.c b/src/core/lib/transport/static_metadata.c index e24f3fa55e..0eb5cd5d74 100644 --- a/src/core/lib/transport/static_metadata.c +++ b/src/core/lib/transport/static_metadata.c @@ -101,14 +101,16 @@ static uint8_t g_raw_bytes[] = { 110, 103, 101, 114, 101, 102, 101, 114, 101, 114, 114, 101, 102, 114, 101, 115, 104, 114, 101, 116, 114, 121, 45, 97, 102, 116, 101, 114, 58, 115, 99, 104, 101, 109, 101, 115, 101, 114, 118, 101, 114, 115, 101, 116, 45, - 99, 111, 111, 107, 105, 101, 47, 47, 105, 110, 100, 101, 120, 46, 104, - 116, 109, 108, 58, 115, 116, 97, 116, 117, 115, 115, 116, 114, 105, 99, - 116, 45, 116, 114, 97, 110, 115, 112, 111, 114, 116, 45, 115, 101, 99, - 117, 114, 105, 116, 121, 116, 101, 116, 114, 97, 105, 108, 101, 114, 115, - 116, 114, 97, 110, 115, 102, 101, 114, 45, 101, 110, 99, 111, 100, 105, - 110, 103, 117, 115, 101, 114, 45, 97, 103, 101, 110, 116, 118, 97, 114, - 121, 118, 105, 97, 119, 119, 119, 45, 97, 117, 116, 104, 101, 110, 116, - 105, 99, 97, 116, 101}; + 99, 111, 111, 107, 105, 101, 47, 47, 103, 114, 112, 99, 46, 108, 98, + 46, 118, 49, 46, 76, 111, 97, 100, 66, 97, 108, 97, 110, 99, 101, + 114, 47, 66, 97, 108, 97, 110, 99, 101, 76, 111, 97, 100, 47, 105, + 110, 100, 101, 120, 46, 104, 116, 109, 108, 58, 115, 116, 97, 116, 117, + 115, 115, 116, 114, 105, 99, 116, 45, 116, 114, 97, 110, 115, 112, 111, + 114, 116, 45, 115, 101, 99, 117, 114, 105, 116, 121, 116, 101, 116, 114, + 97, 105, 108, 101, 114, 115, 116, 114, 97, 110, 115, 102, 101, 114, 45, + 101, 110, 99, 111, 100, 105, 110, 103, 117, 115, 101, 114, 45, 97, 103, + 101, 110, 116, 118, 97, 114, 121, 118, 105, 97, 119, 119, 119, 45, 97, + 117, 116, 104, 101, 110, 116, 105, 99, 97, 116, 101}; static void static_ref(void *unused) {} static void static_unref(grpc_exec_ctx *exec_ctx, void *unused) {} @@ -116,7 +118,7 @@ static grpc_slice_refcount g_refcnt = {static_ref, static_unref}; bool grpc_is_static_metadata_string(grpc_slice slice) { return slice.refcount != NULL && slice.refcount->ref == static_ref; -}; +} const grpc_slice grpc_static_slice_table[GRPC_STATIC_MDSTR_COUNT] = { {.refcount = &g_refcnt, @@ -294,25 +296,27 @@ const grpc_slice grpc_static_slice_table[GRPC_STATIC_MDSTR_COUNT] = { {.refcount = &g_refcnt, .data.refcounted = {.bytes = g_raw_bytes + 891, .length = 1}}, {.refcount = &g_refcnt, - .data.refcounted = {.bytes = g_raw_bytes + 892, .length = 11}}, + .data.refcounted = {.bytes = g_raw_bytes + 892, .length = 36}}, + {.refcount = &g_refcnt, + .data.refcounted = {.bytes = g_raw_bytes + 928, .length = 11}}, {.refcount = &g_refcnt, - .data.refcounted = {.bytes = g_raw_bytes + 903, .length = 7}}, + .data.refcounted = {.bytes = g_raw_bytes + 939, .length = 7}}, {.refcount = &g_refcnt, - .data.refcounted = {.bytes = g_raw_bytes + 910, .length = 25}}, + .data.refcounted = {.bytes = g_raw_bytes + 946, .length = 25}}, {.refcount = &g_refcnt, - .data.refcounted = {.bytes = g_raw_bytes + 935, .length = 2}}, + .data.refcounted = {.bytes = g_raw_bytes + 971, .length = 2}}, {.refcount = &g_refcnt, - .data.refcounted = {.bytes = g_raw_bytes + 937, .length = 8}}, + .data.refcounted = {.bytes = g_raw_bytes + 973, .length = 8}}, {.refcount = &g_refcnt, - .data.refcounted = {.bytes = g_raw_bytes + 945, .length = 17}}, + .data.refcounted = {.bytes = g_raw_bytes + 981, .length = 17}}, {.refcount = &g_refcnt, - .data.refcounted = {.bytes = g_raw_bytes + 962, .length = 10}}, + .data.refcounted = {.bytes = g_raw_bytes + 998, .length = 10}}, {.refcount = &g_refcnt, - .data.refcounted = {.bytes = g_raw_bytes + 972, .length = 4}}, + .data.refcounted = {.bytes = g_raw_bytes + 1008, .length = 4}}, {.refcount = &g_refcnt, - .data.refcounted = {.bytes = g_raw_bytes + 976, .length = 3}}, + .data.refcounted = {.bytes = g_raw_bytes + 1012, .length = 3}}, {.refcount = &g_refcnt, - .data.refcounted = {.bytes = g_raw_bytes + 979, .length = 16}}, + .data.refcounted = {.bytes = g_raw_bytes + 1015, .length = 16}}, }; static const uint8_t g_revmap[] = { @@ -376,13 +380,15 @@ static const uint8_t g_revmap[] = { 255, 255, 82, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 83, 255, 255, 255, 255, 255, 255, 84, 255, 255, 255, 255, 255, 85, 255, 255, 255, 255, 255, 255, 255, 255, 255, 86, 87, 255, 255, 255, 255, 255, 255, 255, - 255, 255, 255, 88, 255, 255, 255, 255, 255, 255, 89, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, - 255, 255, 255, 255, 255, 90, 255, 91, 255, 255, 255, 255, 255, 255, 255, - 92, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, - 255, 255, 93, 255, 255, 255, 255, 255, 255, 255, 255, 255, 94, 255, 255, - 255, 95, 255, 255, 96, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, - 255, 255, 255, 255, 255}; + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 88, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 89, 255, 255, 255, 255, 255, + 255, 90, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 91, 255, 92, 255, + 255, 255, 255, 255, 255, 255, 93, 255, 255, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 94, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 95, 255, 255, 255, 96, 255, 255, 97, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255}; int grpc_static_metadata_index(grpc_slice slice) { if (GRPC_SLICE_LENGTH(slice) == 0) return 33; @@ -390,7 +396,7 @@ int grpc_static_metadata_index(grpc_slice slice) { if (ofs > sizeof(g_revmap)) return -1; uint8_t id = g_revmap[ofs]; return id == 255 ? -1 : id; -}; +} grpc_mdelem grpc_static_mdelem_table[GRPC_STATIC_MDELEM_COUNT]; uintptr_t grpc_static_mdelem_user_data[GRPC_STATIC_MDELEM_COUNT] = { @@ -405,14 +411,14 @@ uintptr_t grpc_static_mdelem_user_data[GRPC_STATIC_MDELEM_COUNT] = { #define ELEMS_PHASHSALT 0x9e3779b9 static const uint8_t elems_tab[] = { - 47, 1, 61, 0, 32, 0, 47, 1, 37, 0, 0, 0, 47, 61, 76, 0, - 76, 0, 61, 0, 32, 37, 51, 0, 47, 47, 79, 4, 76, 1, 0, 0, - 0, 76, 0, 47, 85, 34, 0, 10, 0, 28, 0, 76, 0, 61, 0, 0, - 46, 4, 12, 47, 88, 28, 61, 79, 28, 70, 0, 68, 85, 0, 87, 0, + 0, 17, 61, 28, 4, 12, 47, 0, 0, 0, 61, 0, 47, 0, 61, 76, + 61, 70, 76, 0, 0, 10, 4, 60, 0, 0, 0, 16, 88, 47, 1, 76, + 76, 0, 76, 0, 61, 0, 23, 0, 0, 51, 1, 92, 32, 0, 25, 0, + 34, 0, 37, 0, 76, 76, 32, 38, 70, 79, 81, 0, 64, 0, 0, 0, }; static uint32_t elems_phash(uint32_t val) { - val -= 1003; + val -= 917; uint32_t a, b, rsl; @@ -423,27 +429,27 @@ static uint32_t elems_phash(uint32_t val) { } static const uint16_t elem_keys[] = { - 1218, 8544, 2652, 1973, 7264, 2458, 2734, 3933, 1682, 6435, 3912, 3941, - 4396, 4418, 4850, 4852, 7890, 8541, 6726, 9345, 6338, 6629, 6920, 3939, - 7156, 8540, 8108, 8090, 8181, 8666, 8821, 1876, 8545, 1391, 8957, 1488, - 7405, 7265, 3331, 2943, 2846, 6241, 4851, 2167, 5368, 1585, 1294, 1003, - 9054, 6144, 8542, 8539, 8107, 7793, 7502, 7159, 7696, 2264, 6532, 2749, - 9248, 1197, 7987, 9151, 7017, 4423, 7119, 6823, 3938, 8543, 3525, 3911, - 2070, 2361, 2555, 6047, 1100, 3940, 3622, 3428, 8278, 0, 0, 0, + 2091, 1405, 8728, 2777, 7192, 2287, 2581, 2483, 2973, 4441, 3561, 3951, + 6403, 4463, 9441, 8726, 2875, 5423, 8730, 7338, 6109, 6207, 6697, 6893, + 7229, 8363, 8729, 3952, 8173, 8191, 8725, 8853, 9245, 9343, 1601, 8727, + 7481, 7340, 7971, 7775, 6501, 3973, 3659, 3979, 3463, 3980, 1307, 8190, + 9010, 8731, 4901, 6599, 3365, 7579, 6795, 9147, 9539, 8069, 6305, 7873, + 1209, 1111, 1699, 1503, 7089, 4468, 2189, 4900, 7232, 2385, 6991, 3978, + 1993, 4902, 2679, 2762, 1013, 3981, 1230, 1895, 8265, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; static const uint8_t elem_idxs[] = { - 3, 72, 17, 10, 54, 15, 18, 28, 8, 44, 27, 32, 33, 34, 36, 38, 60, - 69, 47, 80, 43, 46, 49, 30, 52, 68, 64, 62, 65, 74, 75, 9, 73, 5, - 76, 6, 56, 55, 22, 21, 20, 42, 37, 12, 39, 7, 4, 1, 77, 41, 70, - 67, 63, 59, 57, 53, 58, 13, 45, 19, 79, 2, 61, 78, 50, 35, 51, 48, - 29, 71, 24, 26, 11, 14, 16, 40, 0, 31, 25, 23, 66}; + 11, 5, 70, 19, 51, 13, 16, 15, 21, 33, 24, 26, 43, 34, 79, 68, 20, + 39, 72, 54, 40, 41, 46, 48, 52, 66, 71, 27, 62, 64, 67, 74, 77, 78, + 7, 69, 56, 55, 60, 58, 44, 28, 25, 30, 23, 31, 4, 63, 75, 73, 37, + 45, 22, 57, 47, 76, 80, 61, 42, 59, 2, 0, 8, 6, 50, 35, 12, 36, + 53, 14, 49, 29, 10, 38, 17, 18, 1, 32, 3, 9, 65}; grpc_mdelem *grpc_static_mdelem_for_static_strings(int a, int b) { if (a == -1 || b == -1) return NULL; - uint32_t k = (uint32_t)(a * 97 + b); + uint32_t k = (uint32_t)(a * 98 + b); uint32_t h = elems_phash(k); return elem_keys[h] == k ? &grpc_static_mdelem_table[elem_idxs[h]] : NULL; } @@ -455,9 +461,9 @@ const uint8_t grpc_static_metadata_elem_indices[GRPC_STATIC_MDELEM_COUNT * 2] = 40, 32, 40, 53, 40, 58, 40, 59, 40, 60, 40, 61, 45, 31, 45, 53, 45, 58, 50, 0, 50, 1, 50, 2, 55, 33, 62, 33, 63, 33, 64, 33, 65, 33, 66, 33, 67, 33, 68, 33, 69, 33, 70, 33, 71, 33, 72, 33, 73, 38, 73, 75, 73, 78, - 74, 86, 74, 87, 76, 33, 77, 33, 79, 33, 80, 33, 81, 33, 82, 33, 83, 39, - 83, 56, 83, 57, 84, 33, 85, 33, 88, 3, 88, 4, 88, 5, 88, 6, 88, 7, - 88, 8, 88, 9, 89, 33, 90, 91, 92, 33, 93, 33, 94, 33, 95, 33, 96, 33}; + 74, 86, 74, 88, 76, 33, 77, 33, 79, 33, 80, 33, 81, 33, 82, 33, 83, 39, + 83, 56, 83, 57, 84, 33, 85, 33, 89, 3, 89, 4, 89, 5, 89, 6, 89, 7, + 89, 8, 89, 9, 90, 33, 91, 92, 93, 33, 94, 33, 95, 33, 96, 33, 97, 33}; const uint8_t grpc_static_accept_encoding_metadata[8] = {0, 29, 26, 30, 28, 32, 27, 31}; diff --git a/src/core/lib/transport/static_metadata.h b/src/core/lib/transport/static_metadata.h index c6348a5280..282a3231ca 100644 --- a/src/core/lib/transport/static_metadata.h +++ b/src/core/lib/transport/static_metadata.h @@ -44,7 +44,7 @@ #include "src/core/lib/transport/metadata.h" -#define GRPC_STATIC_MDSTR_COUNT 97 +#define GRPC_STATIC_MDSTR_COUNT 98 extern const grpc_slice grpc_static_slice_table[GRPC_STATIC_MDSTR_COUNT]; /* "0" */ #define GRPC_MDSTR_0 (grpc_static_slice_table[0]) @@ -223,26 +223,29 @@ extern const grpc_slice grpc_static_slice_table[GRPC_STATIC_MDSTR_COUNT]; #define GRPC_MDSTR_SET_COOKIE (grpc_static_slice_table[85]) /* "/" */ #define GRPC_MDSTR_SLASH (grpc_static_slice_table[86]) +/* "/grpc.lb.v1.LoadBalancer/BalanceLoad" */ +#define GRPC_MDSTR_SLASH_GRPC_DOT_LB_DOT_V1_DOT_LOADBALANCER_SLASH_BALANCELOAD \ + (grpc_static_slice_table[87]) /* "/index.html" */ -#define GRPC_MDSTR_SLASH_INDEX_DOT_HTML (grpc_static_slice_table[87]) +#define GRPC_MDSTR_SLASH_INDEX_DOT_HTML (grpc_static_slice_table[88]) /* ":status" */ -#define GRPC_MDSTR_STATUS (grpc_static_slice_table[88]) +#define GRPC_MDSTR_STATUS (grpc_static_slice_table[89]) /* "strict-transport-security" */ -#define GRPC_MDSTR_STRICT_TRANSPORT_SECURITY (grpc_static_slice_table[89]) +#define GRPC_MDSTR_STRICT_TRANSPORT_SECURITY (grpc_static_slice_table[90]) /* "te" */ -#define GRPC_MDSTR_TE (grpc_static_slice_table[90]) +#define GRPC_MDSTR_TE (grpc_static_slice_table[91]) /* "trailers" */ -#define GRPC_MDSTR_TRAILERS (grpc_static_slice_table[91]) +#define GRPC_MDSTR_TRAILERS (grpc_static_slice_table[92]) /* "transfer-encoding" */ -#define GRPC_MDSTR_TRANSFER_ENCODING (grpc_static_slice_table[92]) +#define GRPC_MDSTR_TRANSFER_ENCODING (grpc_static_slice_table[93]) /* "user-agent" */ -#define GRPC_MDSTR_USER_AGENT (grpc_static_slice_table[93]) +#define GRPC_MDSTR_USER_AGENT (grpc_static_slice_table[94]) /* "vary" */ -#define GRPC_MDSTR_VARY (grpc_static_slice_table[94]) +#define GRPC_MDSTR_VARY (grpc_static_slice_table[95]) /* "via" */ -#define GRPC_MDSTR_VIA (grpc_static_slice_table[95]) +#define GRPC_MDSTR_VIA (grpc_static_slice_table[96]) /* "www-authenticate" */ -#define GRPC_MDSTR_WWW_AUTHENTICATE (grpc_static_slice_table[96]) +#define GRPC_MDSTR_WWW_AUTHENTICATE (grpc_static_slice_table[97]) bool grpc_is_static_metadata_string(grpc_slice slice); diff --git a/test/core/end2end/fuzzers/hpack.dictionary b/test/core/end2end/fuzzers/hpack.dictionary index 7563482609..d5ee01bfbf 100644 --- a/test/core/end2end/fuzzers/hpack.dictionary +++ b/test/core/end2end/fuzzers/hpack.dictionary @@ -86,6 +86,7 @@ "\x06server" "\x0Aset-cookie" "\x01/" +"$/grpc.lb.v1.LoadBalancer/BalanceLoad" "\x0B/index.html" "\x07:status" "\x19strict-transport-security" diff --git a/tools/codegen/core/gen_static_metadata.py b/tools/codegen/core/gen_static_metadata.py index a9001a6897..d632c97113 100755 --- a/tools/codegen/core/gen_static_metadata.py +++ b/tools/codegen/core/gen_static_metadata.py @@ -63,6 +63,8 @@ CONFIG = [ 'grpc.timeout', 'grpc.max_request_message_bytes', 'grpc.max_response_message_bytes', + # well known method names + '/grpc.lb.v1.LoadBalancer/BalanceLoad', # metadata elements ('grpc-status', '0'), ('grpc-status', '1'), @@ -191,24 +193,30 @@ def put_banner(files, banner): print >>f # build a list of all the strings we need -all_strs = set() -all_elems = set() +all_strs = list() +all_elems = list() static_userdata = {} for elem in CONFIG: if isinstance(elem, tuple): - all_strs.add(elem[0]) - all_strs.add(elem[1]) - all_elems.add(elem) + if elem[0] not in all_strs: + all_strs.append(elem[0]) + if elem[1] not in all_strs: + all_strs.append(elem[1]) + if elem not in all_elems: + all_elems.append(elem) else: - all_strs.add(elem) + if elem not in all_strs: + all_strs.append(elem) compression_elems = [] for mask in range(1, 1<>C, 'static grpc_slice_refcount g_refcnt = {static_ref, static_unref};' print >>C print >>C, 'bool grpc_is_static_metadata_string(grpc_slice slice) {' print >>C, ' return slice.refcount != NULL && slice.refcount->ref == static_ref;' -print >>C, '};' +print >>C, '}' print >>C print >>C, 'const grpc_slice grpc_static_slice_table[GRPC_STATIC_MDSTR_COUNT] = {' str_ofs = 0 @@ -327,7 +335,7 @@ print >>C, ' size_t ofs = (size_t)(GRPC_SLICE_START_PTR(slice) - g_raw_bytes);' print >>C, ' if (ofs > sizeof(g_revmap)) return -1;' print >>C, ' uint8_t id = g_revmap[ofs];' print >>C, ' return id == 255 ? -1 : id;' -print >>C, '};' +print >>C, '}' print >>C print >>D, '# hpack fuzzing dictionary' @@ -361,11 +369,19 @@ def md_idx(m): return i def perfect_hash(keys, name): - tmp = open('/tmp/keys.txt', 'w') - tmp.write(''.join('%d\n' % (x - min(keys)) for x in keys)) - tmp.close() - cmd = '%s/perfect/run.sh %s -ds' % (os.path.dirname(sys.argv[0]), tmp.name) + ok = False + cmd = '%s/perfect/build.sh' % (os.path.dirname(sys.argv[0])) subprocess.check_call(cmd, shell=True) + for offset in reversed(range(0, min(keys))): + tmp = open('/tmp/keys.txt', 'w') + tmp.write(''.join('%d\n' % (x - offset) for x in keys)) + tmp.close() + cmd = '%s/perfect/run.sh %s -dms' % (os.path.dirname(sys.argv[0]), tmp.name) + out = subprocess.check_output(cmd, shell=True) + if 'fatal error' not in out: + ok = True + break + assert ok, "Failed to find hash of keys in /tmp/keys.txt" code = '' @@ -378,14 +394,14 @@ def perfect_hash(keys, name): results[var] = val code += '\n' pycode = 'def f(val):\n' - pycode += ' val -= %d\n' % min(keys) + pycode += ' val -= %d\n' % offset with open('%s/perfect/phash.c' % os.path.dirname(sys.argv[0])) as f: txt = f.read() tabdata = re.search(r'ub1 tab\[\] = \{([^}]+)\}', txt, re.MULTILINE).group(1) code += 'static const uint8_t %s_tab[] = {%s};\n\n' % (name, tabdata) func_body = re.search(r'ub4 phash\(val\)\nub4 val;\n\{([^}]+)\}', txt, re.MULTILINE).group(1).replace('ub4', 'uint32_t') code += 'static uint32_t %s_phash(uint32_t val) {\nval -= %d;\n%s}\n' % (name, - min(keys), func_body.replace('tab', '%s_tab' % name)) + offset, func_body.replace('tab', '%s_tab' % name)) pycode += ' tab=(%s)' % tabdata.replace('\n', '') pycode += '\n'.join(' %s' % s.strip() for s in func_body.splitlines()[2:]) g = {} diff --git a/tools/codegen/core/perfect/build.sh b/tools/codegen/core/perfect/build.sh new file mode 100755 index 0000000000..139556ea48 --- /dev/null +++ b/tools/codegen/core/perfect/build.sh @@ -0,0 +1,4 @@ +#!/bin/bash +set -e +cd $(dirname $0) +gcc -o perfect perfect.c recycle.c lookupa.c perfhex.c 2> compile.txt diff --git a/tools/codegen/core/perfect/run.sh b/tools/codegen/core/perfect/run.sh index 8dc5911cbd..c0d1fc3b81 100755 --- a/tools/codegen/core/perfect/run.sh +++ b/tools/codegen/core/perfect/run.sh @@ -1,7 +1,6 @@ #!/bin/bash set -e cd $(dirname $0) -gcc -o perfect perfect.c recycle.c lookupa.c perfhex.c 2> compile.txt fn=$1 shift -./perfect $* < $fn &> hash.txt +./perfect $* < $fn -- cgit v1.2.3 From bedb18959b9320929d25b230b592b76b96355e03 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 14 Dec 2016 08:06:36 -0800 Subject: Fixes, remove grpc_error_free_string --- src/core/ext/client_channel/subchannel.c | 4 ++-- src/core/ext/resolver/dns/native/dns_resolver.c | 2 +- .../ext/transport/chttp2/server/chttp2_server.c | 4 ++-- .../chttp2/server/insecure/server_chttp2.c | 2 +- .../chttp2/server/secure/server_secure_chttp2.c | 2 +- .../transport/chttp2/transport/chttp2_transport.c | 16 +++++++++---- src/core/ext/transport/chttp2/transport/parsing.c | 2 +- src/core/lib/http/httpcli_security_connector.c | 2 +- src/core/lib/iomgr/error.c | 28 +++++++++++++--------- src/core/lib/iomgr/error.h | 1 - src/core/lib/iomgr/error_internal.h | 1 + src/core/lib/iomgr/tcp_client_posix.c | 4 ++-- src/core/lib/iomgr/tcp_posix.c | 6 ++--- src/core/lib/iomgr/tcp_server_windows.c | 2 +- src/core/lib/iomgr/tcp_uv.c | 2 +- .../lib/security/transport/security_handshaker.c | 2 +- src/core/lib/surface/call.c | 9 ++++--- src/core/lib/surface/completion_queue.c | 8 +++---- src/core/lib/surface/server.c | 2 +- src/core/lib/transport/connectivity_state.c | 2 +- src/core/lib/transport/transport_op_string.c | 6 ++--- test/core/end2end/fixtures/http_proxy.c | 2 +- test/core/util/port_server_client.c | 2 +- 23 files changed, 61 insertions(+), 50 deletions(-) (limited to 'src/core/ext/client_channel') diff --git a/src/core/ext/client_channel/subchannel.c b/src/core/ext/client_channel/subchannel.c index 54764ea7f8..73ae1b859d 100644 --- a/src/core/ext/client_channel/subchannel.c +++ b/src/core/ext/client_channel/subchannel.c @@ -687,7 +687,7 @@ static void subchannel_connected(grpc_exec_ctx *exec_ctx, void *arg, const char *errmsg = grpc_error_string(error); gpr_log(GPR_INFO, "Connect failed: %s", errmsg); - grpc_error_free_string(errmsg); + maybe_start_connecting_locked(exec_ctx, c); GRPC_SUBCHANNEL_WEAK_UNREF(exec_ctx, c, "connecting"); @@ -758,7 +758,7 @@ grpc_error *grpc_connected_subchannel_create_call( if (error != GRPC_ERROR_NONE) { const char *error_string = grpc_error_string(error); gpr_log(GPR_ERROR, "error: %s", error_string); - grpc_error_free_string(error_string); + gpr_free(*call); return error; } diff --git a/src/core/ext/resolver/dns/native/dns_resolver.c b/src/core/ext/resolver/dns/native/dns_resolver.c index bb2b012507..297700143e 100644 --- a/src/core/ext/resolver/dns/native/dns_resolver.c +++ b/src/core/ext/resolver/dns/native/dns_resolver.c @@ -190,7 +190,7 @@ static void dns_on_resolved(grpc_exec_ctx *exec_ctx, void *arg, gpr_timespec timeout = gpr_time_sub(next_try, now); const char *msg = grpc_error_string(error); gpr_log(GPR_DEBUG, "dns resolution failed: %s", msg); - grpc_error_free_string(msg); + GPR_ASSERT(!r->have_retry_timer); r->have_retry_timer = true; GRPC_RESOLVER_REF(&r->base, "retry-timer"); diff --git a/src/core/ext/transport/chttp2/server/chttp2_server.c b/src/core/ext/transport/chttp2/server/chttp2_server.c index 86f5a198c2..fb02960160 100644 --- a/src/core/ext/transport/chttp2/server/chttp2_server.c +++ b/src/core/ext/transport/chttp2/server/chttp2_server.c @@ -139,7 +139,7 @@ static void on_handshake_done(grpc_exec_ctx *exec_ctx, void *arg, if (error != GRPC_ERROR_NONE || connection_state->server_state->shutdown) { const char *error_str = grpc_error_string(error); gpr_log(GPR_ERROR, "Handshaking failed: %s", error_str); - grpc_error_free_string(error_str); + if (error == GRPC_ERROR_NONE && args->endpoint != NULL) { // We were shut down after handshaking completed successfully, so // destroy the endpoint here. @@ -328,7 +328,7 @@ grpc_error *grpc_chttp2_server_add_port( const char *warning_message = grpc_error_string(err); gpr_log(GPR_INFO, "WARNING: %s", warning_message); - grpc_error_free_string(warning_message); + /* we managed to bind some addresses: continue */ } grpc_resolved_addresses_destroy(resolved); diff --git a/src/core/ext/transport/chttp2/server/insecure/server_chttp2.c b/src/core/ext/transport/chttp2/server/insecure/server_chttp2.c index 7e286d4e46..996681842b 100644 --- a/src/core/ext/transport/chttp2/server/insecure/server_chttp2.c +++ b/src/core/ext/transport/chttp2/server/insecure/server_chttp2.c @@ -52,7 +52,7 @@ int grpc_server_add_insecure_http2_port(grpc_server *server, const char *addr) { if (err != GRPC_ERROR_NONE) { const char *msg = grpc_error_string(err); gpr_log(GPR_ERROR, "%s", msg); - grpc_error_free_string(msg); + GRPC_ERROR_UNREF(err); } grpc_exec_ctx_finish(&exec_ctx); diff --git a/src/core/ext/transport/chttp2/server/secure/server_secure_chttp2.c b/src/core/ext/transport/chttp2/server/secure/server_secure_chttp2.c index b5e4996b16..2469b9531f 100644 --- a/src/core/ext/transport/chttp2/server/secure/server_secure_chttp2.c +++ b/src/core/ext/transport/chttp2/server/secure/server_secure_chttp2.c @@ -123,7 +123,7 @@ done: if (err != GRPC_ERROR_NONE) { const char *msg = grpc_error_string(err); gpr_log(GPR_ERROR, "%s", msg); - grpc_error_free_string(msg); + GRPC_ERROR_UNREF(err); } return port_num; diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index 8bfacfac67..6ca00032d1 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -861,7 +861,6 @@ void grpc_chttp2_complete_closure_step(grpc_exec_ctx *exec_ctx, (int)(closure->next_data.scratch / CLOSURE_BARRIER_FIRST_REF_BIT), (int)(closure->next_data.scratch % CLOSURE_BARRIER_FIRST_REF_BIT), desc, errstr); - grpc_error_free_string(errstr); } if (error != GRPC_ERROR_NONE) { if (closure->error_data.error == GRPC_ERROR_NONE) { @@ -1452,7 +1451,6 @@ void grpc_chttp2_cancel_stream(grpc_exec_ctx *exec_ctx, } if (due_to_error != GRPC_ERROR_NONE && !s->seen_error) { s->seen_error = true; - grpc_chttp2_maybe_complete_recv_trailing_metadata(exec_ctx, t, s); } grpc_chttp2_mark_stream_closed(exec_ctx, t, s, 1, 1, due_to_error); } @@ -1559,6 +1557,8 @@ void grpc_chttp2_mark_stream_closed(grpc_exec_ctx *exec_ctx, GRPC_ERROR_UNREF(error); return; } + bool closed_read = false; + bool became_closed = false; if (close_reads && !s->read_closed) { s->read_closed_error = GRPC_ERROR_REF(error); s->read_closed = true; @@ -1567,9 +1567,7 @@ void grpc_chttp2_mark_stream_closed(grpc_exec_ctx *exec_ctx, s->published_metadata[i] = GPRC_METADATA_PUBLISHED_AT_CLOSE; } } - decrement_active_streams_locked(exec_ctx, t, s); - grpc_chttp2_maybe_complete_recv_initial_metadata(exec_ctx, t, s); - grpc_chttp2_maybe_complete_recv_message(exec_ctx, t, s); + closed_read = true; } if (close_writes && !s->write_closed) { s->write_closed_error = GRPC_ERROR_REF(error); @@ -1577,6 +1575,7 @@ void grpc_chttp2_mark_stream_closed(grpc_exec_ctx *exec_ctx, grpc_chttp2_fail_pending_writes(exec_ctx, t, s, GRPC_ERROR_REF(error)); } if (s->read_closed && s->write_closed) { + became_closed = true; grpc_error *overall_error = removal_error(GRPC_ERROR_REF(error), s, "Stream removed"); if (s->id != 0) { @@ -1586,6 +1585,13 @@ void grpc_chttp2_mark_stream_closed(grpc_exec_ctx *exec_ctx, grpc_chttp2_list_remove_waiting_for_concurrency(t, s); } grpc_chttp2_fake_status(exec_ctx, t, s, overall_error); + } + if (closed_read) { + decrement_active_streams_locked(exec_ctx, t, s); + grpc_chttp2_maybe_complete_recv_initial_metadata(exec_ctx, t, s); + grpc_chttp2_maybe_complete_recv_message(exec_ctx, t, s); + } + if (became_closed) { grpc_chttp2_maybe_complete_recv_trailing_metadata(exec_ctx, t, s); GRPC_CHTTP2_STREAM_UNREF(exec_ctx, s, "chttp2"); } diff --git a/src/core/ext/transport/chttp2/transport/parsing.c b/src/core/ext/transport/chttp2/transport/parsing.c index 11e4655f11..2b8563b9d0 100644 --- a/src/core/ext/transport/chttp2/transport/parsing.c +++ b/src/core/ext/transport/chttp2/transport/parsing.c @@ -751,7 +751,7 @@ static grpc_error *parse_frame_slice(grpc_exec_ctx *exec_ctx, if (grpc_http_trace) { const char *msg = grpc_error_string(err); gpr_log(GPR_ERROR, "%s", msg); - grpc_error_free_string(msg); + } grpc_chttp2_parsing_become_skip_parser(exec_ctx, t); if (s) { diff --git a/src/core/lib/http/httpcli_security_connector.c b/src/core/lib/http/httpcli_security_connector.c index 440817c5a6..fd6fda03c3 100644 --- a/src/core/lib/http/httpcli_security_connector.c +++ b/src/core/lib/http/httpcli_security_connector.c @@ -156,7 +156,7 @@ static void on_handshake_done(grpc_exec_ctx *exec_ctx, void *arg, if (error != GRPC_ERROR_NONE) { const char *msg = grpc_error_string(error); gpr_log(GPR_ERROR, "Secure transport setup failed: %s", msg); - grpc_error_free_string(msg); + c->func(exec_ctx, c->arg, NULL); } else { grpc_channel_args_destroy(exec_ctx, args->args); diff --git a/src/core/lib/iomgr/error.c b/src/core/lib/iomgr/error.c index 3cf644f421..ceff429fcc 100644 --- a/src/core/lib/iomgr/error.c +++ b/src/core/lib/iomgr/error.c @@ -190,6 +190,7 @@ static void error_destroy(grpc_error *err) { gpr_avl_unref(err->strs); gpr_avl_unref(err->errs); gpr_avl_unref(err->times); + gpr_free((void *)gpr_atm_acq_load(&err->error_string)); gpr_free(err); } @@ -240,6 +241,7 @@ grpc_error *grpc_error_create(const char *file, int line, const char *desc, err->times = gpr_avl_add(gpr_avl_create(&avl_vtable_times), (void *)(uintptr_t)GRPC_ERROR_TIME_CREATED, box_time(gpr_now(GPR_CLOCK_REALTIME))); + gpr_atm_no_barrier_store(&err->error_string, 0); gpr_ref_init(&err->refs, 1); GPR_TIMER_END("grpc_error_create", 0); return err; @@ -269,6 +271,7 @@ static grpc_error *copy_error_and_unref(grpc_error *in) { out->strs = gpr_avl_ref(in->strs); out->errs = gpr_avl_ref(in->errs); out->times = gpr_avl_ref(in->times); + gpr_atm_no_barrier_store(&out->error_string, 0); out->next_err = in->next_err; gpr_ref_init(&out->refs, 1); GRPC_ERROR_UNREF(in); @@ -495,7 +498,6 @@ static void add_errs(gpr_avl_node *n, char **s, size_t *sz, size_t *cap, *first = false; const char *e = grpc_error_string(n->value); append_str(e, s, sz, cap); - grpc_error_free_string(e); add_errs(n->right, s, sz, cap, first); } @@ -517,7 +519,7 @@ static int cmp_kvs(const void *a, const void *b) { return strcmp(ka->key, kb->key); } -static const char *finish_kvs(kv_pairs *kvs) { +static char *finish_kvs(kv_pairs *kvs) { char *s = NULL; size_t sz = 0; size_t cap = 0; @@ -538,19 +540,18 @@ static const char *finish_kvs(kv_pairs *kvs) { return s; } -void grpc_error_free_string(const char *str) { - if (str == no_error_string) return; - if (str == oom_error_string) return; - if (str == cancelled_error_string) return; - gpr_free((char *)str); -} - const char *grpc_error_string(grpc_error *err) { GPR_TIMER_BEGIN("grpc_error_string", 0); if (err == GRPC_ERROR_NONE) return no_error_string; if (err == GRPC_ERROR_OOM) return oom_error_string; if (err == GRPC_ERROR_CANCELLED) return cancelled_error_string; + void *p = (void *)gpr_atm_acq_load(&err->error_string); + if (p != NULL) { + GPR_TIMER_END("grpc_error_string", 0); + return p; + } + kv_pairs kvs; memset(&kvs, 0, sizeof(kvs)); @@ -563,7 +564,13 @@ const char *grpc_error_string(grpc_error *err) { qsort(kvs.kvs, kvs.num_kvs, sizeof(kv_pair), cmp_kvs); - const char *out = finish_kvs(&kvs); + char *out = finish_kvs(&kvs); + + if (!gpr_atm_rel_cas(&err->error_string, 0, (gpr_atm)out)) { + gpr_free(out); + out = (char *)gpr_atm_no_barrier_load(&err->error_string); + } + GPR_TIMER_END("grpc_error_string", 0); return out; } @@ -598,7 +605,6 @@ bool grpc_log_if_error(const char *what, grpc_error *error, const char *file, if (error == GRPC_ERROR_NONE) return true; const char *msg = grpc_error_string(error); gpr_log(file, line, GPR_LOG_SEVERITY_ERROR, "%s: %s", what, msg); - grpc_error_free_string(msg); GRPC_ERROR_UNREF(error); return false; } diff --git a/src/core/lib/iomgr/error.h b/src/core/lib/iomgr/error.h index 75ca9c4bf9..ffacdac393 100644 --- a/src/core/lib/iomgr/error.h +++ b/src/core/lib/iomgr/error.h @@ -145,7 +145,6 @@ typedef enum { #define GRPC_ERROR_CANCELLED ((grpc_error *)4) const char *grpc_error_string(grpc_error *error); -void grpc_error_free_string(const char *str); /// Create an error - but use GRPC_ERROR_CREATE instead grpc_error *grpc_error_create(const char *file, int line, const char *desc, diff --git a/src/core/lib/iomgr/error_internal.h b/src/core/lib/iomgr/error_internal.h index 5afe9edc7f..8f49ba3992 100644 --- a/src/core/lib/iomgr/error_internal.h +++ b/src/core/lib/iomgr/error_internal.h @@ -46,6 +46,7 @@ struct grpc_error { gpr_avl times; gpr_avl errs; uintptr_t next_err; + gpr_atm error_string; }; bool grpc_error_is_special(grpc_error *err); diff --git a/src/core/lib/iomgr/tcp_client_posix.c b/src/core/lib/iomgr/tcp_client_posix.c index 9a77c92016..a55e7bfc72 100644 --- a/src/core/lib/iomgr/tcp_client_posix.c +++ b/src/core/lib/iomgr/tcp_client_posix.c @@ -118,7 +118,7 @@ static void tc_on_alarm(grpc_exec_ctx *exec_ctx, void *acp, grpc_error *error) { const char *str = grpc_error_string(error); gpr_log(GPR_DEBUG, "CLIENT_CONNECT: %s: on_alarm: error=%s", ac->addr_str, str); - grpc_error_free_string(str); + } gpr_mu_lock(&ac->mu); if (ac->fd != NULL) { @@ -178,7 +178,7 @@ static void on_writable(grpc_exec_ctx *exec_ctx, void *acp, grpc_error *error) { const char *str = grpc_error_string(error); gpr_log(GPR_DEBUG, "CLIENT_CONNECT: %s: on_writable: error=%s", ac->addr_str, str); - grpc_error_free_string(str); + } gpr_mu_lock(&ac->mu); diff --git a/src/core/lib/iomgr/tcp_posix.c b/src/core/lib/iomgr/tcp_posix.c index ece44978b0..402501be5e 100644 --- a/src/core/lib/iomgr/tcp_posix.c +++ b/src/core/lib/iomgr/tcp_posix.c @@ -181,7 +181,7 @@ static void call_read_cb(grpc_exec_ctx *exec_ctx, grpc_tcp *tcp, size_t i; const char *str = grpc_error_string(error); gpr_log(GPR_DEBUG, "read: error=%s", str); - grpc_error_free_string(str); + for (i = 0; i < tcp->incoming_buffer->count; i++) { char *dump = grpc_dump_slice(tcp->incoming_buffer->slices[i], GPR_DUMP_HEX | GPR_DUMP_ASCII); @@ -435,7 +435,7 @@ static void tcp_handle_write(grpc_exec_ctx *exec_ctx, void *arg /* grpc_tcp */, if (grpc_tcp_trace) { const char *str = grpc_error_string(error); gpr_log(GPR_DEBUG, "write: %s", str); - grpc_error_free_string(str); + } grpc_closure_run(exec_ctx, cb, error); @@ -485,7 +485,7 @@ static void tcp_write(grpc_exec_ctx *exec_ctx, grpc_endpoint *ep, if (grpc_tcp_trace) { const char *str = grpc_error_string(error); gpr_log(GPR_DEBUG, "write: %s", str); - grpc_error_free_string(str); + } grpc_closure_sched(exec_ctx, cb, error); } diff --git a/src/core/lib/iomgr/tcp_server_windows.c b/src/core/lib/iomgr/tcp_server_windows.c index 97d7827461..b25783f986 100644 --- a/src/core/lib/iomgr/tcp_server_windows.c +++ b/src/core/lib/iomgr/tcp_server_windows.c @@ -345,7 +345,7 @@ static void on_accept(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) { if (error != GRPC_ERROR_NONE) { const char *msg = grpc_error_string(error); gpr_log(GPR_INFO, "Skipping on_accept due to error: %s", msg); - grpc_error_free_string(msg); + gpr_mu_unlock(&sp->server->mu); return; } diff --git a/src/core/lib/iomgr/tcp_uv.c b/src/core/lib/iomgr/tcp_uv.c index a1ee597d09..4bf2e08684 100644 --- a/src/core/lib/iomgr/tcp_uv.c +++ b/src/core/lib/iomgr/tcp_uv.c @@ -157,7 +157,7 @@ static void read_callback(uv_stream_t *stream, ssize_t nread, size_t i; const char *str = grpc_error_string(error); gpr_log(GPR_DEBUG, "read: error=%s", str); - grpc_error_free_string(str); + for (i = 0; i < tcp->read_slices->count; i++) { char *dump = grpc_dump_slice(tcp->read_slices->slices[i], GPR_DUMP_HEX | GPR_DUMP_ASCII); diff --git a/src/core/lib/security/transport/security_handshaker.c b/src/core/lib/security/transport/security_handshaker.c index e886cc59a0..a14fab71fd 100644 --- a/src/core/lib/security/transport/security_handshaker.c +++ b/src/core/lib/security/transport/security_handshaker.c @@ -123,7 +123,7 @@ static void security_handshake_failed_locked(grpc_exec_ctx *exec_ctx, } const char *msg = grpc_error_string(error); gpr_log(GPR_DEBUG, "Security handshake failed: %s", msg); - grpc_error_free_string(msg); + if (!h->shutdown) { // TODO(ctiller): It is currently necessary to shutdown endpoints // before destroying them, even if we know that there are no diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c index 38b6225abd..f7e7dff097 100644 --- a/src/core/lib/surface/call.c +++ b/src/core/lib/surface/call.c @@ -603,14 +603,14 @@ static void get_final_status(grpc_call *call, int i; for (i = 0; i < STATUS_SOURCE_COUNT; i++) { if (call->status[i].is_set) { - const char *text = grpc_error_string(call->status[i].error); - gpr_log(GPR_DEBUG, "%s", text); - grpc_error_free_string(text); - grpc_status_code code; const char *msg = NULL; grpc_error_get_status(call->status[i].error, call->send_deadline, &code, &msg, NULL); + + gpr_log(GPR_DEBUG, "%s --> %d %s", + grpc_error_string(call->status[i].error), code, msg); + set_value(code, set_value_user_data); if (details != NULL) { *details = grpc_slice_from_copied_string(msg); @@ -630,7 +630,6 @@ static void set_status_from_error(grpc_exec_ctx *exec_ctx, grpc_call *call, const char *es = grpc_error_string(error); gpr_log(GPR_DEBUG, "%p[%d]: set %d[is_set=%d] to %s", call, call->is_client, source, call->status[source].is_set, es); - grpc_error_free_string(es); if (call->status[source].is_set) { GRPC_ERROR_UNREF(error); diff --git a/src/core/lib/surface/completion_queue.c b/src/core/lib/surface/completion_queue.c index 4613c9021e..d1f1cd046e 100644 --- a/src/core/lib/surface/completion_queue.c +++ b/src/core/lib/surface/completion_queue.c @@ -253,7 +253,7 @@ void grpc_cq_end_op(grpc_exec_ctx *exec_ctx, grpc_completion_queue *cc, if (grpc_trace_operation_failures && error != GRPC_ERROR_NONE) { gpr_log(GPR_ERROR, "Operation failed: tag=%p, error=%s", tag, errmsg); } - grpc_error_free_string(errmsg); + } storage->tag = tag; @@ -294,7 +294,7 @@ void grpc_cq_end_op(grpc_exec_ctx *exec_ctx, grpc_completion_queue *cc, if (kick_error != GRPC_ERROR_NONE) { const char *msg = grpc_error_string(kick_error); gpr_log(GPR_ERROR, "Kick failed: %s", msg); - grpc_error_free_string(msg); + GRPC_ERROR_UNREF(kick_error); } } else { @@ -461,7 +461,7 @@ grpc_event grpc_completion_queue_next(grpc_completion_queue *cc, gpr_mu_unlock(cc->mu); const char *msg = grpc_error_string(err); gpr_log(GPR_ERROR, "Completion queue next failed: %s", msg); - grpc_error_free_string(msg); + GRPC_ERROR_UNREF(err); memset(&ret, 0, sizeof(ret)); ret.type = GRPC_QUEUE_TIMEOUT; @@ -647,7 +647,7 @@ grpc_event grpc_completion_queue_pluck(grpc_completion_queue *cc, void *tag, gpr_mu_unlock(cc->mu); const char *msg = grpc_error_string(err); gpr_log(GPR_ERROR, "Completion queue next failed: %s", msg); - grpc_error_free_string(msg); + GRPC_ERROR_UNREF(err); memset(&ret, 0, sizeof(ret)); ret.type = GRPC_QUEUE_TIMEOUT; diff --git a/src/core/lib/surface/server.c b/src/core/lib/surface/server.c index b5f905bf04..c70aaa174b 100644 --- a/src/core/lib/surface/server.c +++ b/src/core/lib/surface/server.c @@ -453,7 +453,7 @@ static void destroy_channel(grpc_exec_ctx *exec_ctx, channel_data *chand, if (grpc_server_channel_trace && error != GRPC_ERROR_NONE) { const char *msg = grpc_error_string(error); gpr_log(GPR_INFO, "Disconnected client: %s", msg); - grpc_error_free_string(msg); + } GRPC_ERROR_UNREF(error); diff --git a/src/core/lib/transport/connectivity_state.c b/src/core/lib/transport/connectivity_state.c index c656d93740..2f55ffd96c 100644 --- a/src/core/lib/transport/connectivity_state.c +++ b/src/core/lib/transport/connectivity_state.c @@ -163,7 +163,7 @@ void grpc_connectivity_state_set(grpc_exec_ctx *exec_ctx, gpr_log(GPR_DEBUG, "SET: %p %s: %s --> %s [%s] error=%p %s", tracker, tracker->name, grpc_connectivity_state_name(tracker->current_state), grpc_connectivity_state_name(state), reason, error, error_string); - grpc_error_free_string(error_string); + } switch (state) { case GRPC_CHANNEL_INIT: diff --git a/src/core/lib/transport/transport_op_string.c b/src/core/lib/transport/transport_op_string.c index 8810a6bbda..9d5f9c2d6e 100644 --- a/src/core/lib/transport/transport_op_string.c +++ b/src/core/lib/transport/transport_op_string.c @@ -121,7 +121,7 @@ char *grpc_transport_stream_op_string(grpc_transport_stream_op *op) { gpr_strvec_add(&b, gpr_strdup(" ")); const char *msg = grpc_error_string(op->cancel_error); gpr_asprintf(&tmp, "CANCEL:%s", msg); - grpc_error_free_string(msg); + gpr_strvec_add(&b, tmp); } @@ -160,7 +160,7 @@ char *grpc_transport_op_string(grpc_transport_op *op) { const char *err = grpc_error_string(op->disconnect_with_error); gpr_asprintf(&tmp, "DISCONNECT:%s", err); gpr_strvec_add(&b, tmp); - grpc_error_free_string(err); + } if (op->goaway_error) { @@ -168,7 +168,7 @@ char *grpc_transport_op_string(grpc_transport_op *op) { first = false; const char *msg = grpc_error_string(op->goaway_error); gpr_asprintf(&tmp, "SEND_GOAWAY:%s", msg); - grpc_error_free_string(msg); + gpr_strvec_add(&b, tmp); } diff --git a/test/core/end2end/fixtures/http_proxy.c b/test/core/end2end/fixtures/http_proxy.c index b62082c6fb..5da6f03e18 100644 --- a/test/core/end2end/fixtures/http_proxy.c +++ b/test/core/end2end/fixtures/http_proxy.c @@ -132,7 +132,7 @@ static void proxy_connection_failed(grpc_exec_ctx* exec_ctx, const char* prefix, grpc_error* error) { const char* msg = grpc_error_string(error); gpr_log(GPR_INFO, "%s: %s", prefix, msg); - grpc_error_free_string(msg); + grpc_endpoint_shutdown(exec_ctx, conn->client_endpoint); if (conn->server_endpoint != NULL) grpc_endpoint_shutdown(exec_ctx, conn->server_endpoint); diff --git a/test/core/util/port_server_client.c b/test/core/util/port_server_client.c index 4baf63dcd7..74a4d14ff8 100644 --- a/test/core/util/port_server_client.c +++ b/test/core/util/port_server_client.c @@ -152,7 +152,7 @@ static void got_port_from_server(grpc_exec_ctx *exec_ctx, void *arg, failed = 1; const char *msg = grpc_error_string(error); gpr_log(GPR_DEBUG, "failed port pick from server: retrying [%s]", msg); - grpc_error_free_string(msg); + } else if (response->status != 200) { failed = 1; gpr_log(GPR_DEBUG, "failed port pick from server: status=%d", -- cgit v1.2.3 From 76dca1995352461a97825a28a816c97f0eac2cfb Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 6 Jan 2017 14:54:18 -0800 Subject: clang-format --- src/core/ext/client_channel/subchannel.c | 3 +-- src/core/ext/resolver/dns/native/dns_resolver.c | 2 +- src/core/ext/transport/chttp2/server/chttp2_server.c | 4 ++-- src/core/ext/transport/chttp2/server/insecure/server_chttp2.c | 2 +- src/core/ext/transport/chttp2/server/secure/server_secure_chttp2.c | 2 +- src/core/ext/transport/chttp2/transport/parsing.c | 1 - src/core/lib/http/httpcli_security_connector.c | 2 +- src/core/lib/iomgr/tcp_client_posix.c | 2 -- src/core/lib/iomgr/tcp_posix.c | 4 +--- src/core/lib/iomgr/tcp_server_windows.c | 2 +- src/core/lib/iomgr/tcp_uv.c | 2 +- src/core/lib/security/transport/security_handshaker.c | 2 +- src/core/lib/surface/completion_queue.c | 7 +++---- src/core/lib/surface/server.c | 1 - src/core/lib/transport/connectivity_state.c | 1 - src/core/lib/transport/transport_op_string.c | 5 ++--- test/core/end2end/fixtures/http_proxy.c | 2 +- test/core/util/port_server_client.c | 2 +- 18 files changed, 18 insertions(+), 28 deletions(-) (limited to 'src/core/ext/client_channel') diff --git a/src/core/ext/client_channel/subchannel.c b/src/core/ext/client_channel/subchannel.c index 73ae1b859d..86a19d99ba 100644 --- a/src/core/ext/client_channel/subchannel.c +++ b/src/core/ext/client_channel/subchannel.c @@ -687,7 +687,6 @@ static void subchannel_connected(grpc_exec_ctx *exec_ctx, void *arg, const char *errmsg = grpc_error_string(error); gpr_log(GPR_INFO, "Connect failed: %s", errmsg); - maybe_start_connecting_locked(exec_ctx, c); GRPC_SUBCHANNEL_WEAK_UNREF(exec_ctx, c, "connecting"); @@ -758,7 +757,7 @@ grpc_error *grpc_connected_subchannel_create_call( if (error != GRPC_ERROR_NONE) { const char *error_string = grpc_error_string(error); gpr_log(GPR_ERROR, "error: %s", error_string); - + gpr_free(*call); return error; } diff --git a/src/core/ext/resolver/dns/native/dns_resolver.c b/src/core/ext/resolver/dns/native/dns_resolver.c index 297700143e..9bb1c0c227 100644 --- a/src/core/ext/resolver/dns/native/dns_resolver.c +++ b/src/core/ext/resolver/dns/native/dns_resolver.c @@ -190,7 +190,7 @@ static void dns_on_resolved(grpc_exec_ctx *exec_ctx, void *arg, gpr_timespec timeout = gpr_time_sub(next_try, now); const char *msg = grpc_error_string(error); gpr_log(GPR_DEBUG, "dns resolution failed: %s", msg); - + GPR_ASSERT(!r->have_retry_timer); r->have_retry_timer = true; GRPC_RESOLVER_REF(&r->base, "retry-timer"); diff --git a/src/core/ext/transport/chttp2/server/chttp2_server.c b/src/core/ext/transport/chttp2/server/chttp2_server.c index fb02960160..86fe8d1ccd 100644 --- a/src/core/ext/transport/chttp2/server/chttp2_server.c +++ b/src/core/ext/transport/chttp2/server/chttp2_server.c @@ -139,7 +139,7 @@ static void on_handshake_done(grpc_exec_ctx *exec_ctx, void *arg, if (error != GRPC_ERROR_NONE || connection_state->server_state->shutdown) { const char *error_str = grpc_error_string(error); gpr_log(GPR_ERROR, "Handshaking failed: %s", error_str); - + if (error == GRPC_ERROR_NONE && args->endpoint != NULL) { // We were shut down after handshaking completed successfully, so // destroy the endpoint here. @@ -328,7 +328,7 @@ grpc_error *grpc_chttp2_server_add_port( const char *warning_message = grpc_error_string(err); gpr_log(GPR_INFO, "WARNING: %s", warning_message); - + /* we managed to bind some addresses: continue */ } grpc_resolved_addresses_destroy(resolved); diff --git a/src/core/ext/transport/chttp2/server/insecure/server_chttp2.c b/src/core/ext/transport/chttp2/server/insecure/server_chttp2.c index 996681842b..77ed26a54a 100644 --- a/src/core/ext/transport/chttp2/server/insecure/server_chttp2.c +++ b/src/core/ext/transport/chttp2/server/insecure/server_chttp2.c @@ -52,7 +52,7 @@ int grpc_server_add_insecure_http2_port(grpc_server *server, const char *addr) { if (err != GRPC_ERROR_NONE) { const char *msg = grpc_error_string(err); gpr_log(GPR_ERROR, "%s", msg); - + GRPC_ERROR_UNREF(err); } grpc_exec_ctx_finish(&exec_ctx); diff --git a/src/core/ext/transport/chttp2/server/secure/server_secure_chttp2.c b/src/core/ext/transport/chttp2/server/secure/server_secure_chttp2.c index 2469b9531f..f909a760d0 100644 --- a/src/core/ext/transport/chttp2/server/secure/server_secure_chttp2.c +++ b/src/core/ext/transport/chttp2/server/secure/server_secure_chttp2.c @@ -123,7 +123,7 @@ done: if (err != GRPC_ERROR_NONE) { const char *msg = grpc_error_string(err); gpr_log(GPR_ERROR, "%s", msg); - + GRPC_ERROR_UNREF(err); } return port_num; diff --git a/src/core/ext/transport/chttp2/transport/parsing.c b/src/core/ext/transport/chttp2/transport/parsing.c index 2b8563b9d0..f58cd696f9 100644 --- a/src/core/ext/transport/chttp2/transport/parsing.c +++ b/src/core/ext/transport/chttp2/transport/parsing.c @@ -751,7 +751,6 @@ static grpc_error *parse_frame_slice(grpc_exec_ctx *exec_ctx, if (grpc_http_trace) { const char *msg = grpc_error_string(err); gpr_log(GPR_ERROR, "%s", msg); - } grpc_chttp2_parsing_become_skip_parser(exec_ctx, t); if (s) { diff --git a/src/core/lib/http/httpcli_security_connector.c b/src/core/lib/http/httpcli_security_connector.c index fd6fda03c3..f4f6f3c27a 100644 --- a/src/core/lib/http/httpcli_security_connector.c +++ b/src/core/lib/http/httpcli_security_connector.c @@ -156,7 +156,7 @@ static void on_handshake_done(grpc_exec_ctx *exec_ctx, void *arg, if (error != GRPC_ERROR_NONE) { const char *msg = grpc_error_string(error); gpr_log(GPR_ERROR, "Secure transport setup failed: %s", msg); - + c->func(exec_ctx, c->arg, NULL); } else { grpc_channel_args_destroy(exec_ctx, args->args); diff --git a/src/core/lib/iomgr/tcp_client_posix.c b/src/core/lib/iomgr/tcp_client_posix.c index a55e7bfc72..16b0f4e73c 100644 --- a/src/core/lib/iomgr/tcp_client_posix.c +++ b/src/core/lib/iomgr/tcp_client_posix.c @@ -118,7 +118,6 @@ static void tc_on_alarm(grpc_exec_ctx *exec_ctx, void *acp, grpc_error *error) { const char *str = grpc_error_string(error); gpr_log(GPR_DEBUG, "CLIENT_CONNECT: %s: on_alarm: error=%s", ac->addr_str, str); - } gpr_mu_lock(&ac->mu); if (ac->fd != NULL) { @@ -178,7 +177,6 @@ static void on_writable(grpc_exec_ctx *exec_ctx, void *acp, grpc_error *error) { const char *str = grpc_error_string(error); gpr_log(GPR_DEBUG, "CLIENT_CONNECT: %s: on_writable: error=%s", ac->addr_str, str); - } gpr_mu_lock(&ac->mu); diff --git a/src/core/lib/iomgr/tcp_posix.c b/src/core/lib/iomgr/tcp_posix.c index 402501be5e..a33e63e845 100644 --- a/src/core/lib/iomgr/tcp_posix.c +++ b/src/core/lib/iomgr/tcp_posix.c @@ -181,7 +181,7 @@ static void call_read_cb(grpc_exec_ctx *exec_ctx, grpc_tcp *tcp, size_t i; const char *str = grpc_error_string(error); gpr_log(GPR_DEBUG, "read: error=%s", str); - + for (i = 0; i < tcp->incoming_buffer->count; i++) { char *dump = grpc_dump_slice(tcp->incoming_buffer->slices[i], GPR_DUMP_HEX | GPR_DUMP_ASCII); @@ -435,7 +435,6 @@ static void tcp_handle_write(grpc_exec_ctx *exec_ctx, void *arg /* grpc_tcp */, if (grpc_tcp_trace) { const char *str = grpc_error_string(error); gpr_log(GPR_DEBUG, "write: %s", str); - } grpc_closure_run(exec_ctx, cb, error); @@ -485,7 +484,6 @@ static void tcp_write(grpc_exec_ctx *exec_ctx, grpc_endpoint *ep, if (grpc_tcp_trace) { const char *str = grpc_error_string(error); gpr_log(GPR_DEBUG, "write: %s", str); - } grpc_closure_sched(exec_ctx, cb, error); } diff --git a/src/core/lib/iomgr/tcp_server_windows.c b/src/core/lib/iomgr/tcp_server_windows.c index b25783f986..7041f8fb7e 100644 --- a/src/core/lib/iomgr/tcp_server_windows.c +++ b/src/core/lib/iomgr/tcp_server_windows.c @@ -345,7 +345,7 @@ static void on_accept(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) { if (error != GRPC_ERROR_NONE) { const char *msg = grpc_error_string(error); gpr_log(GPR_INFO, "Skipping on_accept due to error: %s", msg); - + gpr_mu_unlock(&sp->server->mu); return; } diff --git a/src/core/lib/iomgr/tcp_uv.c b/src/core/lib/iomgr/tcp_uv.c index 4bf2e08684..7f4ea49a1c 100644 --- a/src/core/lib/iomgr/tcp_uv.c +++ b/src/core/lib/iomgr/tcp_uv.c @@ -157,7 +157,7 @@ static void read_callback(uv_stream_t *stream, ssize_t nread, size_t i; const char *str = grpc_error_string(error); gpr_log(GPR_DEBUG, "read: error=%s", str); - + for (i = 0; i < tcp->read_slices->count; i++) { char *dump = grpc_dump_slice(tcp->read_slices->slices[i], GPR_DUMP_HEX | GPR_DUMP_ASCII); diff --git a/src/core/lib/security/transport/security_handshaker.c b/src/core/lib/security/transport/security_handshaker.c index a14fab71fd..6891debfd3 100644 --- a/src/core/lib/security/transport/security_handshaker.c +++ b/src/core/lib/security/transport/security_handshaker.c @@ -123,7 +123,7 @@ static void security_handshake_failed_locked(grpc_exec_ctx *exec_ctx, } const char *msg = grpc_error_string(error); gpr_log(GPR_DEBUG, "Security handshake failed: %s", msg); - + if (!h->shutdown) { // TODO(ctiller): It is currently necessary to shutdown endpoints // before destroying them, even if we know that there are no diff --git a/src/core/lib/surface/completion_queue.c b/src/core/lib/surface/completion_queue.c index d1f1cd046e..20c22ea2bc 100644 --- a/src/core/lib/surface/completion_queue.c +++ b/src/core/lib/surface/completion_queue.c @@ -253,7 +253,6 @@ void grpc_cq_end_op(grpc_exec_ctx *exec_ctx, grpc_completion_queue *cc, if (grpc_trace_operation_failures && error != GRPC_ERROR_NONE) { gpr_log(GPR_ERROR, "Operation failed: tag=%p, error=%s", tag, errmsg); } - } storage->tag = tag; @@ -294,7 +293,7 @@ void grpc_cq_end_op(grpc_exec_ctx *exec_ctx, grpc_completion_queue *cc, if (kick_error != GRPC_ERROR_NONE) { const char *msg = grpc_error_string(kick_error); gpr_log(GPR_ERROR, "Kick failed: %s", msg); - + GRPC_ERROR_UNREF(kick_error); } } else { @@ -461,7 +460,7 @@ grpc_event grpc_completion_queue_next(grpc_completion_queue *cc, gpr_mu_unlock(cc->mu); const char *msg = grpc_error_string(err); gpr_log(GPR_ERROR, "Completion queue next failed: %s", msg); - + GRPC_ERROR_UNREF(err); memset(&ret, 0, sizeof(ret)); ret.type = GRPC_QUEUE_TIMEOUT; @@ -647,7 +646,7 @@ grpc_event grpc_completion_queue_pluck(grpc_completion_queue *cc, void *tag, gpr_mu_unlock(cc->mu); const char *msg = grpc_error_string(err); gpr_log(GPR_ERROR, "Completion queue next failed: %s", msg); - + GRPC_ERROR_UNREF(err); memset(&ret, 0, sizeof(ret)); ret.type = GRPC_QUEUE_TIMEOUT; diff --git a/src/core/lib/surface/server.c b/src/core/lib/surface/server.c index c70aaa174b..7d0b30e97a 100644 --- a/src/core/lib/surface/server.c +++ b/src/core/lib/surface/server.c @@ -453,7 +453,6 @@ static void destroy_channel(grpc_exec_ctx *exec_ctx, channel_data *chand, if (grpc_server_channel_trace && error != GRPC_ERROR_NONE) { const char *msg = grpc_error_string(error); gpr_log(GPR_INFO, "Disconnected client: %s", msg); - } GRPC_ERROR_UNREF(error); diff --git a/src/core/lib/transport/connectivity_state.c b/src/core/lib/transport/connectivity_state.c index 2f55ffd96c..8fc5bf3e9a 100644 --- a/src/core/lib/transport/connectivity_state.c +++ b/src/core/lib/transport/connectivity_state.c @@ -163,7 +163,6 @@ void grpc_connectivity_state_set(grpc_exec_ctx *exec_ctx, gpr_log(GPR_DEBUG, "SET: %p %s: %s --> %s [%s] error=%p %s", tracker, tracker->name, grpc_connectivity_state_name(tracker->current_state), grpc_connectivity_state_name(state), reason, error, error_string); - } switch (state) { case GRPC_CHANNEL_INIT: diff --git a/src/core/lib/transport/transport_op_string.c b/src/core/lib/transport/transport_op_string.c index 9d5f9c2d6e..28360e3784 100644 --- a/src/core/lib/transport/transport_op_string.c +++ b/src/core/lib/transport/transport_op_string.c @@ -121,7 +121,7 @@ char *grpc_transport_stream_op_string(grpc_transport_stream_op *op) { gpr_strvec_add(&b, gpr_strdup(" ")); const char *msg = grpc_error_string(op->cancel_error); gpr_asprintf(&tmp, "CANCEL:%s", msg); - + gpr_strvec_add(&b, tmp); } @@ -160,7 +160,6 @@ char *grpc_transport_op_string(grpc_transport_op *op) { const char *err = grpc_error_string(op->disconnect_with_error); gpr_asprintf(&tmp, "DISCONNECT:%s", err); gpr_strvec_add(&b, tmp); - } if (op->goaway_error) { @@ -168,7 +167,7 @@ char *grpc_transport_op_string(grpc_transport_op *op) { first = false; const char *msg = grpc_error_string(op->goaway_error); gpr_asprintf(&tmp, "SEND_GOAWAY:%s", msg); - + gpr_strvec_add(&b, tmp); } diff --git a/test/core/end2end/fixtures/http_proxy.c b/test/core/end2end/fixtures/http_proxy.c index 5da6f03e18..dac9baf3ce 100644 --- a/test/core/end2end/fixtures/http_proxy.c +++ b/test/core/end2end/fixtures/http_proxy.c @@ -132,7 +132,7 @@ static void proxy_connection_failed(grpc_exec_ctx* exec_ctx, const char* prefix, grpc_error* error) { const char* msg = grpc_error_string(error); gpr_log(GPR_INFO, "%s: %s", prefix, msg); - + grpc_endpoint_shutdown(exec_ctx, conn->client_endpoint); if (conn->server_endpoint != NULL) grpc_endpoint_shutdown(exec_ctx, conn->server_endpoint); diff --git a/test/core/util/port_server_client.c b/test/core/util/port_server_client.c index 74a4d14ff8..118708d419 100644 --- a/test/core/util/port_server_client.c +++ b/test/core/util/port_server_client.c @@ -152,7 +152,7 @@ static void got_port_from_server(grpc_exec_ctx *exec_ctx, void *arg, failed = 1; const char *msg = grpc_error_string(error); gpr_log(GPR_DEBUG, "failed port pick from server: retrying [%s]", msg); - + } else if (response->status != 200) { failed = 1; gpr_log(GPR_DEBUG, "failed port pick from server: status=%d", -- cgit v1.2.3 From 81a7937b4b5d1a0d3dd0114d9ff0442fdaf48210 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 6 Jan 2017 14:56:25 -0800 Subject: Fixes --- src/core/ext/client_channel/subchannel.c | 5 ++--- src/core/lib/surface/channel.c | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) (limited to 'src/core/ext/client_channel') diff --git a/src/core/ext/client_channel/subchannel.c b/src/core/ext/client_channel/subchannel.c index 86a19d99ba..f55fa60f91 100644 --- a/src/core/ext/client_channel/subchannel.c +++ b/src/core/ext/client_channel/subchannel.c @@ -620,9 +620,8 @@ static void publish_transport_locked(grpc_exec_ctx *exec_ctx, grpc_error *error = grpc_channel_stack_builder_finish( exec_ctx, builder, 0, 1, connection_destroy, NULL, (void **)&con); if (error != GRPC_ERROR_NONE) { - const char *msg = grpc_error_string(error); - gpr_log(GPR_ERROR, "error initializing subchannel stack: %s", msg); - grpc_error_free_string(msg); + gpr_log(GPR_ERROR, "error initializing subchannel stack: %s", + grpc_error_string(error)); GRPC_ERROR_UNREF(error); abort(); /* TODO(ctiller): what to do here? */ } diff --git a/src/core/lib/surface/channel.c b/src/core/lib/surface/channel.c index f2708fdf43..1ef023ed62 100644 --- a/src/core/lib/surface/channel.c +++ b/src/core/lib/surface/channel.c @@ -102,9 +102,8 @@ grpc_channel *grpc_channel_create(grpc_exec_ctx *exec_ctx, const char *target, exec_ctx, builder, sizeof(grpc_channel), 1, destroy_channel, NULL, (void **)&channel); if (error != GRPC_ERROR_NONE) { - const char *msg = grpc_error_string(error); - gpr_log(GPR_ERROR, "channel stack builder failed: %s", msg); - grpc_error_free_string(msg); + gpr_log(GPR_ERROR, "channel stack builder failed: %s", + grpc_error_string(error)); GRPC_ERROR_UNREF(error); goto done; } -- cgit v1.2.3 From 0748f3925cf0892b4780de25199bdd82aab30e57 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Fri, 13 Jan 2017 09:22:44 -0800 Subject: Store subchannel address in a channel arg. --- src/core/ext/client_channel/connector.h | 3 -- src/core/ext/client_channel/subchannel.c | 40 +++++++++++++++++----- src/core/ext/client_channel/subchannel.h | 8 +++-- src/core/ext/client_channel/subchannel_index.c | 12 ------- src/core/ext/lb_policy/pick_first/pick_first.c | 15 ++++++-- src/core/ext/lb_policy/round_robin/round_robin.c | 15 ++++++-- .../ext/transport/chttp2/client/chttp2_connector.c | 10 ++++-- 7 files changed, 69 insertions(+), 34 deletions(-) (limited to 'src/core/ext/client_channel') diff --git a/src/core/ext/client_channel/connector.h b/src/core/ext/client_channel/connector.h index 3de061620e..395f89b3b2 100644 --- a/src/core/ext/client_channel/connector.h +++ b/src/core/ext/client_channel/connector.h @@ -48,9 +48,6 @@ struct grpc_connector { typedef struct { /** set of pollsets interested in this connection */ grpc_pollset_set *interested_parties; - /** address to connect to */ - const grpc_resolved_address *addr; - size_t addr_len; /** initial connect string to send */ grpc_slice initial_connect_string; /** deadline for connection */ diff --git a/src/core/ext/client_channel/subchannel.c b/src/core/ext/client_channel/subchannel.c index 1bac82b451..7a2ad98433 100644 --- a/src/core/ext/client_channel/subchannel.c +++ b/src/core/ext/client_channel/subchannel.c @@ -41,9 +41,12 @@ #include "src/core/ext/client_channel/client_channel.h" #include "src/core/ext/client_channel/initial_connect_string.h" +#include "src/core/ext/client_channel/parse_address.h" #include "src/core/ext/client_channel/subchannel_index.h" +#include "src/core/ext/client_channel/uri_parser.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/connected_channel.h" +#include "src/core/lib/iomgr/sockaddr_utils.h" #include "src/core/lib/iomgr/timer.h" #include "src/core/lib/profiling/timers.h" #include "src/core/lib/slice/slice_internal.h" @@ -95,8 +98,6 @@ struct grpc_subchannel { size_t num_filters; /** channel arguments */ grpc_channel_args *args; - /** address to connect to */ - grpc_resolved_address *addr; grpc_subchannel_key *key; @@ -211,7 +212,6 @@ static void subchannel_destroy(grpc_exec_ctx *exec_ctx, void *arg, grpc_subchannel *c = arg; gpr_free((void *)c->filters); grpc_channel_args_destroy(exec_ctx, c->args); - gpr_free(c->addr); grpc_slice_unref_internal(exec_ctx, c->initial_connect_string); grpc_connectivity_state_destroy(exec_ctx, &c->state_tracker); grpc_connector_unref(exec_ctx, c->connector); @@ -327,12 +327,22 @@ grpc_subchannel *grpc_subchannel_create(grpc_exec_ctx *exec_ctx, } else { c->filters = NULL; } - c->addr = gpr_malloc(sizeof(grpc_resolved_address)); - if (args->addr->len) - memcpy(c->addr, args->addr, sizeof(grpc_resolved_address)); c->pollset_set = grpc_pollset_set_create(); - grpc_set_initial_connect_string(&c->addr, &c->initial_connect_string); - c->args = grpc_channel_args_copy(args->args); + const grpc_arg *addr_arg = + grpc_channel_args_find(args->args, GRPC_ARG_SUBCHANNEL_ADDRESS); + GPR_ASSERT(addr_arg != NULL); // Should have been set by LB policy. + grpc_resolved_address *addr = gpr_malloc(sizeof(*addr)); + grpc_uri_to_sockaddr(addr_arg->value.string, addr); + grpc_set_initial_connect_string(&addr, &c->initial_connect_string); + static const char *keys_to_remove[] = {GRPC_ARG_SUBCHANNEL_ADDRESS}; + grpc_arg new_arg; + new_arg.key = GRPC_ARG_SUBCHANNEL_ADDRESS; + new_arg.type = GRPC_ARG_STRING; + new_arg.value.string = grpc_sockaddr_to_uri(addr); + gpr_free(addr); + c->args = grpc_channel_args_copy_and_add_and_remove( + args->args, keys_to_remove, GPR_ARRAY_SIZE(keys_to_remove), &new_arg, 1); + gpr_free(new_arg.value.string); c->root_external_state_watcher.next = c->root_external_state_watcher.prev = &c->root_external_state_watcher; grpc_closure_init(&c->connected, subchannel_connected, c, @@ -385,7 +395,6 @@ static void continue_connect_locked(grpc_exec_ctx *exec_ctx, grpc_connect_in_args args; args.interested_parties = c->pollset_set; - args.addr = c->addr; args.deadline = c->next_attempt; args.channel_args = c->args; args.initial_connect_string = c->initial_connect_string; @@ -771,3 +780,16 @@ grpc_call_stack *grpc_subchannel_call_get_call_stack( grpc_subchannel_call *subchannel_call) { return SUBCHANNEL_CALL_TO_CALL_STACK(subchannel_call); } + +void grpc_uri_to_sockaddr(char *uri_str, grpc_resolved_address *addr) { + grpc_uri *uri = grpc_uri_parse(uri_str, 0 /* suppress_errors */); + GPR_ASSERT(uri != NULL); + if (strcmp(uri->scheme, "ipv4") == 0) { + GPR_ASSERT(parse_ipv4(uri, addr)); + } else if (strcmp(uri->scheme, "ipv6") == 0) { + GPR_ASSERT(parse_ipv6(uri, addr)); + } else { + GPR_ASSERT(parse_unix(uri, addr)); + } + grpc_uri_destroy(uri); +} diff --git a/src/core/ext/client_channel/subchannel.h b/src/core/ext/client_channel/subchannel.h index 24aa9f73dc..4cc33fc79b 100644 --- a/src/core/ext/client_channel/subchannel.h +++ b/src/core/ext/client_channel/subchannel.h @@ -40,6 +40,9 @@ #include "src/core/lib/transport/connectivity_state.h" #include "src/core/lib/transport/metadata.h" +// Channel arg containing a grpc_resolved_address to connect to. +#define GRPC_ARG_SUBCHANNEL_ADDRESS "grpc.subchannel_address" + /** A (sub-)channel that knows how to connect to exactly one target address. Provides a target for load balancing. */ typedef struct grpc_subchannel grpc_subchannel; @@ -164,8 +167,6 @@ struct grpc_subchannel_args { size_t filter_count; /** Channel arguments to be supplied to the newly created channel */ const grpc_channel_args *args; - /** Address to connect to */ - grpc_resolved_address *addr; }; /** create a subchannel given a connector */ @@ -173,4 +174,7 @@ grpc_subchannel *grpc_subchannel_create(grpc_exec_ctx *exec_ctx, grpc_connector *connector, const grpc_subchannel_args *args); +/// Sets \a addr from \a uri_str. +void grpc_uri_to_sockaddr(char *uri_str, grpc_resolved_address *addr); + #endif /* GRPC_CORE_EXT_CLIENT_CHANNEL_SUBCHANNEL_H */ diff --git a/src/core/ext/client_channel/subchannel_index.c b/src/core/ext/client_channel/subchannel_index.c index 1ebe03ef11..11889300a2 100644 --- a/src/core/ext/client_channel/subchannel_index.c +++ b/src/core/ext/client_channel/subchannel_index.c @@ -86,11 +86,6 @@ static grpc_subchannel_key *create_key( } else { k->args.filters = NULL; } - k->args.addr = gpr_malloc(sizeof(grpc_resolved_address)); - k->args.addr->len = args->addr->len; - if (k->args.addr->len > 0) { - memcpy(k->args.addr, args->addr, sizeof(grpc_resolved_address)); - } k->args.args = copy_channel_args(args->args); return k; } @@ -108,14 +103,8 @@ 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 = GPR_ICMP(a->args.filter_count, b->args.filter_count); if (c != 0) return c; - if (a->args.addr->len) { - c = memcmp(a->args.addr->addr, b->args.addr->addr, a->args.addr->len); - if (c != 0) return c; - } if (a->args.filter_count > 0) { c = memcmp(a->args.filters, b->args.filters, a->args.filter_count * sizeof(*a->args.filters)); @@ -129,7 +118,6 @@ void grpc_subchannel_key_destroy(grpc_exec_ctx *exec_ctx, grpc_connector_unref(exec_ctx, k->connector); gpr_free((grpc_channel_args *)k->args.filters); grpc_channel_args_destroy(exec_ctx, (grpc_channel_args *)k->args.args); - gpr_free(k->args.addr); gpr_free(k); } diff --git a/src/core/ext/lb_policy/pick_first/pick_first.c b/src/core/ext/lb_policy/pick_first/pick_first.c index 821becff69..4677fb2343 100644 --- a/src/core/ext/lb_policy/pick_first/pick_first.c +++ b/src/core/ext/lb_policy/pick_first/pick_first.c @@ -36,7 +36,9 @@ #include #include "src/core/ext/client_channel/lb_policy_registry.h" +#include "src/core/ext/client_channel/subchannel.h" #include "src/core/lib/channel/channel_args.h" +#include "src/core/lib/iomgr/sockaddr_utils.h" #include "src/core/lib/transport/connectivity_state.h" typedef struct pending_pick { @@ -466,11 +468,18 @@ static grpc_lb_policy *create_pick_first(grpc_exec_ctx *exec_ctx, } memset(&sc_args, 0, sizeof(grpc_subchannel_args)); - sc_args.addr = &addresses->addresses[i].address; - sc_args.args = args->args; - + grpc_arg addr_arg; + addr_arg.key = GRPC_ARG_SUBCHANNEL_ADDRESS; + addr_arg.type = GRPC_ARG_STRING; + addr_arg.value.string = + grpc_sockaddr_to_uri(&addresses->addresses[i].address); + grpc_channel_args *new_args = + grpc_channel_args_copy_and_add(args->args, &addr_arg, 1); + gpr_free(addr_arg.value.string); + sc_args.args = new_args; grpc_subchannel *subchannel = grpc_client_channel_factory_create_subchannel( exec_ctx, args->client_channel_factory, &sc_args); + grpc_channel_args_destroy(exec_ctx, new_args); if (subchannel != NULL) { p->subchannels[subchannel_idx++] = subchannel; diff --git a/src/core/ext/lb_policy/round_robin/round_robin.c b/src/core/ext/lb_policy/round_robin/round_robin.c index cb679489c3..3fe157a23c 100644 --- a/src/core/ext/lb_policy/round_robin/round_robin.c +++ b/src/core/ext/lb_policy/round_robin/round_robin.c @@ -64,8 +64,10 @@ #include #include "src/core/ext/client_channel/lb_policy_registry.h" +#include "src/core/ext/client_channel/subchannel.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/debug/trace.h" +#include "src/core/lib/iomgr/sockaddr_utils.h" #include "src/core/lib/transport/connectivity_state.h" #include "src/core/lib/transport/static_metadata.h" @@ -729,11 +731,18 @@ static grpc_lb_policy *round_robin_create(grpc_exec_ctx *exec_ctx, if (addresses->addresses[i].is_balancer) continue; memset(&sc_args, 0, sizeof(grpc_subchannel_args)); - sc_args.addr = &addresses->addresses[i].address; - sc_args.args = args->args; - + grpc_arg addr_arg; + addr_arg.key = GRPC_ARG_SUBCHANNEL_ADDRESS; + addr_arg.type = GRPC_ARG_STRING; + addr_arg.value.string = + grpc_sockaddr_to_uri(&addresses->addresses[i].address); + grpc_channel_args *new_args = + grpc_channel_args_copy_and_add(args->args, &addr_arg, 1); + gpr_free(addr_arg.value.string); + sc_args.args = new_args; grpc_subchannel *subchannel = grpc_client_channel_factory_create_subchannel( exec_ctx, args->client_channel_factory, &sc_args); + grpc_channel_args_destroy(exec_ctx, new_args); if (subchannel != NULL) { subchannel_data *sd = gpr_malloc(sizeof(*sd)); diff --git a/src/core/ext/transport/chttp2/client/chttp2_connector.c b/src/core/ext/transport/chttp2/client/chttp2_connector.c index 2c5dfaea60..ebe884b115 100644 --- a/src/core/ext/transport/chttp2/client/chttp2_connector.c +++ b/src/core/ext/transport/chttp2/client/chttp2_connector.c @@ -43,6 +43,7 @@ #include "src/core/ext/client_channel/connector.h" #include "src/core/ext/client_channel/http_connect_handshaker.h" +#include "src/core/ext/client_channel/subchannel.h" #include "src/core/ext/transport/chttp2/transport/chttp2_transport.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/handshaker.h" @@ -220,6 +221,11 @@ static void chttp2_connector_connect(grpc_exec_ctx *exec_ctx, grpc_connect_out_args *result, grpc_closure *notify) { chttp2_connector *c = (chttp2_connector *)con; + const grpc_arg *addr_arg = + grpc_channel_args_find(args->channel_args, GRPC_ARG_SUBCHANNEL_ADDRESS); + GPR_ASSERT(addr_arg != NULL); // Should have been set by LB policy. + grpc_resolved_address addr; + grpc_uri_to_sockaddr(addr_arg->value.string, &addr); gpr_mu_lock(&c->mu); GPR_ASSERT(c->notify == NULL); c->notify = notify; @@ -231,8 +237,8 @@ static void chttp2_connector_connect(grpc_exec_ctx *exec_ctx, GPR_ASSERT(!c->connecting); c->connecting = true; grpc_tcp_client_connect(exec_ctx, &c->connected, &c->endpoint, - args->interested_parties, args->channel_args, - args->addr, args->deadline); + args->interested_parties, args->channel_args, &addr, + args->deadline); gpr_mu_unlock(&c->mu); } -- cgit v1.2.3 From df8f12203ce662860d514376af92c76237fd5b7f Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Fri, 13 Jan 2017 22:59:39 +0000 Subject: Fix API fuzzer tests. --- src/core/ext/client_channel/subchannel.c | 35 ++++++++++++++++------ src/core/ext/client_channel/subchannel.h | 9 ++++-- src/core/ext/lb_policy/pick_first/pick_first.c | 7 ++--- src/core/ext/lb_policy/round_robin/round_robin.c | 7 ++--- .../ext/transport/chttp2/client/chttp2_connector.c | 5 +--- 5 files changed, 38 insertions(+), 25 deletions(-) (limited to 'src/core/ext/client_channel') diff --git a/src/core/ext/client_channel/subchannel.c b/src/core/ext/client_channel/subchannel.c index 7a2ad98433..f9de9993b7 100644 --- a/src/core/ext/client_channel/subchannel.c +++ b/src/core/ext/client_channel/subchannel.c @@ -38,6 +38,7 @@ #include #include +#include #include "src/core/ext/client_channel/client_channel.h" #include "src/core/ext/client_channel/initial_connect_string.h" @@ -328,21 +329,16 @@ grpc_subchannel *grpc_subchannel_create(grpc_exec_ctx *exec_ctx, c->filters = NULL; } c->pollset_set = grpc_pollset_set_create(); - const grpc_arg *addr_arg = - grpc_channel_args_find(args->args, GRPC_ARG_SUBCHANNEL_ADDRESS); - GPR_ASSERT(addr_arg != NULL); // Should have been set by LB policy. grpc_resolved_address *addr = gpr_malloc(sizeof(*addr)); - grpc_uri_to_sockaddr(addr_arg->value.string, addr); + grpc_get_subchannel_address_arg(args->args, addr); grpc_set_initial_connect_string(&addr, &c->initial_connect_string); static const char *keys_to_remove[] = {GRPC_ARG_SUBCHANNEL_ADDRESS}; - grpc_arg new_arg; - new_arg.key = GRPC_ARG_SUBCHANNEL_ADDRESS; - new_arg.type = GRPC_ARG_STRING; - new_arg.value.string = grpc_sockaddr_to_uri(addr); + grpc_arg new_arg = grpc_create_subchannel_address_arg(addr); gpr_free(addr); c->args = grpc_channel_args_copy_and_add_and_remove( args->args, keys_to_remove, GPR_ARRAY_SIZE(keys_to_remove), &new_arg, 1); gpr_free(new_arg.value.string); + c->root_external_state_watcher.next = c->root_external_state_watcher.prev = &c->root_external_state_watcher; grpc_closure_init(&c->connected, subchannel_connected, c, @@ -781,7 +777,7 @@ grpc_call_stack *grpc_subchannel_call_get_call_stack( return SUBCHANNEL_CALL_TO_CALL_STACK(subchannel_call); } -void grpc_uri_to_sockaddr(char *uri_str, grpc_resolved_address *addr) { +static void grpc_uri_to_sockaddr(char *uri_str, grpc_resolved_address *addr) { grpc_uri *uri = grpc_uri_parse(uri_str, 0 /* suppress_errors */); GPR_ASSERT(uri != NULL); if (strcmp(uri->scheme, "ipv4") == 0) { @@ -793,3 +789,24 @@ void grpc_uri_to_sockaddr(char *uri_str, grpc_resolved_address *addr) { } grpc_uri_destroy(uri); } + +void grpc_get_subchannel_address_arg(const grpc_channel_args *args, + grpc_resolved_address *addr) { + const grpc_arg *addr_arg = + grpc_channel_args_find(args, GRPC_ARG_SUBCHANNEL_ADDRESS); + GPR_ASSERT(addr_arg != NULL); // Should have been set by LB policy. + GPR_ASSERT(addr_arg->type == GRPC_ARG_STRING); + memset(addr, 0, sizeof(*addr)); + if (*addr_arg->value.string != '\0') { + grpc_uri_to_sockaddr(addr_arg->value.string, addr); + } +} + +grpc_arg grpc_create_subchannel_address_arg(const grpc_resolved_address* addr) { + grpc_arg new_arg; + new_arg.key = GRPC_ARG_SUBCHANNEL_ADDRESS; + new_arg.type = GRPC_ARG_STRING; + new_arg.value.string = + addr->len > 0 ? grpc_sockaddr_to_uri(addr) : gpr_strdup(""); + return new_arg; +} diff --git a/src/core/ext/client_channel/subchannel.h b/src/core/ext/client_channel/subchannel.h index 4cc33fc79b..c7a9577ce9 100644 --- a/src/core/ext/client_channel/subchannel.h +++ b/src/core/ext/client_channel/subchannel.h @@ -174,7 +174,12 @@ grpc_subchannel *grpc_subchannel_create(grpc_exec_ctx *exec_ctx, grpc_connector *connector, const grpc_subchannel_args *args); -/// Sets \a addr from \a uri_str. -void grpc_uri_to_sockaddr(char *uri_str, grpc_resolved_address *addr); +/// Sets \a addr from \a args. +void grpc_get_subchannel_address_arg(const grpc_channel_args *args, + grpc_resolved_address *addr); + +/// Returns a new channel arg encoding the subchannel address as a string. +/// Caller is responsible for freeing the string. +grpc_arg grpc_create_subchannel_address_arg(const grpc_resolved_address* addr); #endif /* GRPC_CORE_EXT_CLIENT_CHANNEL_SUBCHANNEL_H */ diff --git a/src/core/ext/lb_policy/pick_first/pick_first.c b/src/core/ext/lb_policy/pick_first/pick_first.c index 4677fb2343..9f2aa461be 100644 --- a/src/core/ext/lb_policy/pick_first/pick_first.c +++ b/src/core/ext/lb_policy/pick_first/pick_first.c @@ -468,11 +468,8 @@ static grpc_lb_policy *create_pick_first(grpc_exec_ctx *exec_ctx, } memset(&sc_args, 0, sizeof(grpc_subchannel_args)); - grpc_arg addr_arg; - addr_arg.key = GRPC_ARG_SUBCHANNEL_ADDRESS; - addr_arg.type = GRPC_ARG_STRING; - addr_arg.value.string = - grpc_sockaddr_to_uri(&addresses->addresses[i].address); + grpc_arg addr_arg = + grpc_create_subchannel_address_arg(&addresses->addresses[i].address); grpc_channel_args *new_args = grpc_channel_args_copy_and_add(args->args, &addr_arg, 1); gpr_free(addr_arg.value.string); diff --git a/src/core/ext/lb_policy/round_robin/round_robin.c b/src/core/ext/lb_policy/round_robin/round_robin.c index 3fe157a23c..d17d8fa057 100644 --- a/src/core/ext/lb_policy/round_robin/round_robin.c +++ b/src/core/ext/lb_policy/round_robin/round_robin.c @@ -731,11 +731,8 @@ static grpc_lb_policy *round_robin_create(grpc_exec_ctx *exec_ctx, if (addresses->addresses[i].is_balancer) continue; memset(&sc_args, 0, sizeof(grpc_subchannel_args)); - grpc_arg addr_arg; - addr_arg.key = GRPC_ARG_SUBCHANNEL_ADDRESS; - addr_arg.type = GRPC_ARG_STRING; - addr_arg.value.string = - grpc_sockaddr_to_uri(&addresses->addresses[i].address); + grpc_arg addr_arg = + grpc_create_subchannel_address_arg(&addresses->addresses[i].address); grpc_channel_args *new_args = grpc_channel_args_copy_and_add(args->args, &addr_arg, 1); gpr_free(addr_arg.value.string); diff --git a/src/core/ext/transport/chttp2/client/chttp2_connector.c b/src/core/ext/transport/chttp2/client/chttp2_connector.c index ebe884b115..013c96dc70 100644 --- a/src/core/ext/transport/chttp2/client/chttp2_connector.c +++ b/src/core/ext/transport/chttp2/client/chttp2_connector.c @@ -221,11 +221,8 @@ static void chttp2_connector_connect(grpc_exec_ctx *exec_ctx, grpc_connect_out_args *result, grpc_closure *notify) { chttp2_connector *c = (chttp2_connector *)con; - const grpc_arg *addr_arg = - grpc_channel_args_find(args->channel_args, GRPC_ARG_SUBCHANNEL_ADDRESS); - GPR_ASSERT(addr_arg != NULL); // Should have been set by LB policy. grpc_resolved_address addr; - grpc_uri_to_sockaddr(addr_arg->value.string, &addr); + grpc_get_subchannel_address_arg(args->channel_args, &addr); gpr_mu_lock(&c->mu); GPR_ASSERT(c->notify == NULL); c->notify = notify; -- cgit v1.2.3 From 31f2dd43e22f41c89cfdd3d25002972247d01954 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Tue, 17 Jan 2017 07:25:06 -0800 Subject: clang-format --- src/core/ext/client_channel/subchannel.c | 2 +- src/core/ext/client_channel/subchannel.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'src/core/ext/client_channel') diff --git a/src/core/ext/client_channel/subchannel.c b/src/core/ext/client_channel/subchannel.c index f9de9993b7..8bd284507d 100644 --- a/src/core/ext/client_channel/subchannel.c +++ b/src/core/ext/client_channel/subchannel.c @@ -802,7 +802,7 @@ void grpc_get_subchannel_address_arg(const grpc_channel_args *args, } } -grpc_arg grpc_create_subchannel_address_arg(const grpc_resolved_address* addr) { +grpc_arg grpc_create_subchannel_address_arg(const grpc_resolved_address *addr) { grpc_arg new_arg; new_arg.key = GRPC_ARG_SUBCHANNEL_ADDRESS; new_arg.type = GRPC_ARG_STRING; diff --git a/src/core/ext/client_channel/subchannel.h b/src/core/ext/client_channel/subchannel.h index c7a9577ce9..684675eb37 100644 --- a/src/core/ext/client_channel/subchannel.h +++ b/src/core/ext/client_channel/subchannel.h @@ -180,6 +180,6 @@ void grpc_get_subchannel_address_arg(const grpc_channel_args *args, /// Returns a new channel arg encoding the subchannel address as a string. /// Caller is responsible for freeing the string. -grpc_arg grpc_create_subchannel_address_arg(const grpc_resolved_address* addr); +grpc_arg grpc_create_subchannel_address_arg(const grpc_resolved_address *addr); #endif /* GRPC_CORE_EXT_CLIENT_CHANNEL_SUBCHANNEL_H */ -- cgit v1.2.3 From d0f7bf4a76be6e0f8d8212c8ce91d054f478f29c Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Wed, 18 Jan 2017 11:49:22 -0800 Subject: Document new args in grpc_resolver_create --- src/core/ext/client_channel/resolver_registry.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src/core/ext/client_channel') diff --git a/src/core/ext/client_channel/resolver_registry.h b/src/core/ext/client_channel/resolver_registry.h index 4fb16131db..a4606463eb 100644 --- a/src/core/ext/client_channel/resolver_registry.h +++ b/src/core/ext/client_channel/resolver_registry.h @@ -60,7 +60,9 @@ void grpc_register_resolver_type(grpc_resolver_factory *factory); return it. If a resolver factory was not found, return NULL. \a args is a set of channel arguments to be included in the result - (typically the set of arguments passed in from the client API). */ + (typically the set of arguments passed in from the client API). + \a pollset_set is used to drive IO in the name resolution process, it + should not be NULL. */ grpc_resolver *grpc_resolver_create(grpc_exec_ctx *exec_ctx, const char *target, const grpc_channel_args *args, grpc_pollset_set *pollset_set); -- cgit v1.2.3