From 0badbe8b119c7375dfb0f8e6f853180009a2ffb7 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 23 Jun 2016 10:15:12 -0700 Subject: [PATCH 01/16] Change grpc_channel_filter init_call_elem() method to return grpc_error. --- src/core/ext/census/grpc_filter.c | 14 ++++++++------ src/core/ext/client_config/client_channel.c | 6 ++++-- src/core/ext/client_config/subchannel.c | 7 +++++-- .../load_reporting/load_reporting_filter.c | 6 ++++-- src/core/lib/channel/channel_stack.c | 19 ++++++++++++------- src/core/lib/channel/channel_stack.h | 18 ++++++++++-------- src/core/lib/channel/compress_filter.c | 7 +++++-- src/core/lib/channel/connected_channel.c | 13 +++++++------ src/core/lib/channel/http_client_filter.c | 6 ++++-- src/core/lib/channel/http_server_filter.c | 6 ++++-- .../security/transport/client_auth_filter.c | 6 ++++-- .../security/transport/server_auth_filter.c | 7 +++++-- src/core/lib/surface/call.c | 9 ++++++--- src/core/lib/surface/lame_client.c | 7 +++++-- src/core/lib/surface/server.c | 6 ++++-- test/core/channel/channel_stack_test.c | 12 ++++++++---- test/core/end2end/tests/filter_causes_close.c | 7 +++++-- 17 files changed, 100 insertions(+), 56 deletions(-) diff --git a/src/core/ext/census/grpc_filter.c b/src/core/ext/census/grpc_filter.c index 72e4e5427eb..dd2e2e01244 100644 --- a/src/core/ext/census/grpc_filter.c +++ b/src/core/ext/census/grpc_filter.c @@ -124,13 +124,14 @@ static void server_start_transport_op(grpc_exec_ctx *exec_ctx, grpc_call_next_op(exec_ctx, elem, op); } -static void client_init_call_elem(grpc_exec_ctx *exec_ctx, - grpc_call_element *elem, - grpc_call_element_args *args) { +static grpc_error* client_init_call_elem(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, + grpc_call_element_args *args) { call_data *d = elem->call_data; GPR_ASSERT(d != NULL); memset(d, 0, sizeof(*d)); d->start_ts = gpr_now(GPR_CLOCK_REALTIME); + return GRPC_ERROR_NONE; } static void client_destroy_call_elem(grpc_exec_ctx *exec_ctx, @@ -142,15 +143,16 @@ static void client_destroy_call_elem(grpc_exec_ctx *exec_ctx, /* TODO(hongyu): record rpc client stats and census_rpc_end_op here */ } -static void server_init_call_elem(grpc_exec_ctx *exec_ctx, - grpc_call_element *elem, - grpc_call_element_args *args) { +static grpc_error* server_init_call_elem(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, + grpc_call_element_args *args) { call_data *d = elem->call_data; GPR_ASSERT(d != NULL); memset(d, 0, sizeof(*d)); d->start_ts = gpr_now(GPR_CLOCK_REALTIME); /* TODO(hongyu): call census_tracing_start_op here. */ grpc_closure_init(&d->finish_recv, server_on_done_recv, elem); + return GRPC_ERROR_NONE; } static void server_destroy_call_elem(grpc_exec_ctx *exec_ctx, diff --git a/src/core/ext/client_config/client_channel.c b/src/core/ext/client_config/client_channel.c index 1d5a7d52246..b0a09dab932 100644 --- a/src/core/ext/client_config/client_channel.c +++ b/src/core/ext/client_config/client_channel.c @@ -430,10 +430,12 @@ static int cc_pick_subchannel(grpc_exec_ctx *exec_ctx, void *elemp, } /* Constructor for call_data */ -static void init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, - grpc_call_element_args *args) { +static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, + grpc_call_element_args *args) { grpc_subchannel_call_holder_init(elem->call_data, cc_pick_subchannel, elem, args->call_stack); + return GRPC_ERROR_NONE; } /* Destructor for call_data */ diff --git a/src/core/ext/client_config/subchannel.c b/src/core/ext/client_config/subchannel.c index 468067ea57c..2e9cf7d7d4e 100644 --- a/src/core/ext/client_config/subchannel.c +++ b/src/core/ext/client_config/subchannel.c @@ -709,8 +709,11 @@ grpc_subchannel_call *grpc_connected_subchannel_create_call( grpc_call_stack *callstk = SUBCHANNEL_CALL_TO_CALL_STACK(call); call->connection = con; GRPC_CONNECTED_SUBCHANNEL_REF(con, "subchannel_call"); - grpc_call_stack_init(exec_ctx, chanstk, 1, subchannel_call_destroy, call, - NULL, NULL, callstk); + grpc_error* error = grpc_call_stack_init(exec_ctx, chanstk, 1, + subchannel_call_destroy, call, + NULL, NULL, callstk); +// FIXME: handle error (probably requires changing this function's API) +GPR_ASSERT(error == GRPC_ERROR_NONE); grpc_call_stack_set_pollset_or_pollset_set(exec_ctx, callstk, pollent); return call; } diff --git a/src/core/ext/load_reporting/load_reporting_filter.c b/src/core/ext/load_reporting/load_reporting_filter.c index f372f88c3a6..cfe752d6e2b 100644 --- a/src/core/ext/load_reporting/load_reporting_filter.c +++ b/src/core/ext/load_reporting/load_reporting_filter.c @@ -56,10 +56,12 @@ static void invoke_lr_fn_locked(grpc_load_reporting_config *lrc, } /* Constructor for call_data */ -static void init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, - grpc_call_element_args *args) { +static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, + grpc_call_element_args *args) { call_data *calld = elem->call_data; memset(calld, 0, sizeof(call_data)); + return GRPC_ERROR_NONE; } /* Destructor for call_data */ diff --git a/src/core/lib/channel/channel_stack.c b/src/core/lib/channel/channel_stack.c index bbba85d80ba..0613f94c299 100644 --- a/src/core/lib/channel/channel_stack.c +++ b/src/core/lib/channel/channel_stack.c @@ -157,12 +157,13 @@ void grpc_channel_stack_destroy(grpc_exec_ctx *exec_ctx, } } -void grpc_call_stack_init(grpc_exec_ctx *exec_ctx, - grpc_channel_stack *channel_stack, int initial_refs, - grpc_iomgr_cb_func destroy, void *destroy_arg, - grpc_call_context_element *context, - const void *transport_server_data, - grpc_call_stack *call_stack) { +grpc_error* grpc_call_stack_init(grpc_exec_ctx *exec_ctx, + grpc_channel_stack *channel_stack, + int initial_refs, grpc_iomgr_cb_func destroy, + void *destroy_arg, + grpc_call_context_element *context, + const void *transport_server_data, + grpc_call_stack *call_stack) { grpc_channel_element *channel_elems = CHANNEL_ELEMS_FROM_STACK(channel_stack); grpc_call_element_args args; size_t count = channel_stack->count; @@ -185,10 +186,14 @@ void grpc_call_stack_init(grpc_exec_ctx *exec_ctx, call_elems[i].filter = channel_elems[i].filter; call_elems[i].channel_data = channel_elems[i].channel_data; call_elems[i].call_data = user_data; - call_elems[i].filter->init_call_elem(exec_ctx, &call_elems[i], &args); + grpc_error* error = call_elems[i].filter->init_call_elem( + exec_ctx, &call_elems[i], &args); + if (error != GRPC_ERROR_NONE) + return error; user_data += ROUND_UP_TO_ALIGNMENT_SIZE(call_elems[i].filter->sizeof_call_data); } + return GRPC_ERROR_NONE; } void grpc_call_stack_set_pollset_or_pollset_set(grpc_exec_ctx *exec_ctx, diff --git a/src/core/lib/channel/channel_stack.h b/src/core/lib/channel/channel_stack.h index 41dd4a0d8a1..f56c6540674 100644 --- a/src/core/lib/channel/channel_stack.h +++ b/src/core/lib/channel/channel_stack.h @@ -110,8 +110,9 @@ typedef struct { on a client; if it is non-NULL, then it points to memory owned by the transport and is on the server. Most filters want to ignore this argument. */ - void (*init_call_elem)(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, - grpc_call_element_args *args); + grpc_error* (*init_call_elem)(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, + grpc_call_element_args *args); void (*set_pollset_or_pollset_set)(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_polling_entity *pollent); @@ -209,12 +210,13 @@ void grpc_channel_stack_destroy(grpc_exec_ctx *exec_ctx, /* Initialize a call stack given a channel stack. transport_server_data is expected to be NULL on a client, or an opaque transport owned pointer on the server. */ -void grpc_call_stack_init(grpc_exec_ctx *exec_ctx, - grpc_channel_stack *channel_stack, int initial_refs, - grpc_iomgr_cb_func destroy, void *destroy_arg, - grpc_call_context_element *context, - const void *transport_server_data, - grpc_call_stack *call_stack); +grpc_error* grpc_call_stack_init(grpc_exec_ctx *exec_ctx, + grpc_channel_stack *channel_stack, + int initial_refs, grpc_iomgr_cb_func destroy, + void *destroy_arg, + grpc_call_context_element *context, + const void *transport_server_data, + grpc_call_stack *call_stack); /* Set a pollset or a pollset_set for a call stack: must occur before the first * op is started */ void grpc_call_stack_set_pollset_or_pollset_set(grpc_exec_ctx *exec_ctx, diff --git a/src/core/lib/channel/compress_filter.c b/src/core/lib/channel/compress_filter.c index 32ebe53ee64..bcc8e17a9db 100644 --- a/src/core/lib/channel/compress_filter.c +++ b/src/core/lib/channel/compress_filter.c @@ -256,8 +256,9 @@ static void compress_start_transport_stream_op(grpc_exec_ctx *exec_ctx, } /* Constructor for call_data */ -static void init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, - grpc_call_element_args *args) { +static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, + grpc_call_element_args *args) { /* grab pointers to our data from the call element */ call_data *calld = elem->call_data; @@ -266,6 +267,8 @@ static void init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, calld->has_compression_algorithm = 0; grpc_closure_init(&calld->got_slice, got_slice, elem); grpc_closure_init(&calld->send_done, send_done, elem); + + return GRPC_ERROR_NONE; } /* Destructor for call_data */ diff --git a/src/core/lib/channel/connected_channel.c b/src/core/lib/channel/connected_channel.c index 0a7d27a1dc5..8dde688df2c 100644 --- a/src/core/lib/channel/connected_channel.c +++ b/src/core/lib/channel/connected_channel.c @@ -81,16 +81,17 @@ static void con_start_transport_op(grpc_exec_ctx *exec_ctx, } /* Constructor for call_data */ -static void init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, - grpc_call_element_args *args) { +static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, + grpc_call_element_args *args) { call_data *calld = elem->call_data; channel_data *chand = elem->channel_data; - int r; - - r = grpc_transport_init_stream( + int r = grpc_transport_init_stream( exec_ctx, chand->transport, TRANSPORT_STREAM_FROM_CALL_DATA(calld), &args->call_stack->refcount, args->server_transport_data); - GPR_ASSERT(r == 0); + return r == 0 + ? GRPC_ERROR_NONE + : GRPC_ERROR_CREATE("transport initialization failed"); } static void set_pollset_or_pollset_set(grpc_exec_ctx *exec_ctx, diff --git a/src/core/lib/channel/http_client_filter.c b/src/core/lib/channel/http_client_filter.c index ab6c6c9ef00..30f4ed1f6fc 100644 --- a/src/core/lib/channel/http_client_filter.c +++ b/src/core/lib/channel/http_client_filter.c @@ -169,11 +169,13 @@ static void hc_start_transport_op(grpc_exec_ctx *exec_ctx, } /* Constructor for call_data */ -static void init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, - grpc_call_element_args *args) { +static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, + grpc_call_element_args *args) { call_data *calld = elem->call_data; calld->on_done_recv = NULL; grpc_closure_init(&calld->hc_on_recv, hc_on_recv, elem); + return GRPC_ERROR_NONE; } /* Destructor for call_data */ diff --git a/src/core/lib/channel/http_server_filter.c b/src/core/lib/channel/http_server_filter.c index d0beebd817f..960d1ce45e5 100644 --- a/src/core/lib/channel/http_server_filter.c +++ b/src/core/lib/channel/http_server_filter.c @@ -224,13 +224,15 @@ static void hs_start_transport_op(grpc_exec_ctx *exec_ctx, } /* Constructor for call_data */ -static void init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, - grpc_call_element_args *args) { +static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, + grpc_call_element_args *args) { /* grab pointers to our data from the call element */ call_data *calld = elem->call_data; /* initialize members */ memset(calld, 0, sizeof(*calld)); grpc_closure_init(&calld->hs_on_recv, hs_on_recv, elem); + return GRPC_ERROR_NONE; } /* Destructor for call_data */ diff --git a/src/core/lib/security/transport/client_auth_filter.c b/src/core/lib/security/transport/client_auth_filter.c index 76be2acd728..6179b9e18cf 100644 --- a/src/core/lib/security/transport/client_auth_filter.c +++ b/src/core/lib/security/transport/client_auth_filter.c @@ -264,10 +264,12 @@ static void auth_start_transport_op(grpc_exec_ctx *exec_ctx, } /* Constructor for call_data */ -static void init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, - grpc_call_element_args *args) { +static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, + grpc_call_element_args *args) { call_data *calld = elem->call_data; memset(calld, 0, sizeof(*calld)); + return GRPC_ERROR_NONE; } static void set_pollset_or_pollset_set(grpc_exec_ctx *exec_ctx, diff --git a/src/core/lib/security/transport/server_auth_filter.c b/src/core/lib/security/transport/server_auth_filter.c index 12e789bde92..379247131b6 100644 --- a/src/core/lib/security/transport/server_auth_filter.c +++ b/src/core/lib/security/transport/server_auth_filter.c @@ -199,8 +199,9 @@ static void auth_start_transport_op(grpc_exec_ctx *exec_ctx, } /* Constructor for call_data */ -static void init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, - grpc_call_element_args *args) { +static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, + grpc_call_element_args *args) { /* grab pointers to our data from the call element */ call_data *calld = elem->call_data; channel_data *chand = elem->channel_data; @@ -222,6 +223,8 @@ static void init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, args->context[GRPC_CONTEXT_SECURITY].value = server_ctx; args->context[GRPC_CONTEXT_SECURITY].destroy = grpc_server_security_context_destroy; + + return GRPC_ERROR_NONE; } /* Destructor for call_data */ diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c index 04291b0ee0a..d64ca64a15f 100644 --- a/src/core/lib/surface/call.c +++ b/src/core/lib/surface/call.c @@ -262,9 +262,12 @@ grpc_call *grpc_call_create( call->send_deadline = send_deadline; GRPC_CHANNEL_INTERNAL_REF(channel, "call"); /* initial refcount dropped by grpc_call_destroy */ - grpc_call_stack_init(&exec_ctx, channel_stack, 1, destroy_call, call, - call->context, server_transport_data, - CALL_STACK_FROM_CALL(call)); + grpc_error* error = grpc_call_stack_init(&exec_ctx, channel_stack, 1, + destroy_call, call, call->context, + server_transport_data, + CALL_STACK_FROM_CALL(call)); +// FIXME: handle error (probably requires changing this function's API) +GPR_ASSERT(error == GRPC_ERROR_NONE); if (cq != NULL) { GPR_ASSERT( pollset_set_alternative == NULL && diff --git a/src/core/lib/surface/lame_client.c b/src/core/lib/surface/lame_client.c index 5ea4cba5d1a..7d9168f2e0b 100644 --- a/src/core/lib/surface/lame_client.c +++ b/src/core/lib/surface/lame_client.c @@ -107,8 +107,11 @@ static void lame_start_transport_op(grpc_exec_ctx *exec_ctx, GRPC_ERROR_UNREF(op->disconnect_with_error); } -static void init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, - grpc_call_element_args *args) {} +static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, + grpc_call_element_args *args) { + return GRPC_ERROR_NONE; +} static void destroy_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, const grpc_call_stats *stats, diff --git a/src/core/lib/surface/server.c b/src/core/lib/surface/server.c index def6e5068b0..0a0c224b4d6 100644 --- a/src/core/lib/surface/server.c +++ b/src/core/lib/surface/server.c @@ -848,8 +848,9 @@ static void channel_connectivity_changed(grpc_exec_ctx *exec_ctx, void *cd, } } -static void init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, - grpc_call_element_args *args) { +static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, + grpc_call_element_args *args) { call_data *calld = elem->call_data; channel_data *chand = elem->channel_data; memset(calld, 0, sizeof(call_data)); @@ -861,6 +862,7 @@ static void init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, server_on_recv_initial_metadata, elem); server_ref(chand->server); + return GRPC_ERROR_NONE; } static void destroy_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, diff --git a/test/core/channel/channel_stack_test.c b/test/core/channel/channel_stack_test.c index f9561bed707..9c6a47eb520 100644 --- a/test/core/channel/channel_stack_test.c +++ b/test/core/channel/channel_stack_test.c @@ -53,10 +53,12 @@ static void channel_init_func(grpc_exec_ctx *exec_ctx, *(int *)(elem->channel_data) = 0; } -static void call_init_func(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, - grpc_call_element_args *args) { +static grpc_error* call_init_func(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, + grpc_call_element_args *args) { ++*(int *)(elem->channel_data); *(int *)(elem->call_data) = 0; + return GRPC_ERROR_NONE; } static void channel_destroy_func(grpc_exec_ctx *exec_ctx, @@ -132,8 +134,10 @@ static void test_create_channel_stack(void) { GPR_ASSERT(*channel_data == 0); call_stack = gpr_malloc(channel_stack->call_stack_size); - grpc_call_stack_init(&exec_ctx, channel_stack, 1, free_call, call_stack, NULL, - NULL, call_stack); + grpc_error* error = grpc_call_stack_init(&exec_ctx, channel_stack, 1, + free_call, call_stack, NULL, NULL, + call_stack); + GPR_ASSERT(error == GRPC_ERROR_NONE); GPR_ASSERT(call_stack->count == 1); call_elem = grpc_call_stack_element(call_stack, 0); GPR_ASSERT(call_elem->filter == channel_elem->filter); diff --git a/test/core/end2end/tests/filter_causes_close.c b/test/core/end2end/tests/filter_causes_close.c index 526c05ca3e8..3ff0abed637 100644 --- a/test/core/end2end/tests/filter_causes_close.c +++ b/test/core/end2end/tests/filter_causes_close.c @@ -233,8 +233,11 @@ static void start_transport_stream_op(grpc_exec_ctx *exec_ctx, grpc_call_next_op(exec_ctx, elem, op); } -static void init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, - grpc_call_element_args *args) {} +static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, + grpc_call_element_args *args) { + return GRPC_ERROR_NONE; +} static void destroy_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, const grpc_call_stats *stats, From 9f97cca37b21fbac12d840fa2337f1fba251e77c Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 23 Jun 2016 10:47:05 -0700 Subject: [PATCH 02/16] Fix error handling in grpc_call_create(). --- src/core/lib/surface/call.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c index d64ca64a15f..2e89393815f 100644 --- a/src/core/lib/surface/call.c +++ b/src/core/lib/surface/call.c @@ -266,8 +266,18 @@ grpc_call *grpc_call_create( destroy_call, call, call->context, server_transport_data, CALL_STACK_FROM_CALL(call)); -// FIXME: handle error (probably requires changing this function's API) -GPR_ASSERT(error == GRPC_ERROR_NONE); + if (error != GRPC_ERROR_NONE) { + intptr_t status; + if (!grpc_error_get_int(error, GRPC_ERROR_INT_GRPC_STATUS, &status)) + status = GRPC_STATUS_UNKNOWN; + const char* error_string = grpc_error_string(error); + received_status* status_struct = &call->status[STATUS_FROM_CORE]; + status_struct->is_set = true; + status_struct->code = status; + status_struct->details = grpc_mdstr_from_string(error_string); + grpc_error_free_string(error_string); + grpc_error_unref(error); + } if (cq != NULL) { GPR_ASSERT( pollset_set_alternative == NULL && From 09e669878f57e4e4a56bb920d4e3e48615fde55d Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 23 Jun 2016 11:14:18 -0700 Subject: [PATCH 03/16] Propagate error up through grpc_connected_subchannel_create_call(). --- src/core/ext/client_config/subchannel.c | 23 ++++++----- src/core/ext/client_config/subchannel.h | 4 +- .../client_config/subchannel_call_holder.c | 38 ++++++++++++++----- 3 files changed, 44 insertions(+), 21 deletions(-) diff --git a/src/core/ext/client_config/subchannel.c b/src/core/ext/client_config/subchannel.c index 2e9cf7d7d4e..80cf154f96a 100644 --- a/src/core/ext/client_config/subchannel.c +++ b/src/core/ext/client_config/subchannel.c @@ -700,22 +700,25 @@ grpc_connected_subchannel *grpc_subchannel_get_connected_subchannel( return GET_CONNECTED_SUBCHANNEL(c, acq); } -grpc_subchannel_call *grpc_connected_subchannel_create_call( +grpc_error *grpc_connected_subchannel_create_call( grpc_exec_ctx *exec_ctx, grpc_connected_subchannel *con, - grpc_polling_entity *pollent) { + grpc_polling_entity *pollent, grpc_subchannel_call **call) { grpc_channel_stack *chanstk = CHANNEL_STACK_FROM_CONNECTION(con); - grpc_subchannel_call *call = - gpr_malloc(sizeof(grpc_subchannel_call) + chanstk->call_stack_size); - grpc_call_stack *callstk = SUBCHANNEL_CALL_TO_CALL_STACK(call); - call->connection = con; + *call = gpr_malloc(sizeof(grpc_subchannel_call) + chanstk->call_stack_size); + grpc_call_stack *callstk = SUBCHANNEL_CALL_TO_CALL_STACK(*call); + (*call)->connection = con; GRPC_CONNECTED_SUBCHANNEL_REF(con, "subchannel_call"); grpc_error* error = grpc_call_stack_init(exec_ctx, chanstk, 1, - subchannel_call_destroy, call, + subchannel_call_destroy, *call, NULL, NULL, callstk); -// FIXME: handle error (probably requires changing this function's API) -GPR_ASSERT(error == GRPC_ERROR_NONE); + 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); + return error; + } grpc_call_stack_set_pollset_or_pollset_set(exec_ctx, callstk, pollent); - return call; + return GRPC_ERROR_NONE; } grpc_call_stack *grpc_subchannel_call_get_call_stack( diff --git a/src/core/ext/client_config/subchannel.h b/src/core/ext/client_config/subchannel.h index b6d39f5dc52..21454cb7a4d 100644 --- a/src/core/ext/client_config/subchannel.h +++ b/src/core/ext/client_config/subchannel.h @@ -108,9 +108,9 @@ void grpc_subchannel_call_unref(grpc_exec_ctx *exec_ctx, GRPC_SUBCHANNEL_REF_EXTRA_ARGS); /** construct a subchannel call */ -grpc_subchannel_call *grpc_connected_subchannel_create_call( +grpc_error *grpc_connected_subchannel_create_call( grpc_exec_ctx *exec_ctx, grpc_connected_subchannel *connected_subchannel, - grpc_polling_entity *pollent); + grpc_polling_entity *pollent, grpc_subchannel_call** subchannel_call); /** process a transport level op */ void grpc_connected_subchannel_process_transport_op( diff --git a/src/core/ext/client_config/subchannel_call_holder.c b/src/core/ext/client_config/subchannel_call_holder.c index e31800edd98..25bd9798f0a 100644 --- a/src/core/ext/client_config/subchannel_call_holder.c +++ b/src/core/ext/client_config/subchannel_call_holder.c @@ -84,6 +84,11 @@ void grpc_subchannel_call_holder_destroy(grpc_exec_ctx *exec_ctx, gpr_free(holder->waiting_ops); } +// The logic here is fairly complicated, due to (a) the fact that we +// need to handle the case where we receive the send op before the +// initial metadata op, and (b) the need for efficiency, especially in +// the streaming case. +// TODO(ctiller): Explain this more thoroughly. void grpc_subchannel_call_holder_perform_op(grpc_exec_ctx *exec_ctx, grpc_subchannel_call_holder *holder, grpc_transport_stream_op *op) { @@ -121,7 +126,8 @@ retry: } /* if this is a cancellation, then we can raise our cancelled flag */ if (op->cancel_with_status != GRPC_STATUS_OK) { - if (!gpr_atm_rel_cas(&holder->subchannel_call, 0, 1)) { + if (!gpr_atm_rel_cas(&holder->subchannel_call, 0, + (gpr_atm)(uintptr_t)CANCELLED_CALL)) { goto retry; } else { switch (holder->creation_phase) { @@ -161,10 +167,17 @@ retry: /* if we've got a subchannel, then let's ask it to create a call */ if (holder->creation_phase == GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING && holder->connected_subchannel != NULL) { - gpr_atm_rel_store( - &holder->subchannel_call, - (gpr_atm)(uintptr_t)grpc_connected_subchannel_create_call( - exec_ctx, holder->connected_subchannel, holder->pollent)); + grpc_subchannel_call* subchannel_call = NULL; + grpc_error* error = grpc_connected_subchannel_create_call( + exec_ctx, holder->connected_subchannel, holder->pollent, + &subchannel_call); + if (error != GRPC_ERROR_NONE) { + subchannel_call = CANCELLED_CALL; + fail_locked(exec_ctx, holder, error); + grpc_transport_stream_op_finish_with_failure(exec_ctx, op, error); + } + gpr_atm_rel_store(&holder->subchannel_call, + (gpr_atm)(uintptr_t)subchannel_call); retry_waiting_locked(exec_ctx, holder); goto retry; } @@ -192,10 +205,17 @@ static void subchannel_ready(grpc_exec_ctx *exec_ctx, void *arg, GRPC_ERROR_CREATE_REFERENCING( "Cancelled before creating subchannel", &error, 1)); } else { - gpr_atm_rel_store( - &holder->subchannel_call, - (gpr_atm)(uintptr_t)grpc_connected_subchannel_create_call( - exec_ctx, holder->connected_subchannel, holder->pollent)); + grpc_subchannel_call* subchannel_call = NULL; + grpc_error* new_error = grpc_connected_subchannel_create_call( + exec_ctx, holder->connected_subchannel, holder->pollent, + &subchannel_call); + if (new_error != GRPC_ERROR_NONE) { + grpc_error_add_child(new_error, error); + subchannel_call = CANCELLED_CALL; + fail_locked(exec_ctx, holder, new_error); + } + gpr_atm_rel_store(&holder->subchannel_call, + (gpr_atm)(uintptr_t)subchannel_call); retry_waiting_locked(exec_ctx, holder); } gpr_mu_unlock(&holder->mu); From 5d11e43ce34b371fb0ab1fa69e541a4d513025b8 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 23 Jun 2016 13:14:05 -0700 Subject: [PATCH 04/16] Added test for filter whose call initialization fails, and fixed bugs uncovered by the test. --- Makefile | 2 + src/core/lib/channel/channel_stack.c | 7 +- src/core/lib/iomgr/error.c | 6 + src/core/lib/iomgr/error.h | 3 + src/core/lib/surface/call.c | 10 +- test/core/end2end/end2end_nosec_tests.c | 8 + test/core/end2end/end2end_tests.c | 8 + test/core/end2end/gen_build_yaml.py | 1 + .../end2end/tests/filter_call_init_fails.c | 263 ++++++++ tools/run_tests/sources_and_headers.json | 2 + tools/run_tests/tests.json | 627 +++++++++++++++++- .../end2end_nosec_tests.vcxproj | 2 + .../end2end_nosec_tests.vcxproj.filters | 3 + .../tests/end2end_tests/end2end_tests.vcxproj | 2 + .../end2end_tests.vcxproj.filters | 3 + 15 files changed, 929 insertions(+), 18 deletions(-) create mode 100644 test/core/end2end/tests/filter_call_init_fails.c diff --git a/Makefile b/Makefile index 9be3e5784cc..8e5d0102c3f 100644 --- a/Makefile +++ b/Makefile @@ -6366,6 +6366,7 @@ LIBEND2END_TESTS_SRC = \ test/core/end2end/tests/default_host.c \ test/core/end2end/tests/disappearing_server.c \ test/core/end2end/tests/empty_batch.c \ + test/core/end2end/tests/filter_call_init_fails.c \ test/core/end2end/tests/filter_causes_close.c \ test/core/end2end/tests/graceful_server_shutdown.c \ test/core/end2end/tests/high_initial_seqno.c \ @@ -6443,6 +6444,7 @@ LIBEND2END_NOSEC_TESTS_SRC = \ test/core/end2end/tests/default_host.c \ test/core/end2end/tests/disappearing_server.c \ test/core/end2end/tests/empty_batch.c \ + test/core/end2end/tests/filter_call_init_fails.c \ test/core/end2end/tests/filter_causes_close.c \ test/core/end2end/tests/graceful_server_shutdown.c \ test/core/end2end/tests/high_initial_seqno.c \ diff --git a/src/core/lib/channel/channel_stack.c b/src/core/lib/channel/channel_stack.c index 0613f94c299..715f6316254 100644 --- a/src/core/lib/channel/channel_stack.c +++ b/src/core/lib/channel/channel_stack.c @@ -179,6 +179,7 @@ grpc_error* grpc_call_stack_init(grpc_exec_ctx *exec_ctx, ROUND_UP_TO_ALIGNMENT_SIZE(count * sizeof(grpc_call_element)); /* init per-filter data */ + grpc_error* first_error = GRPC_ERROR_NONE; for (i = 0; i < count; i++) { args.call_stack = call_stack; args.server_transport_data = transport_server_data; @@ -188,12 +189,12 @@ grpc_error* grpc_call_stack_init(grpc_exec_ctx *exec_ctx, call_elems[i].call_data = user_data; grpc_error* error = call_elems[i].filter->init_call_elem( exec_ctx, &call_elems[i], &args); - if (error != GRPC_ERROR_NONE) - return error; + if (error != GRPC_ERROR_NONE && first_error == GRPC_ERROR_NONE) + first_error = error; user_data += ROUND_UP_TO_ALIGNMENT_SIZE(call_elems[i].filter->sizeof_call_data); } - return GRPC_ERROR_NONE; + return first_error; } void grpc_call_stack_set_pollset_or_pollset_set(grpc_exec_ctx *exec_ctx, diff --git a/src/core/lib/iomgr/error.c b/src/core/lib/iomgr/error.c index 540fb4fa7e4..1b15963e57d 100644 --- a/src/core/lib/iomgr/error.c +++ b/src/core/lib/iomgr/error.c @@ -286,6 +286,12 @@ grpc_error *grpc_error_set_str(grpc_error *src, grpc_error_strs which, return new; } +const char *grpc_error_get_str(grpc_error *error, grpc_error_strs which) { + void *s = NULL; + gpr_avl_maybe_get(error->strs, (void *)(uintptr_t)which, &s); + return s; +} + grpc_error *grpc_error_add_child(grpc_error *src, grpc_error *child) { grpc_error *new = copy_error_and_unref(src); new->errs = gpr_avl_add(new->errs, (void *)(new->next_err++), child); diff --git a/src/core/lib/iomgr/error.h b/src/core/lib/iomgr/error.h index 69cdf3028e4..4bff591fe31 100644 --- a/src/core/lib/iomgr/error.h +++ b/src/core/lib/iomgr/error.h @@ -169,6 +169,9 @@ grpc_error *grpc_error_set_time(grpc_error *src, grpc_error_times which, gpr_timespec value); grpc_error *grpc_error_set_str(grpc_error *src, grpc_error_strs which, const char *value); +/// Returns NULL if the specified string is not set. +/// Caller does NOT own return value. +const char *grpc_error_get_str(grpc_error *error, grpc_error_strs which); /// Add a child error: an error that is believed to have contributed to this /// error occurring. Allows root causing high level errors from lower level /// errors that contributed to them. diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c index 2e89393815f..d68afff39f7 100644 --- a/src/core/lib/surface/call.c +++ b/src/core/lib/surface/call.c @@ -270,12 +270,10 @@ grpc_call *grpc_call_create( intptr_t status; if (!grpc_error_get_int(error, GRPC_ERROR_INT_GRPC_STATUS, &status)) status = GRPC_STATUS_UNKNOWN; - const char* error_string = grpc_error_string(error); - received_status* status_struct = &call->status[STATUS_FROM_CORE]; - status_struct->is_set = true; - status_struct->code = status; - status_struct->details = grpc_mdstr_from_string(error_string); - grpc_error_free_string(error_string); + const char* error_str = grpc_error_get_str(error, + GRPC_ERROR_STR_DESCRIPTION); + close_with_status(&exec_ctx, call, status, + error_str == NULL ? "unknown error" : error_str); grpc_error_unref(error); } if (cq != NULL) { diff --git a/test/core/end2end/end2end_nosec_tests.c b/test/core/end2end/end2end_nosec_tests.c index 2893bddc43c..03959308987 100644 --- a/test/core/end2end/end2end_nosec_tests.c +++ b/test/core/end2end/end2end_nosec_tests.c @@ -69,6 +69,8 @@ extern void disappearing_server(grpc_end2end_test_config config); extern void disappearing_server_pre_init(void); extern void empty_batch(grpc_end2end_test_config config); extern void empty_batch_pre_init(void); +extern void filter_call_init_fails(grpc_end2end_test_config config); +extern void filter_call_init_fails_pre_init(void); extern void filter_causes_close(grpc_end2end_test_config config); extern void filter_causes_close_pre_init(void); extern void graceful_server_shutdown(grpc_end2end_test_config config); @@ -136,6 +138,7 @@ void grpc_end2end_tests_pre_init(void) { default_host_pre_init(); disappearing_server_pre_init(); empty_batch_pre_init(); + filter_call_init_fails_pre_init(); filter_causes_close_pre_init(); graceful_server_shutdown_pre_init(); high_initial_seqno_pre_init(); @@ -183,6 +186,7 @@ void grpc_end2end_tests(int argc, char **argv, default_host(config); disappearing_server(config); empty_batch(config); + filter_call_init_fails(config); filter_causes_close(config); graceful_server_shutdown(config); high_initial_seqno(config); @@ -264,6 +268,10 @@ void grpc_end2end_tests(int argc, char **argv, empty_batch(config); continue; } + if (0 == strcmp("filter_call_init_fails", argv[i])) { + filter_call_init_fails(config); + continue; + } if (0 == strcmp("filter_causes_close", argv[i])) { filter_causes_close(config); continue; diff --git a/test/core/end2end/end2end_tests.c b/test/core/end2end/end2end_tests.c index 96a38e76dcc..1d6ada72540 100644 --- a/test/core/end2end/end2end_tests.c +++ b/test/core/end2end/end2end_tests.c @@ -71,6 +71,8 @@ extern void disappearing_server(grpc_end2end_test_config config); extern void disappearing_server_pre_init(void); extern void empty_batch(grpc_end2end_test_config config); extern void empty_batch_pre_init(void); +extern void filter_call_init_fails(grpc_end2end_test_config config); +extern void filter_call_init_fails_pre_init(void); extern void filter_causes_close(grpc_end2end_test_config config); extern void filter_causes_close_pre_init(void); extern void graceful_server_shutdown(grpc_end2end_test_config config); @@ -139,6 +141,7 @@ void grpc_end2end_tests_pre_init(void) { default_host_pre_init(); disappearing_server_pre_init(); empty_batch_pre_init(); + filter_call_init_fails_pre_init(); filter_causes_close_pre_init(); graceful_server_shutdown_pre_init(); high_initial_seqno_pre_init(); @@ -187,6 +190,7 @@ void grpc_end2end_tests(int argc, char **argv, default_host(config); disappearing_server(config); empty_batch(config); + filter_call_init_fails(config); filter_causes_close(config); graceful_server_shutdown(config); high_initial_seqno(config); @@ -272,6 +276,10 @@ void grpc_end2end_tests(int argc, char **argv, empty_batch(config); continue; } + if (0 == strcmp("filter_call_init_fails", argv[i])) { + filter_call_init_fails(config); + continue; + } if (0 == strcmp("filter_causes_close", argv[i])) { filter_causes_close(config); continue; diff --git a/test/core/end2end/gen_build_yaml.py b/test/core/end2end/gen_build_yaml.py index 6d3d8f8d3c4..5037559fea0 100755 --- a/test/core/end2end/gen_build_yaml.py +++ b/test/core/end2end/gen_build_yaml.py @@ -102,6 +102,7 @@ END2END_TESTS = { 'disappearing_server': connectivity_test_options, 'empty_batch': default_test_options, 'filter_causes_close': default_test_options, + 'filter_call_init_fails': default_test_options, 'graceful_server_shutdown': default_test_options._replace(cpu_cost=LOWCPU), 'hpack_size': default_test_options._replace(proxyable=False, traceable=False), diff --git a/test/core/end2end/tests/filter_call_init_fails.c b/test/core/end2end/tests/filter_call_init_fails.c new file mode 100644 index 00000000000..d97369fa98b --- /dev/null +++ b/test/core/end2end/tests/filter_call_init_fails.c @@ -0,0 +1,263 @@ +/* + * + * Copyright 2016, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#include "test/core/end2end/end2end_tests.h" + +#include +#include +#include + +#include +#include +#include +#include +#include +#include "src/core/lib/channel/channel_stack_builder.h" +#include "src/core/lib/surface/channel_init.h" +#include "test/core/end2end/cq_verifier.h" + +enum { TIMEOUT = 200000 }; + +static bool g_enable_filter = false; + +static void *tag(intptr_t t) { return (void *)t; } + +static grpc_end2end_test_fixture begin_test(grpc_end2end_test_config config, + const char *test_name, + grpc_channel_args *client_args, + grpc_channel_args *server_args) { + grpc_end2end_test_fixture f; + gpr_log(GPR_INFO, "%s/%s", test_name, config.name); + f = config.create_fixture(client_args, server_args); + config.init_server(&f, server_args); + config.init_client(&f, client_args); + return f; +} + +static gpr_timespec n_seconds_time(int n) { + return GRPC_TIMEOUT_SECONDS_TO_DEADLINE(n); +} + +static gpr_timespec five_seconds_time(void) { return n_seconds_time(5); } + +static void drain_cq(grpc_completion_queue *cq) { + grpc_event ev; + do { + ev = grpc_completion_queue_next(cq, five_seconds_time(), NULL); + } while (ev.type != GRPC_QUEUE_SHUTDOWN); +} + +static void shutdown_server(grpc_end2end_test_fixture *f) { + if (!f->server) return; + grpc_server_shutdown_and_notify(f->server, f->cq, tag(1000)); + GPR_ASSERT(grpc_completion_queue_pluck( + f->cq, tag(1000), GRPC_TIMEOUT_SECONDS_TO_DEADLINE(5), NULL) + .type == GRPC_OP_COMPLETE); + grpc_server_destroy(f->server); + f->server = NULL; +} + +static void shutdown_client(grpc_end2end_test_fixture *f) { + if (!f->client) return; + grpc_channel_destroy(f->client); + f->client = NULL; +} + +static void end_test(grpc_end2end_test_fixture *f) { + shutdown_server(f); + shutdown_client(f); + + grpc_completion_queue_shutdown(f->cq); + drain_cq(f->cq); + grpc_completion_queue_destroy(f->cq); +} + +// Simple request via a server filter that always fails to initialize +// the call. +static void test_request(grpc_end2end_test_config config) { + grpc_call *c; + grpc_call *s; + gpr_slice request_payload_slice = gpr_slice_from_copied_string("hello world"); + grpc_byte_buffer *request_payload = + grpc_raw_byte_buffer_create(&request_payload_slice, 1); + gpr_timespec deadline = five_seconds_time(); + grpc_end2end_test_fixture f = + begin_test(config, "filter_call_init_fails", NULL, NULL); + cq_verifier *cqv = cq_verifier_create(f.cq); + grpc_op ops[6]; + grpc_op *op; + grpc_metadata_array initial_metadata_recv; + grpc_metadata_array trailing_metadata_recv; + grpc_metadata_array request_metadata_recv; + grpc_byte_buffer *request_payload_recv = NULL; + grpc_call_details call_details; + grpc_status_code status; + grpc_call_error error; + char *details = NULL; + size_t details_capacity = 0; + + c = grpc_channel_create_call(f.client, NULL, GRPC_PROPAGATE_DEFAULTS, f.cq, + "/foo", "foo.test.google.fr", deadline, NULL); + GPR_ASSERT(c); + + grpc_metadata_array_init(&initial_metadata_recv); + grpc_metadata_array_init(&trailing_metadata_recv); + grpc_metadata_array_init(&request_metadata_recv); + grpc_call_details_init(&call_details); + + memset(ops, 0, sizeof(ops)); + op = ops; + op->op = GRPC_OP_SEND_INITIAL_METADATA; + op->data.send_initial_metadata.count = 0; + op->data.send_initial_metadata.metadata = NULL; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_SEND_MESSAGE; + op->data.send_message = request_payload; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_INITIAL_METADATA; + op->data.recv_initial_metadata = &initial_metadata_recv; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_STATUS_ON_CLIENT; + op->data.recv_status_on_client.trailing_metadata = &trailing_metadata_recv; + op->data.recv_status_on_client.status = &status; + op->data.recv_status_on_client.status_details = &details; + op->data.recv_status_on_client.status_details_capacity = &details_capacity; + op->flags = 0; + op->reserved = NULL; + op++; + error = grpc_call_start_batch(c, ops, (size_t)(op - ops), tag(1), NULL); + GPR_ASSERT(GRPC_CALL_OK == error); + + error = + grpc_server_request_call(f.server, &s, &call_details, + &request_metadata_recv, f.cq, f.cq, tag(101)); + GPR_ASSERT(GRPC_CALL_OK == error); + + cq_expect_completion(cqv, tag(1), 1); + cq_verify(cqv); + + GPR_ASSERT(status == GRPC_STATUS_PERMISSION_DENIED); + GPR_ASSERT(0 == strcmp(details, "access denied")); + + gpr_free(details); + grpc_metadata_array_destroy(&initial_metadata_recv); + grpc_metadata_array_destroy(&trailing_metadata_recv); + grpc_metadata_array_destroy(&request_metadata_recv); + grpc_call_details_destroy(&call_details); + + grpc_call_destroy(c); + + cq_verifier_destroy(cqv); + + grpc_byte_buffer_destroy(request_payload); + grpc_byte_buffer_destroy(request_payload_recv); + + end_test(&f); + config.tear_down_data(&f); +} + +/******************************************************************************* + * Test filter - always fails to initialize a call + */ + +static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, + grpc_call_element_args *args) { + return grpc_error_set_int(GRPC_ERROR_CREATE("access denied"), + GRPC_ERROR_INT_GRPC_STATUS, + GRPC_STATUS_PERMISSION_DENIED); +} + +static void destroy_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, + const grpc_call_stats *stats, + void *and_free_memory) {} + +static void init_channel_elem(grpc_exec_ctx *exec_ctx, + grpc_channel_element *elem, + grpc_channel_element_args *args) {} + +static void destroy_channel_elem(grpc_exec_ctx *exec_ctx, + grpc_channel_element *elem) {} + +static const grpc_channel_filter test_filter = { + grpc_call_next_op, + grpc_channel_next_op, + 0, + init_call_elem, + grpc_call_stack_ignore_set_pollset_or_pollset_set, + destroy_call_elem, + 0, + init_channel_elem, + destroy_channel_elem, + grpc_call_next_get_peer, + "filter_call_init_fails"}; + +/******************************************************************************* + * Registration + */ + +static bool maybe_add_filter(grpc_channel_stack_builder *builder, void *arg) { + if (g_enable_filter) { + return grpc_channel_stack_builder_prepend_filter(builder, &test_filter, + NULL, NULL); + } else { + return true; + } +} + +static void init_plugin(void) { + grpc_channel_init_register_stage(GRPC_SERVER_CHANNEL, 0, maybe_add_filter, + NULL); +} + +static void destroy_plugin(void) {} + +void filter_call_init_fails(grpc_end2end_test_config config) { + g_enable_filter = true; + test_request(config); + g_enable_filter = false; +} + +void filter_call_init_fails_pre_init(void) { + grpc_register_plugin(init_plugin, destroy_plugin); +} diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index 00018834af9..2bea121baad 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -5377,6 +5377,7 @@ "test/core/end2end/tests/default_host.c", "test/core/end2end/tests/disappearing_server.c", "test/core/end2end/tests/empty_batch.c", + "test/core/end2end/tests/filter_call_init_fails.c", "test/core/end2end/tests/filter_causes_close.c", "test/core/end2end/tests/graceful_server_shutdown.c", "test/core/end2end/tests/high_initial_seqno.c", @@ -5436,6 +5437,7 @@ "test/core/end2end/tests/default_host.c", "test/core/end2end/tests/disappearing_server.c", "test/core/end2end/tests/empty_batch.c", + "test/core/end2end/tests/filter_call_init_fails.c", "test/core/end2end/tests/filter_causes_close.c", "test/core/end2end/tests/graceful_server_shutdown.c", "test/core/end2end/tests/high_initial_seqno.c", diff --git a/tools/run_tests/tests.json b/tools/run_tests/tests.json index c62058ede4e..381931db0fa 100644 --- a/tools/run_tests/tests.json +++ b/tools/run_tests/tests.json @@ -4680,6 +4680,28 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_census_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -5538,6 +5560,28 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_compress_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -6382,6 +6426,27 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_fakesec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -7127,6 +7192,26 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_fd_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -7895,6 +7980,28 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_full_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -8669,6 +8776,22 @@ "linux" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "linux" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_full+pipe_test", + "platforms": [ + "linux" + ] + }, { "args": [ "filter_causes_close" @@ -9377,6 +9500,28 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_full+trace_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -10213,6 +10358,28 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_loadreporting_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -11057,6 +11224,27 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_oauth2_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -11834,6 +12022,27 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_proxy_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -12506,6 +12715,27 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_sockpair_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -13220,6 +13450,27 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_sockpair+trace_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -13915,7 +14166,7 @@ }, { "args": [ - "filter_causes_close" + "filter_call_init_fails" ], "ci_platforms": [ "windows", @@ -13936,14 +14187,14 @@ }, { "args": [ - "graceful_server_shutdown" + "filter_causes_close" ], "ci_platforms": [ "windows", "linux", "posix" ], - "cpu_cost": 0.1, + "cpu_cost": 1.0, "exclude_configs": [], "flaky": false, "language": "c", @@ -13957,14 +14208,14 @@ }, { "args": [ - "high_initial_seqno" + "graceful_server_shutdown" ], "ci_platforms": [ "windows", "linux", "posix" ], - "cpu_cost": 1.0, + "cpu_cost": 0.1, "exclude_configs": [], "flaky": false, "language": "c", @@ -13978,7 +14229,7 @@ }, { "args": [ - "hpack_size" + "high_initial_seqno" ], "ci_platforms": [ "windows", @@ -13999,7 +14250,7 @@ }, { "args": [ - "idempotent_request" + "hpack_size" ], "ci_platforms": [ "windows", @@ -14020,7 +14271,7 @@ }, { "args": [ - "invoke_large_request" + "idempotent_request" ], "ci_platforms": [ "windows", @@ -14041,7 +14292,28 @@ }, { "args": [ - "large_metadata" + "invoke_large_request" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_sockpair_1byte_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, + { + "args": [ + "large_metadata" ], "ci_platforms": [ "windows", @@ -14704,6 +14976,28 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_ssl_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -15562,6 +15856,28 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_ssl_cert_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -16364,6 +16680,27 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_ssl_proxy_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -17065,6 +17402,26 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_uds_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -17851,6 +18208,28 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_census_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -18687,6 +19066,28 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_compress_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -19437,6 +19838,26 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_fd_nosec_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -20183,6 +20604,28 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_full_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -20941,6 +21384,22 @@ "linux" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "linux" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_full+pipe_nosec_test", + "platforms": [ + "linux" + ] + }, { "args": [ "filter_causes_close" @@ -21627,6 +22086,28 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_full+trace_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -22441,6 +22922,28 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_loadreporting_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -23222,6 +23725,27 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_proxy_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -23873,6 +24397,27 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_sockpair_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -24566,6 +25111,27 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_sockpair+trace_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -25258,6 +25824,29 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [ + "msan" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair_1byte_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" @@ -26027,6 +26616,26 @@ "posix" ] }, + { + "args": [ + "filter_call_init_fails" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_uds_nosec_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "filter_causes_close" diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj index 923c1d1ab41..eb5f285c663 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj @@ -179,6 +179,8 @@ + + diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters index 6533eaa057b..9995b92ae56 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters @@ -43,6 +43,9 @@ test\core\end2end\tests + + test\core\end2end\tests + test\core\end2end\tests diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj index 0b859e25ce4..33132e1138f 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj @@ -181,6 +181,8 @@ + + diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters index ea1c5e3c237..484fb0c9cc5 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters @@ -46,6 +46,9 @@ test\core\end2end\tests + + test\core\end2end\tests + test\core\end2end\tests From 76d24420d7a6471dc3b135b62318277991ebdb08 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 23 Jun 2016 13:22:10 -0700 Subject: [PATCH 05/16] clang-format --- src/core/ext/census/grpc_filter.c | 4 ++-- src/core/ext/client_config/client_channel.c | 2 +- src/core/ext/client_config/subchannel.c | 8 ++++---- src/core/ext/client_config/subchannel.h | 2 +- src/core/ext/client_config/subchannel_call_holder.c | 8 ++++---- src/core/ext/load_reporting/load_reporting_filter.c | 2 +- src/core/lib/channel/channel_stack.c | 8 ++++---- src/core/lib/channel/channel_stack.h | 4 ++-- src/core/lib/channel/compress_filter.c | 2 +- src/core/lib/channel/connected_channel.c | 7 +++---- src/core/lib/channel/http_client_filter.c | 2 +- src/core/lib/channel/http_server_filter.c | 2 +- .../lib/security/transport/client_auth_filter.c | 2 +- .../lib/security/transport/server_auth_filter.c | 2 +- src/core/lib/surface/call.c | 13 ++++++------- src/core/lib/surface/lame_client.c | 2 +- src/core/lib/surface/server.c | 2 +- test/core/channel/channel_stack_test.c | 8 ++++---- test/core/end2end/tests/filter_call_init_fails.c | 2 +- test/core/end2end/tests/filter_causes_close.c | 2 +- 20 files changed, 41 insertions(+), 43 deletions(-) diff --git a/src/core/ext/census/grpc_filter.c b/src/core/ext/census/grpc_filter.c index dd2e2e01244..c5a6e6d3dc2 100644 --- a/src/core/ext/census/grpc_filter.c +++ b/src/core/ext/census/grpc_filter.c @@ -124,7 +124,7 @@ static void server_start_transport_op(grpc_exec_ctx *exec_ctx, grpc_call_next_op(exec_ctx, elem, op); } -static grpc_error* client_init_call_elem(grpc_exec_ctx *exec_ctx, +static grpc_error *client_init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_call_element_args *args) { call_data *d = elem->call_data; @@ -143,7 +143,7 @@ static void client_destroy_call_elem(grpc_exec_ctx *exec_ctx, /* TODO(hongyu): record rpc client stats and census_rpc_end_op here */ } -static grpc_error* server_init_call_elem(grpc_exec_ctx *exec_ctx, +static grpc_error *server_init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_call_element_args *args) { call_data *d = elem->call_data; diff --git a/src/core/ext/client_config/client_channel.c b/src/core/ext/client_config/client_channel.c index b0a09dab932..84e3fb4da65 100644 --- a/src/core/ext/client_config/client_channel.c +++ b/src/core/ext/client_config/client_channel.c @@ -430,7 +430,7 @@ static int cc_pick_subchannel(grpc_exec_ctx *exec_ctx, void *elemp, } /* Constructor for call_data */ -static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, +static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_call_element_args *args) { grpc_subchannel_call_holder_init(elem->call_data, cc_pick_subchannel, elem, diff --git a/src/core/ext/client_config/subchannel.c b/src/core/ext/client_config/subchannel.c index 80cf154f96a..8a1ac68c6e3 100644 --- a/src/core/ext/client_config/subchannel.c +++ b/src/core/ext/client_config/subchannel.c @@ -708,11 +708,11 @@ grpc_error *grpc_connected_subchannel_create_call( grpc_call_stack *callstk = SUBCHANNEL_CALL_TO_CALL_STACK(*call); (*call)->connection = con; GRPC_CONNECTED_SUBCHANNEL_REF(con, "subchannel_call"); - grpc_error* error = grpc_call_stack_init(exec_ctx, chanstk, 1, - subchannel_call_destroy, *call, - NULL, NULL, callstk); + grpc_error *error = + grpc_call_stack_init(exec_ctx, chanstk, 1, subchannel_call_destroy, *call, + NULL, NULL, callstk); if (error != GRPC_ERROR_NONE) { - const char* error_string = grpc_error_string(error); + const char *error_string = grpc_error_string(error); gpr_log(GPR_ERROR, "error: %s", error_string); grpc_error_free_string(error_string); return error; diff --git a/src/core/ext/client_config/subchannel.h b/src/core/ext/client_config/subchannel.h index 21454cb7a4d..ae1d96e6400 100644 --- a/src/core/ext/client_config/subchannel.h +++ b/src/core/ext/client_config/subchannel.h @@ -110,7 +110,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_subchannel_call** subchannel_call); + grpc_polling_entity *pollent, grpc_subchannel_call **subchannel_call); /** process a transport level op */ void grpc_connected_subchannel_process_transport_op( diff --git a/src/core/ext/client_config/subchannel_call_holder.c b/src/core/ext/client_config/subchannel_call_holder.c index 25bd9798f0a..5b542331538 100644 --- a/src/core/ext/client_config/subchannel_call_holder.c +++ b/src/core/ext/client_config/subchannel_call_holder.c @@ -167,8 +167,8 @@ retry: /* if we've got a subchannel, then let's ask it to create a call */ if (holder->creation_phase == GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING && holder->connected_subchannel != NULL) { - grpc_subchannel_call* subchannel_call = NULL; - grpc_error* error = grpc_connected_subchannel_create_call( + grpc_subchannel_call *subchannel_call = NULL; + grpc_error *error = grpc_connected_subchannel_create_call( exec_ctx, holder->connected_subchannel, holder->pollent, &subchannel_call); if (error != GRPC_ERROR_NONE) { @@ -205,8 +205,8 @@ static void subchannel_ready(grpc_exec_ctx *exec_ctx, void *arg, GRPC_ERROR_CREATE_REFERENCING( "Cancelled before creating subchannel", &error, 1)); } else { - grpc_subchannel_call* subchannel_call = NULL; - grpc_error* new_error = grpc_connected_subchannel_create_call( + grpc_subchannel_call *subchannel_call = NULL; + grpc_error *new_error = grpc_connected_subchannel_create_call( exec_ctx, holder->connected_subchannel, holder->pollent, &subchannel_call); if (new_error != GRPC_ERROR_NONE) { diff --git a/src/core/ext/load_reporting/load_reporting_filter.c b/src/core/ext/load_reporting/load_reporting_filter.c index cfe752d6e2b..b584e31c5dd 100644 --- a/src/core/ext/load_reporting/load_reporting_filter.c +++ b/src/core/ext/load_reporting/load_reporting_filter.c @@ -56,7 +56,7 @@ static void invoke_lr_fn_locked(grpc_load_reporting_config *lrc, } /* Constructor for call_data */ -static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, +static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_call_element_args *args) { call_data *calld = elem->call_data; diff --git a/src/core/lib/channel/channel_stack.c b/src/core/lib/channel/channel_stack.c index 715f6316254..ff824c781f1 100644 --- a/src/core/lib/channel/channel_stack.c +++ b/src/core/lib/channel/channel_stack.c @@ -157,7 +157,7 @@ void grpc_channel_stack_destroy(grpc_exec_ctx *exec_ctx, } } -grpc_error* grpc_call_stack_init(grpc_exec_ctx *exec_ctx, +grpc_error *grpc_call_stack_init(grpc_exec_ctx *exec_ctx, grpc_channel_stack *channel_stack, int initial_refs, grpc_iomgr_cb_func destroy, void *destroy_arg, @@ -179,7 +179,7 @@ grpc_error* grpc_call_stack_init(grpc_exec_ctx *exec_ctx, ROUND_UP_TO_ALIGNMENT_SIZE(count * sizeof(grpc_call_element)); /* init per-filter data */ - grpc_error* first_error = GRPC_ERROR_NONE; + grpc_error *first_error = GRPC_ERROR_NONE; for (i = 0; i < count; i++) { args.call_stack = call_stack; args.server_transport_data = transport_server_data; @@ -187,8 +187,8 @@ grpc_error* grpc_call_stack_init(grpc_exec_ctx *exec_ctx, call_elems[i].filter = channel_elems[i].filter; call_elems[i].channel_data = channel_elems[i].channel_data; call_elems[i].call_data = user_data; - grpc_error* error = call_elems[i].filter->init_call_elem( - exec_ctx, &call_elems[i], &args); + grpc_error *error = + call_elems[i].filter->init_call_elem(exec_ctx, &call_elems[i], &args); if (error != GRPC_ERROR_NONE && first_error == GRPC_ERROR_NONE) first_error = error; user_data += diff --git a/src/core/lib/channel/channel_stack.h b/src/core/lib/channel/channel_stack.h index f56c6540674..ba7baeb23fc 100644 --- a/src/core/lib/channel/channel_stack.h +++ b/src/core/lib/channel/channel_stack.h @@ -110,7 +110,7 @@ typedef struct { on a client; if it is non-NULL, then it points to memory owned by the transport and is on the server. Most filters want to ignore this argument. */ - grpc_error* (*init_call_elem)(grpc_exec_ctx *exec_ctx, + grpc_error *(*init_call_elem)(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_call_element_args *args); void (*set_pollset_or_pollset_set)(grpc_exec_ctx *exec_ctx, @@ -210,7 +210,7 @@ void grpc_channel_stack_destroy(grpc_exec_ctx *exec_ctx, /* Initialize a call stack given a channel stack. transport_server_data is expected to be NULL on a client, or an opaque transport owned pointer on the server. */ -grpc_error* grpc_call_stack_init(grpc_exec_ctx *exec_ctx, +grpc_error *grpc_call_stack_init(grpc_exec_ctx *exec_ctx, grpc_channel_stack *channel_stack, int initial_refs, grpc_iomgr_cb_func destroy, void *destroy_arg, diff --git a/src/core/lib/channel/compress_filter.c b/src/core/lib/channel/compress_filter.c index bcc8e17a9db..5be32929dad 100644 --- a/src/core/lib/channel/compress_filter.c +++ b/src/core/lib/channel/compress_filter.c @@ -256,7 +256,7 @@ static void compress_start_transport_stream_op(grpc_exec_ctx *exec_ctx, } /* Constructor for call_data */ -static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, +static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_call_element_args *args) { /* grab pointers to our data from the call element */ diff --git a/src/core/lib/channel/connected_channel.c b/src/core/lib/channel/connected_channel.c index 8dde688df2c..ecafcc99c79 100644 --- a/src/core/lib/channel/connected_channel.c +++ b/src/core/lib/channel/connected_channel.c @@ -81,7 +81,7 @@ static void con_start_transport_op(grpc_exec_ctx *exec_ctx, } /* Constructor for call_data */ -static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, +static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_call_element_args *args) { call_data *calld = elem->call_data; @@ -89,9 +89,8 @@ static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, int r = grpc_transport_init_stream( exec_ctx, chand->transport, TRANSPORT_STREAM_FROM_CALL_DATA(calld), &args->call_stack->refcount, args->server_transport_data); - return r == 0 - ? GRPC_ERROR_NONE - : GRPC_ERROR_CREATE("transport initialization failed"); + return r == 0 ? GRPC_ERROR_NONE + : GRPC_ERROR_CREATE("transport initialization failed"); } static void set_pollset_or_pollset_set(grpc_exec_ctx *exec_ctx, diff --git a/src/core/lib/channel/http_client_filter.c b/src/core/lib/channel/http_client_filter.c index 30f4ed1f6fc..7f8ca1dc9f4 100644 --- a/src/core/lib/channel/http_client_filter.c +++ b/src/core/lib/channel/http_client_filter.c @@ -169,7 +169,7 @@ static void hc_start_transport_op(grpc_exec_ctx *exec_ctx, } /* Constructor for call_data */ -static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, +static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_call_element_args *args) { call_data *calld = elem->call_data; diff --git a/src/core/lib/channel/http_server_filter.c b/src/core/lib/channel/http_server_filter.c index 960d1ce45e5..e1f76510b63 100644 --- a/src/core/lib/channel/http_server_filter.c +++ b/src/core/lib/channel/http_server_filter.c @@ -224,7 +224,7 @@ static void hs_start_transport_op(grpc_exec_ctx *exec_ctx, } /* Constructor for call_data */ -static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, +static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_call_element_args *args) { /* grab pointers to our data from the call element */ diff --git a/src/core/lib/security/transport/client_auth_filter.c b/src/core/lib/security/transport/client_auth_filter.c index 6179b9e18cf..bccb8f755e5 100644 --- a/src/core/lib/security/transport/client_auth_filter.c +++ b/src/core/lib/security/transport/client_auth_filter.c @@ -264,7 +264,7 @@ static void auth_start_transport_op(grpc_exec_ctx *exec_ctx, } /* Constructor for call_data */ -static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, +static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_call_element_args *args) { call_data *calld = elem->call_data; diff --git a/src/core/lib/security/transport/server_auth_filter.c b/src/core/lib/security/transport/server_auth_filter.c index 379247131b6..86d4d616371 100644 --- a/src/core/lib/security/transport/server_auth_filter.c +++ b/src/core/lib/security/transport/server_auth_filter.c @@ -199,7 +199,7 @@ static void auth_start_transport_op(grpc_exec_ctx *exec_ctx, } /* Constructor for call_data */ -static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, +static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_call_element_args *args) { /* grab pointers to our data from the call element */ diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c index d68afff39f7..f862e8dee99 100644 --- a/src/core/lib/surface/call.c +++ b/src/core/lib/surface/call.c @@ -262,18 +262,17 @@ grpc_call *grpc_call_create( call->send_deadline = send_deadline; GRPC_CHANNEL_INTERNAL_REF(channel, "call"); /* initial refcount dropped by grpc_call_destroy */ - grpc_error* error = grpc_call_stack_init(&exec_ctx, channel_stack, 1, - destroy_call, call, call->context, - server_transport_data, - CALL_STACK_FROM_CALL(call)); + grpc_error *error = grpc_call_stack_init( + &exec_ctx, channel_stack, 1, destroy_call, call, call->context, + server_transport_data, CALL_STACK_FROM_CALL(call)); if (error != GRPC_ERROR_NONE) { intptr_t status; if (!grpc_error_get_int(error, GRPC_ERROR_INT_GRPC_STATUS, &status)) status = GRPC_STATUS_UNKNOWN; - const char* error_str = grpc_error_get_str(error, - GRPC_ERROR_STR_DESCRIPTION); + const char *error_str = + grpc_error_get_str(error, GRPC_ERROR_STR_DESCRIPTION); close_with_status(&exec_ctx, call, status, - error_str == NULL ? "unknown error" : error_str); + error_str == NULL ? "unknown error" : error_str); grpc_error_unref(error); } if (cq != NULL) { diff --git a/src/core/lib/surface/lame_client.c b/src/core/lib/surface/lame_client.c index 7d9168f2e0b..1cf18ba0605 100644 --- a/src/core/lib/surface/lame_client.c +++ b/src/core/lib/surface/lame_client.c @@ -107,7 +107,7 @@ static void lame_start_transport_op(grpc_exec_ctx *exec_ctx, GRPC_ERROR_UNREF(op->disconnect_with_error); } -static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, +static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_call_element_args *args) { return GRPC_ERROR_NONE; diff --git a/src/core/lib/surface/server.c b/src/core/lib/surface/server.c index 0a0c224b4d6..165d8aa647a 100644 --- a/src/core/lib/surface/server.c +++ b/src/core/lib/surface/server.c @@ -848,7 +848,7 @@ static void channel_connectivity_changed(grpc_exec_ctx *exec_ctx, void *cd, } } -static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, +static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_call_element_args *args) { call_data *calld = elem->call_data; diff --git a/test/core/channel/channel_stack_test.c b/test/core/channel/channel_stack_test.c index 9c6a47eb520..d6c8a9a1428 100644 --- a/test/core/channel/channel_stack_test.c +++ b/test/core/channel/channel_stack_test.c @@ -53,7 +53,7 @@ static void channel_init_func(grpc_exec_ctx *exec_ctx, *(int *)(elem->channel_data) = 0; } -static grpc_error* call_init_func(grpc_exec_ctx *exec_ctx, +static grpc_error *call_init_func(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_call_element_args *args) { ++*(int *)(elem->channel_data); @@ -134,9 +134,9 @@ static void test_create_channel_stack(void) { GPR_ASSERT(*channel_data == 0); call_stack = gpr_malloc(channel_stack->call_stack_size); - grpc_error* error = grpc_call_stack_init(&exec_ctx, channel_stack, 1, - free_call, call_stack, NULL, NULL, - call_stack); + grpc_error *error = + grpc_call_stack_init(&exec_ctx, channel_stack, 1, free_call, call_stack, + NULL, NULL, call_stack); GPR_ASSERT(error == GRPC_ERROR_NONE); GPR_ASSERT(call_stack->count == 1); call_elem = grpc_call_stack_element(call_stack, 0); diff --git a/test/core/end2end/tests/filter_call_init_fails.c b/test/core/end2end/tests/filter_call_init_fails.c index d97369fa98b..e8ceb33eb7e 100644 --- a/test/core/end2end/tests/filter_call_init_fails.c +++ b/test/core/end2end/tests/filter_call_init_fails.c @@ -200,7 +200,7 @@ static void test_request(grpc_end2end_test_config config) { * Test filter - always fails to initialize a call */ -static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, +static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_call_element_args *args) { return grpc_error_set_int(GRPC_ERROR_CREATE("access denied"), diff --git a/test/core/end2end/tests/filter_causes_close.c b/test/core/end2end/tests/filter_causes_close.c index 3ff0abed637..8b0c8dba61d 100644 --- a/test/core/end2end/tests/filter_causes_close.c +++ b/test/core/end2end/tests/filter_causes_close.c @@ -233,7 +233,7 @@ static void start_transport_stream_op(grpc_exec_ctx *exec_ctx, grpc_call_next_op(exec_ctx, elem, op); } -static grpc_error* init_call_elem(grpc_exec_ctx *exec_ctx, +static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_call_element_args *args) { return GRPC_ERROR_NONE; From acfb34333d09b2182c79ff2df7f5e0a6102c7b80 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Wed, 29 Jun 2016 13:02:56 -0700 Subject: [PATCH 06/16] Fix msan/asan/tsan build failure. --- src/core/lib/surface/call.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c index f862e8dee99..cdb896909a5 100644 --- a/src/core/lib/surface/call.c +++ b/src/core/lib/surface/call.c @@ -271,7 +271,7 @@ grpc_call *grpc_call_create( status = GRPC_STATUS_UNKNOWN; const char *error_str = grpc_error_get_str(error, GRPC_ERROR_STR_DESCRIPTION); - close_with_status(&exec_ctx, call, status, + close_with_status(&exec_ctx, call, (grpc_status_code)status, error_str == NULL ? "unknown error" : error_str); grpc_error_unref(error); } From afa864244d921626480abc6e4408a2aa183396c3 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Wed, 29 Jun 2016 14:38:37 -0700 Subject: [PATCH 07/16] When call creation fails, free the allocated call. --- src/core/ext/client_config/subchannel.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/ext/client_config/subchannel.c b/src/core/ext/client_config/subchannel.c index 8a1ac68c6e3..495ca46cc87 100644 --- a/src/core/ext/client_config/subchannel.c +++ b/src/core/ext/client_config/subchannel.c @@ -706,8 +706,7 @@ grpc_error *grpc_connected_subchannel_create_call( grpc_channel_stack *chanstk = CHANNEL_STACK_FROM_CONNECTION(con); *call = gpr_malloc(sizeof(grpc_subchannel_call) + chanstk->call_stack_size); grpc_call_stack *callstk = SUBCHANNEL_CALL_TO_CALL_STACK(*call); - (*call)->connection = con; - GRPC_CONNECTED_SUBCHANNEL_REF(con, "subchannel_call"); + (*call)->connection = con; // Ref is added below. grpc_error *error = grpc_call_stack_init(exec_ctx, chanstk, 1, subchannel_call_destroy, *call, NULL, NULL, callstk); @@ -715,8 +714,10 @@ grpc_error *grpc_connected_subchannel_create_call( 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; } + GRPC_CONNECTED_SUBCHANNEL_REF(con, "subchannel_call"); grpc_call_stack_set_pollset_or_pollset_set(exec_ctx, callstk, pollent); return GRPC_ERROR_NONE; } From 51d002244816d53623d1743df15fe26e94f9e8b6 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Fri, 15 Jul 2016 09:12:26 -0700 Subject: [PATCH 08/16] Change priority for adding the filter in the filter_call_init_fails test. --- test/core/end2end/tests/filter_call_init_fails.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/core/end2end/tests/filter_call_init_fails.c b/test/core/end2end/tests/filter_call_init_fails.c index e8ceb33eb7e..a1699e86c9f 100644 --- a/test/core/end2end/tests/filter_call_init_fails.c +++ b/test/core/end2end/tests/filter_call_init_fails.c @@ -238,16 +238,16 @@ static const grpc_channel_filter test_filter = { static bool maybe_add_filter(grpc_channel_stack_builder *builder, void *arg) { if (g_enable_filter) { - return grpc_channel_stack_builder_prepend_filter(builder, &test_filter, - NULL, NULL); + return grpc_channel_stack_builder_append_filter(builder, &test_filter, + NULL, NULL); } else { return true; } } static void init_plugin(void) { - grpc_channel_init_register_stage(GRPC_SERVER_CHANNEL, 0, maybe_add_filter, - NULL); + grpc_channel_init_register_stage(GRPC_SERVER_CHANNEL, MAX_INT, + maybe_add_filter, NULL); } static void destroy_plugin(void) {} From 44a2f25667fa902edcfa0e184be9e980ac002664 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Mon, 18 Jul 2016 14:13:31 -0700 Subject: [PATCH 09/16] s/MAX_INT/INT_MAX/ --- test/core/end2end/tests/filter_call_init_fails.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/core/end2end/tests/filter_call_init_fails.c b/test/core/end2end/tests/filter_call_init_fails.c index a1699e86c9f..78cc56921e0 100644 --- a/test/core/end2end/tests/filter_call_init_fails.c +++ b/test/core/end2end/tests/filter_call_init_fails.c @@ -33,6 +33,7 @@ #include "test/core/end2end/end2end_tests.h" +#include #include #include #include @@ -246,7 +247,7 @@ static bool maybe_add_filter(grpc_channel_stack_builder *builder, void *arg) { } static void init_plugin(void) { - grpc_channel_init_register_stage(GRPC_SERVER_CHANNEL, MAX_INT, + grpc_channel_init_register_stage(GRPC_SERVER_CHANNEL, INT_MAX, maybe_add_filter, NULL); } From b5f32f0aad59c98e89734f7932fc374cdf475b3a Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Tue, 19 Jul 2016 08:36:52 -0700 Subject: [PATCH 10/16] Add test filter right before connected channel filter. --- test/core/end2end/tests/filter_call_init_fails.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/test/core/end2end/tests/filter_call_init_fails.c b/test/core/end2end/tests/filter_call_init_fails.c index 78cc56921e0..1be6b8bdeaa 100644 --- a/test/core/end2end/tests/filter_call_init_fails.c +++ b/test/core/end2end/tests/filter_call_init_fails.c @@ -239,8 +239,15 @@ static const grpc_channel_filter test_filter = { static bool maybe_add_filter(grpc_channel_stack_builder *builder, void *arg) { if (g_enable_filter) { - return grpc_channel_stack_builder_append_filter(builder, &test_filter, - NULL, NULL); + // Want to add the filter as close to the end as possible, to make + // sure that all of the filters work well together. However, we + // can't add it at the very end, because the connected channel filter + // must be the last one. So we add it right before the last one. + grpc_channel_stack_builder_iterator *it = + grpc_channel_stack_builder_create_iterator_at_last(builder); + GPR_ASSERT(grpc_channel_stack_builder_move_prev(it)); + return grpc_channel_stack_builder_add_filter_before(it, &test_filter, + NULL, NULL); } else { return true; } From e7ab7203311e3c30569ad98963ac066f335ecb9b Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 21 Jul 2016 11:02:48 -0700 Subject: [PATCH 11/16] clang-format --- test/core/end2end/tests/filter_call_init_fails.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/core/end2end/tests/filter_call_init_fails.c b/test/core/end2end/tests/filter_call_init_fails.c index 1be6b8bdeaa..4980b372784 100644 --- a/test/core/end2end/tests/filter_call_init_fails.c +++ b/test/core/end2end/tests/filter_call_init_fails.c @@ -246,8 +246,8 @@ static bool maybe_add_filter(grpc_channel_stack_builder *builder, void *arg) { grpc_channel_stack_builder_iterator *it = grpc_channel_stack_builder_create_iterator_at_last(builder); GPR_ASSERT(grpc_channel_stack_builder_move_prev(it)); - return grpc_channel_stack_builder_add_filter_before(it, &test_filter, - NULL, NULL); + return grpc_channel_stack_builder_add_filter_before(it, &test_filter, NULL, + NULL); } else { return true; } From 05d73af3d18ec301d2778f343edd1cae7d4acd1a Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Wed, 27 Jul 2016 15:52:46 +0000 Subject: [PATCH 12/16] Fixed asan failure and some refcounting bugs. --- src/core/ext/client_config/subchannel_call_holder.c | 2 +- src/core/lib/channel/channel_stack.c | 9 +++++++-- src/core/lib/channel/connected_channel.c | 2 +- src/core/lib/surface/call.c | 2 +- test/core/end2end/tests/filter_call_init_fails.c | 6 ++++-- third_party/protobuf | 2 +- 6 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/core/ext/client_config/subchannel_call_holder.c b/src/core/ext/client_config/subchannel_call_holder.c index ac286825f68..be6d054af4e 100644 --- a/src/core/ext/client_config/subchannel_call_holder.c +++ b/src/core/ext/client_config/subchannel_call_holder.c @@ -170,7 +170,7 @@ retry: &subchannel_call); if (error != GRPC_ERROR_NONE) { subchannel_call = CANCELLED_CALL; - fail_locked(exec_ctx, holder, error); + fail_locked(exec_ctx, holder, GRPC_ERROR_REF(error)); grpc_transport_stream_op_finish_with_failure(exec_ctx, op, error); } gpr_atm_rel_store(&holder->subchannel_call, diff --git a/src/core/lib/channel/channel_stack.c b/src/core/lib/channel/channel_stack.c index 70062e0c3b0..7fcc0ef97f4 100644 --- a/src/core/lib/channel/channel_stack.c +++ b/src/core/lib/channel/channel_stack.c @@ -189,8 +189,13 @@ grpc_error *grpc_call_stack_init(grpc_exec_ctx *exec_ctx, call_elems[i].call_data = user_data; grpc_error *error = call_elems[i].filter->init_call_elem(exec_ctx, &call_elems[i], &args); - if (error != GRPC_ERROR_NONE && first_error == GRPC_ERROR_NONE) - first_error = error; + if (error != GRPC_ERROR_NONE) { + if (first_error == GRPC_ERROR_NONE) { + first_error = error; + } else { + GRPC_ERROR_UNREF(error); + } + } user_data += ROUND_UP_TO_ALIGNMENT_SIZE(call_elems[i].filter->sizeof_call_data); } diff --git a/src/core/lib/channel/connected_channel.c b/src/core/lib/channel/connected_channel.c index ecafcc99c79..bad1c50a71c 100644 --- a/src/core/lib/channel/connected_channel.c +++ b/src/core/lib/channel/connected_channel.c @@ -90,7 +90,7 @@ static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, exec_ctx, chand->transport, TRANSPORT_STREAM_FROM_CALL_DATA(calld), &args->call_stack->refcount, args->server_transport_data); return r == 0 ? GRPC_ERROR_NONE - : GRPC_ERROR_CREATE("transport initialization failed"); + : GRPC_ERROR_CREATE("transport stream initialization failed"); } static void set_pollset_or_pollset_set(grpc_exec_ctx *exec_ctx, diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c index 345698092a0..b3bf1427a3e 100644 --- a/src/core/lib/surface/call.c +++ b/src/core/lib/surface/call.c @@ -274,7 +274,7 @@ grpc_call *grpc_call_create( grpc_error_get_str(error, GRPC_ERROR_STR_DESCRIPTION); close_with_status(&exec_ctx, call, (grpc_status_code)status, error_str == NULL ? "unknown error" : error_str); - grpc_error_unref(error); + GRPC_ERROR_UNREF(error); } if (cq != NULL) { GPR_ASSERT( diff --git a/test/core/end2end/tests/filter_call_init_fails.c b/test/core/end2end/tests/filter_call_init_fails.c index 4980b372784..08c377a281c 100644 --- a/test/core/end2end/tests/filter_call_init_fails.c +++ b/test/core/end2end/tests/filter_call_init_fails.c @@ -246,8 +246,10 @@ static bool maybe_add_filter(grpc_channel_stack_builder *builder, void *arg) { grpc_channel_stack_builder_iterator *it = grpc_channel_stack_builder_create_iterator_at_last(builder); GPR_ASSERT(grpc_channel_stack_builder_move_prev(it)); - return grpc_channel_stack_builder_add_filter_before(it, &test_filter, NULL, - NULL); + const bool retval = grpc_channel_stack_builder_add_filter_before( + it, &test_filter, NULL, NULL); + grpc_channel_stack_builder_iterator_destroy(it); + return retval; } else { return true; } diff --git a/third_party/protobuf b/third_party/protobuf index bdeb215cab2..a1938b2aa9c 160000 --- a/third_party/protobuf +++ b/third_party/protobuf @@ -1 +1 @@ -Subproject commit bdeb215cab2985195325fcd5e70c3fa751f46e0f +Subproject commit a1938b2aa9ca86ce7ce50c27ff9737c1008d2a03 From f9eef0bfd32e4a9b18c6fcdb5f73b40baec792e9 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Wed, 27 Jul 2016 17:05:37 +0000 Subject: [PATCH 13/16] Undo accidental revert to old version of protobuf code. --- third_party/protobuf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third_party/protobuf b/third_party/protobuf index a1938b2aa9c..ba52f2b6780 160000 --- a/third_party/protobuf +++ b/third_party/protobuf @@ -1 +1 @@ -Subproject commit a1938b2aa9ca86ce7ce50c27ff9737c1008d2a03 +Subproject commit ba52f2b6780fa5e6bee86cf7e8ee6f6ba617862c From 9fd00425c50800906f4deb2c5aafa9b64217b54c Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Wed, 27 Jul 2016 18:36:26 +0000 Subject: [PATCH 14/16] Yet another attempt to undo my protobuf version screw-up. --- third_party/protobuf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third_party/protobuf b/third_party/protobuf index ba52f2b6780..bdeb215cab2 160000 --- a/third_party/protobuf +++ b/third_party/protobuf @@ -1 +1 @@ -Subproject commit ba52f2b6780fa5e6bee86cf7e8ee6f6ba617862c +Subproject commit bdeb215cab2985195325fcd5e70c3fa751f46e0f From 0ff122e3e55a297643805735986deb7f336c7159 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Tue, 2 Aug 2016 10:07:19 -0700 Subject: [PATCH 15/16] Ran generate_projects.sh. --- tools/run_tests/tests.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/run_tests/tests.json b/tools/run_tests/tests.json index 5cd74ff7edd..99ca67da33d 100644 --- a/tools/run_tests/tests.json +++ b/tools/run_tests/tests.json @@ -10679,7 +10679,7 @@ "exclude_configs": [], "flaky": false, "language": "c", - "name": "h2_loadreporting_test", + "name": "h2_load_reporting_test", "platforms": [ "windows", "linux", @@ -23915,7 +23915,7 @@ "exclude_configs": [], "flaky": false, "language": "c", - "name": "h2_loadreporting_nosec_test", + "name": "h2_load_reporting_nosec_test", "platforms": [ "windows", "linux", From cca4a199e7786b2cee1743cd069551433e141e54 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Tue, 2 Aug 2016 10:36:53 -0700 Subject: [PATCH 16/16] Fix destroy_call_elem API in test. --- test/core/end2end/tests/filter_call_init_fails.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/end2end/tests/filter_call_init_fails.c b/test/core/end2end/tests/filter_call_init_fails.c index 08c377a281c..a09183b7868 100644 --- a/test/core/end2end/tests/filter_call_init_fails.c +++ b/test/core/end2end/tests/filter_call_init_fails.c @@ -210,7 +210,7 @@ static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, } static void destroy_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, - const grpc_call_stats *stats, + const grpc_call_final_info *final_info, void *and_free_memory) {} static void init_channel_elem(grpc_exec_ctx *exec_ctx,