diff options
| -rw-r--r-- | src/core/ext/filters/client_channel/client_channel.cc | 9 | ||||
| -rw-r--r-- | src/core/ext/filters/message_size/message_size_filter.cc | 60 | ||||
| -rw-r--r-- | src/core/lib/transport/service_config.cc | 32 | ||||
| -rw-r--r-- | src/core/lib/transport/service_config.h | 5 | ||||
| -rw-r--r-- | test/core/end2end/tests/cancel_after_accept.c | 3 | ||||
| -rw-r--r-- | test/core/end2end/tests/max_message_length.c | 3 |
6 files changed, 80 insertions, 32 deletions
diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index ea5e076c3b..49a87c77b9 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -87,7 +87,12 @@ static void method_parameters_unref(method_parameters *method_params) { } } -static void method_parameters_free(grpc_exec_ctx *exec_ctx, void *value) { +// Wrappers to pass to grpc_service_config_create_method_config_table(). +static void *method_parameters_ref_wrapper(void *value) { + return method_parameters_ref((method_parameters *)value); +} +static void method_parameters_unref_wrapper(grpc_exec_ctx *exec_ctx, + void *value) { method_parameters_unref((method_parameters *)value); } @@ -472,7 +477,7 @@ static void on_resolver_result_changed_locked(grpc_exec_ctx *exec_ctx, retry_throttle_data = parsing_state.retry_throttle_data; method_params_table = grpc_service_config_create_method_config_table( exec_ctx, service_config, method_parameters_create_from_json, - method_parameters_free); + method_parameters_ref_wrapper, method_parameters_unref_wrapper); grpc_service_config_destroy(service_config); } } diff --git a/src/core/ext/filters/message_size/message_size_filter.cc b/src/core/ext/filters/message_size/message_size_filter.cc index 5dc131b9f6..9376d978b4 100644 --- a/src/core/ext/filters/message_size/message_size_filter.cc +++ b/src/core/ext/filters/message_size/message_size_filter.cc @@ -30,16 +30,34 @@ #include "src/core/lib/surface/channel_init.h" #include "src/core/lib/transport/service_config.h" -typedef struct message_size_limits { +typedef struct { int max_send_size; int max_recv_size; } message_size_limits; -static void message_size_limits_free(grpc_exec_ctx* exec_ctx, void* value) { - gpr_free(value); +typedef struct { + gpr_refcount refs; + message_size_limits limits; +} refcounted_message_size_limits; + +static void* refcounted_message_size_limits_ref(void* value) { + refcounted_message_size_limits* limits = + (refcounted_message_size_limits*)value; + gpr_ref(&limits->refs); + return value; +} + +static void refcounted_message_size_limits_unref(grpc_exec_ctx* exec_ctx, + void* value) { + refcounted_message_size_limits* limits = + (refcounted_message_size_limits*)value; + if (gpr_unref(&limits->refs)) { + gpr_free(value); + } } -static void* message_size_limits_create_from_json(const grpc_json* json) { +static void* refcounted_message_size_limits_create_from_json( + const grpc_json* json) { int max_request_message_bytes = -1; int max_response_message_bytes = -1; for (grpc_json* field = json->child; field != NULL; field = field->next) { @@ -60,10 +78,12 @@ static void* message_size_limits_create_from_json(const grpc_json* json) { if (max_response_message_bytes == -1) return NULL; } } - message_size_limits* value = - (message_size_limits*)gpr_malloc(sizeof(message_size_limits)); - value->max_send_size = max_request_message_bytes; - value->max_recv_size = max_response_message_bytes; + refcounted_message_size_limits* value = + (refcounted_message_size_limits*)gpr_malloc( + sizeof(refcounted_message_size_limits)); + gpr_ref_init(&value->refs, 1); + value->limits.max_send_size = max_request_message_bytes; + value->limits.max_recv_size = max_response_message_bytes; return value; } @@ -82,7 +102,7 @@ typedef struct call_data { typedef struct channel_data { message_size_limits limits; - // Maps path names to message_size_limits structs. + // Maps path names to refcounted_message_size_limits structs. grpc_slice_hash_table* method_limit_table; } channel_data; @@ -164,19 +184,19 @@ static grpc_error* init_call_elem(grpc_exec_ctx* exec_ctx, // size to the receive limit. calld->limits = chand->limits; if (chand->method_limit_table != NULL) { - message_size_limits* limits = - (message_size_limits*)grpc_method_config_table_get( + refcounted_message_size_limits* limits = + (refcounted_message_size_limits*)grpc_method_config_table_get( exec_ctx, chand->method_limit_table, args->path); if (limits != NULL) { - if (limits->max_send_size >= 0 && - (limits->max_send_size < calld->limits.max_send_size || + if (limits->limits.max_send_size >= 0 && + (limits->limits.max_send_size < calld->limits.max_send_size || calld->limits.max_send_size < 0)) { - calld->limits.max_send_size = limits->max_send_size; + calld->limits.max_send_size = limits->limits.max_send_size; } - if (limits->max_recv_size >= 0 && - (limits->max_recv_size < calld->limits.max_recv_size || + if (limits->limits.max_recv_size >= 0 && + (limits->limits.max_recv_size < calld->limits.max_recv_size || calld->limits.max_recv_size < 0)) { - calld->limits.max_recv_size = limits->max_recv_size; + calld->limits.max_recv_size = limits->limits.max_recv_size; } } } @@ -237,8 +257,10 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, if (service_config != NULL) { chand->method_limit_table = grpc_service_config_create_method_config_table( - exec_ctx, service_config, message_size_limits_create_from_json, - message_size_limits_free); + exec_ctx, service_config, + refcounted_message_size_limits_create_from_json, + refcounted_message_size_limits_ref, + refcounted_message_size_limits_unref); grpc_service_config_destroy(service_config); } } diff --git a/src/core/lib/transport/service_config.cc b/src/core/lib/transport/service_config.cc index 070a13a2b4..05907de7d7 100644 --- a/src/core/lib/transport/service_config.cc +++ b/src/core/lib/transport/service_config.cc @@ -111,7 +111,13 @@ const char* grpc_service_config_get_lb_policy_name( static size_t count_names_in_method_config_json(grpc_json* json) { size_t num_names = 0; for (grpc_json* field = json->child; field != NULL; field = field->next) { - if (field->key != NULL && strcmp(field->key, "name") == 0) ++num_names; + if (field->key != NULL && strcmp(field->key, "name") == 0) { + if (field->type != GRPC_JSON_ARRAY) return -1; + for (grpc_json* name = field->child; name != NULL; name = name->next) { + if (name->type != GRPC_JSON_OBJECT) return -1; + ++num_names; + } + } } return num_names; } @@ -148,6 +154,8 @@ static char* parse_json_method_name(grpc_json* json) { static bool parse_json_method_config( grpc_exec_ctx* exec_ctx, grpc_json* json, void* (*create_value)(const grpc_json* method_config_json), + void* (*ref_value)(void* value), + void (*unref_value)(grpc_exec_ctx* exec_ctx, void* value), grpc_slice_hash_table_entry* entries, size_t* idx) { // Construct value. void* method_config = create_value(json); @@ -162,6 +170,7 @@ static bool parse_json_method_config( if (child->type != GRPC_JSON_ARRAY) goto done; for (grpc_json* name = child->child; name != NULL; name = name->next) { char* path = parse_json_method_name(name); + if (path == NULL) goto done; gpr_strvec_add(&paths, path); } } @@ -170,11 +179,12 @@ static bool parse_json_method_config( // Add entry for each path. for (size_t i = 0; i < paths.count; ++i) { entries[*idx].key = grpc_slice_from_copied_string(paths.strs[i]); - entries[*idx].value = method_config; + entries[*idx].value = ref_value(method_config); ++*idx; } success = true; done: + unref_value(exec_ctx, method_config); gpr_strvec_destroy(&paths); return success; } @@ -182,7 +192,8 @@ done: grpc_slice_hash_table* grpc_service_config_create_method_config_table( grpc_exec_ctx* exec_ctx, const grpc_service_config* service_config, void* (*create_value)(const grpc_json* method_config_json), - void (*destroy_value)(grpc_exec_ctx* exec_ctx, void* value)) { + void* (*ref_value)(void* value), + void (*unref_value)(grpc_exec_ctx* exec_ctx, void* value)) { const grpc_json* json = service_config->json_tree; // Traverse parsed JSON tree. if (json->type != GRPC_JSON_OBJECT || json->key != NULL) return NULL; @@ -196,7 +207,9 @@ grpc_slice_hash_table* grpc_service_config_create_method_config_table( // Find number of entries. for (grpc_json* method = field->child; method != NULL; method = method->next) { - num_entries += count_names_in_method_config_json(method); + size_t count = count_names_in_method_config_json(method); + if (count <= 0) return NULL; + num_entries += count; } // Populate method config table entries. entries = (grpc_slice_hash_table_entry*)gpr_malloc( @@ -204,8 +217,13 @@ grpc_slice_hash_table* grpc_service_config_create_method_config_table( size_t idx = 0; for (grpc_json* method = field->child; method != NULL; method = method->next) { - if (!parse_json_method_config(exec_ctx, method, create_value, entries, - &idx)) { + if (!parse_json_method_config(exec_ctx, method, create_value, ref_value, + unref_value, entries, &idx)) { + for (size_t i = 0; i < idx; ++i) { + grpc_slice_unref_internal(exec_ctx, entries[i].key); + unref_value(exec_ctx, entries[i].value); + } + gpr_free(entries); return NULL; } } @@ -216,7 +234,7 @@ grpc_slice_hash_table* grpc_service_config_create_method_config_table( grpc_slice_hash_table* method_config_table = NULL; if (entries != NULL) { method_config_table = - grpc_slice_hash_table_create(num_entries, entries, destroy_value, NULL); + grpc_slice_hash_table_create(num_entries, entries, unref_value, NULL); gpr_free(entries); } return method_config_table; diff --git a/src/core/lib/transport/service_config.h b/src/core/lib/transport/service_config.h index 9c43093627..405d0f5b41 100644 --- a/src/core/lib/transport/service_config.h +++ b/src/core/lib/transport/service_config.h @@ -46,12 +46,13 @@ const char* grpc_service_config_get_lb_policy_name( /// Creates a method config table based on the data in \a json. /// The table's keys are request paths. The table's value type is /// returned by \a create_value(), based on data parsed from the JSON tree. -/// \a destroy_value is used to clean up values. +/// \a ref_value() and \a unref_value() are used to ref and unref values. /// Returns NULL on error. grpc_slice_hash_table* grpc_service_config_create_method_config_table( grpc_exec_ctx* exec_ctx, const grpc_service_config* service_config, void* (*create_value)(const grpc_json* method_config_json), - void (*destroy_value)(grpc_exec_ctx* exec_ctx, void* value)); + void* (*ref_value)(void* value), + void (*unref_value)(grpc_exec_ctx* exec_ctx, void* value)); /// A helper function for looking up values in the table returned by /// \a grpc_service_config_create_method_config_table(). diff --git a/test/core/end2end/tests/cancel_after_accept.c b/test/core/end2end/tests/cancel_after_accept.c index c3ac0c3201..96262b1156 100644 --- a/test/core/end2end/tests/cancel_after_accept.c +++ b/test/core/end2end/tests/cancel_after_accept.c @@ -130,7 +130,8 @@ static void test_cancel_after_accept(grpc_end2end_test_config config, "{\n" " \"methodConfig\": [ {\n" " \"name\": [\n" - " { \"service\": \"service\", \"method\": \"method\" }\n" + " { \"service\": \"service\", \"method\": \"method\" },\n" + " { \"service\": \"unused\" }\n" " ],\n" " \"timeout\": \"5s\"\n" " } ]\n" diff --git a/test/core/end2end/tests/max_message_length.c b/test/core/end2end/tests/max_message_length.c index 01eb8d365e..8925de9fe4 100644 --- a/test/core/end2end/tests/max_message_length.c +++ b/test/core/end2end/tests/max_message_length.c @@ -138,7 +138,8 @@ static void test_max_message_length_on_request(grpc_end2end_test_config config, ? "{\n" " \"methodConfig\": [ {\n" " \"name\": [\n" - " { \"service\": \"service\", \"method\": \"method\" }\n" + " { \"service\": \"service\", \"method\": \"method\" },\n" + " { \"service\": \"unused\" }\n" " ],\n" " \"maxRequestMessageBytes\": \"5\"\n" " } ]\n" |