From 86f1c7a5df2eb71e653ab8f227582e40c73a4c5c Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Fri, 7 Sep 2018 16:54:31 -0700 Subject: Be cautious and wait for possible error causing callbacks before we treat trailing metadata --- .../ext/filters/http/client/http_client_filter.cc | 18 ++++++++++++++- .../ext/filters/http/server/http_server_filter.cc | 20 +++++++++++++++- .../filters/message_size/message_size_filter.cc | 18 ++++++++++++++- .../lib/security/transport/server_auth_filter.cc | 27 +++++++++++++++++++--- 4 files changed, 77 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/core/ext/filters/http/client/http_client_filter.cc b/src/core/ext/filters/http/client/http_client_filter.cc index 91fa163fec..c7d7f333a5 100644 --- a/src/core/ext/filters/http/client/http_client_filter.cc +++ b/src/core/ext/filters/http/client/http_client_filter.cc @@ -67,6 +67,8 @@ struct call_data { grpc_closure on_send_message_next_done; grpc_closure* original_send_message_on_complete; grpc_closure send_message_on_complete; + grpc_error* recv_trailing_metadata_err; + bool seen_recv_trailing_metadata_ready; }; struct channel_data { @@ -157,12 +159,25 @@ static void recv_initial_metadata_ready(void* user_data, grpc_error* error) { } else { GRPC_ERROR_REF(error); } - GRPC_CLOSURE_RUN(calld->original_recv_initial_metadata_ready, error); + grpc_closure* closure = calld->original_recv_initial_metadata_ready; + calld->original_recv_initial_metadata_ready = nullptr; + if (calld->seen_recv_trailing_metadata_ready) { + GRPC_CALL_COMBINER_START( + calld->call_combiner, &calld->recv_trailing_metadata_ready, + calld->recv_trailing_metadata_err, "continue recv trailing metadata"); + } + GRPC_CLOSURE_RUN(closure, error); } static void recv_trailing_metadata_ready(void* user_data, grpc_error* error) { grpc_call_element* elem = static_cast(user_data); call_data* calld = static_cast(elem->call_data); + if (calld->original_recv_initial_metadata_ready != nullptr) { + calld->recv_trailing_metadata_err = GRPC_ERROR_REF(error); + calld->seen_recv_trailing_metadata_ready = true; + GRPC_CALL_COMBINER_STOP(calld->call_combiner, "wait for initial metadata"); + return; + } if (error == GRPC_ERROR_NONE) { error = client_filter_incoming_metadata(elem, calld->recv_trailing_metadata); @@ -427,6 +442,7 @@ static grpc_error* init_call_elem(grpc_call_element* elem, const grpc_call_element_args* args) { call_data* calld = static_cast(elem->call_data); calld->call_combiner = args->call_combiner; + calld->seen_recv_trailing_metadata_ready = false; GRPC_CLOSURE_INIT(&calld->recv_initial_metadata_ready, recv_initial_metadata_ready, elem, grpc_schedule_on_exec_ctx); diff --git a/src/core/ext/filters/http/server/http_server_filter.cc b/src/core/ext/filters/http/server/http_server_filter.cc index 926afeec84..484ce9c22f 100644 --- a/src/core/ext/filters/http/server/http_server_filter.cc +++ b/src/core/ext/filters/http/server/http_server_filter.cc @@ -64,6 +64,9 @@ struct call_data { grpc_closure recv_trailing_metadata_ready; grpc_closure* original_recv_trailing_metadata_ready; + + grpc_error* recv_trailing_metadata_ready_error; + bool seen_recv_trailing_metadata_ready; }; } // namespace @@ -291,7 +294,15 @@ static void hs_recv_initial_metadata_ready(void* user_data, grpc_error* err) { } else { GRPC_ERROR_REF(err); } - GRPC_CLOSURE_RUN(calld->original_recv_initial_metadata_ready, err); + grpc_closure* closure = calld->original_recv_initial_metadata_ready; + calld->original_recv_initial_metadata_ready = nullptr; + if (calld->seen_recv_trailing_metadata_ready) { + GRPC_CALL_COMBINER_START(calld->call_combiner, + &calld->recv_trailing_metadata_ready, + calld->recv_trailing_metadata_ready_error, + "continue recv trailing metadata"); + } + GRPC_CLOSURE_RUN(closure, err); } static void hs_recv_message_ready(void* user_data, grpc_error* err) { @@ -321,6 +332,12 @@ static void hs_recv_message_ready(void* user_data, grpc_error* err) { static void hs_recv_trailing_metadata_ready(void* user_data, grpc_error* err) { grpc_call_element* elem = static_cast(user_data); call_data* calld = static_cast(elem->call_data); + if (calld->original_recv_initial_metadata_ready) { + calld->recv_trailing_metadata_ready_error = GRPC_ERROR_REF(err); + calld->seen_recv_trailing_metadata_ready = true; + GRPC_CALL_COMBINER_STOP(calld->call_combiner, "wait for initial metadata"); + return; + } err = grpc_error_add_child( GRPC_ERROR_REF(err), GRPC_ERROR_REF(calld->recv_initial_metadata_ready_error)); @@ -405,6 +422,7 @@ static grpc_error* hs_init_call_elem(grpc_call_element* elem, const grpc_call_element_args* args) { call_data* calld = static_cast(elem->call_data); calld->call_combiner = args->call_combiner; + calld->seen_recv_trailing_metadata_ready = false; GRPC_CLOSURE_INIT(&calld->recv_initial_metadata_ready, hs_recv_initial_metadata_ready, elem, grpc_schedule_on_exec_ctx); 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 c17df86f3d..01d483f45e 100644 --- a/src/core/ext/filters/message_size/message_size_filter.cc +++ b/src/core/ext/filters/message_size/message_size_filter.cc @@ -108,6 +108,8 @@ struct call_data { grpc_closure* next_recv_message_ready; // Original recv_trailing_metadata callback, invoked after our own. grpc_closure* original_recv_trailing_metadata_ready; + bool seen_recv_trailing_metadata; + grpc_error* recv_trailing_metadata_error; }; struct channel_data { @@ -147,7 +149,14 @@ static void recv_message_ready(void* user_data, grpc_error* error) { GRPC_ERROR_REF(error); } // Invoke the next callback. - GRPC_CLOSURE_RUN(calld->next_recv_message_ready, error); + grpc_closure* closure = calld->next_recv_message_ready; + calld->next_recv_message_ready = nullptr; + if (calld->seen_recv_trailing_metadata) { + GRPC_CALL_COMBINER_START( + calld->call_combiner, &calld->recv_trailing_metadata_ready, + calld->recv_trailing_metadata_error, "continue recv trailing metadata"); + } + GRPC_CLOSURE_RUN(closure, error); } // Callback invoked on completion of recv_trailing_metadata @@ -155,6 +164,12 @@ static void recv_message_ready(void* user_data, grpc_error* error) { static void recv_trailing_metadata_ready(void* user_data, grpc_error* error) { grpc_call_element* elem = static_cast(user_data); call_data* calld = static_cast(elem->call_data); + if (calld->next_recv_message_ready) { + calld->seen_recv_trailing_metadata = true; + calld->recv_trailing_metadata_error = GRPC_ERROR_REF(error); + GRPC_CALL_COMBINER_STOP(calld->call_combiner, "wait for recv message"); + return; + } error = grpc_error_add_child(GRPC_ERROR_REF(error), GRPC_ERROR_REF(calld->error)); // Invoke the next callback. @@ -209,6 +224,7 @@ static grpc_error* init_call_elem(grpc_call_element* elem, calld->next_recv_message_ready = nullptr; calld->original_recv_trailing_metadata_ready = nullptr; calld->error = GRPC_ERROR_NONE; + calld->seen_recv_trailing_metadata = false; GRPC_CLOSURE_INIT(&calld->recv_message_ready, recv_message_ready, elem, grpc_schedule_on_exec_ctx); GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, diff --git a/src/core/lib/security/transport/server_auth_filter.cc b/src/core/lib/security/transport/server_auth_filter.cc index 552e70130a..05dfd09ffb 100644 --- a/src/core/lib/security/transport/server_auth_filter.cc +++ b/src/core/lib/security/transport/server_auth_filter.cc @@ -49,6 +49,8 @@ struct call_data { size_t num_consumed_md; grpc_closure cancel_closure; gpr_atm state; // async_state + grpc_error* recv_trailing_metadata_error; + bool seen_recv_trailing_ready; }; struct channel_data { @@ -115,7 +117,14 @@ static void on_md_processing_done_inner(grpc_call_element* elem, remove_consumed_md, elem, "Response metadata filtering error"); } calld->error = GRPC_ERROR_REF(error); - GRPC_CLOSURE_SCHED(calld->original_recv_initial_metadata_ready, error); + grpc_closure* closure = calld->original_recv_initial_metadata_ready; + calld->original_recv_initial_metadata_ready = nullptr; + if (calld->seen_recv_trailing_ready) { + GRPC_CALL_COMBINER_START( + calld->call_combiner, &calld->recv_trailing_metadata_ready, + calld->recv_trailing_metadata_error, "continue recv trailing metadata"); + } + GRPC_CLOSURE_SCHED(closure, error); } // Called from application code. @@ -184,13 +193,24 @@ static void recv_initial_metadata_ready(void* arg, grpc_error* error) { return; } } - GRPC_CLOSURE_RUN(calld->original_recv_initial_metadata_ready, - GRPC_ERROR_REF(error)); + grpc_closure* closure = calld->original_recv_initial_metadata_ready; + calld->original_recv_initial_metadata_ready = nullptr; + if (calld->seen_recv_trailing_ready) { + GRPC_CALL_COMBINER_START( + calld->call_combiner, &calld->recv_trailing_metadata_ready, + calld->recv_trailing_metadata_error, "continue recv trailing metadata"); + } + GRPC_CLOSURE_RUN(closure, GRPC_ERROR_REF(error)); } static void recv_trailing_metadata_ready(void* user_data, grpc_error* err) { grpc_call_element* elem = static_cast(user_data); call_data* calld = static_cast(elem->call_data); + if (calld->original_recv_initial_metadata_ready) { + calld->recv_trailing_metadata_error = GRPC_ERROR_REF(err); + calld->seen_recv_trailing_ready = true; + GRPC_CALL_COMBINER_STOP(calld->call_combiner, "wait for initial metadata"); + } err = grpc_error_add_child(GRPC_ERROR_REF(err), GRPC_ERROR_REF(calld->error)); GRPC_CLOSURE_RUN(calld->original_recv_trailing_metadata_ready, err); } @@ -228,6 +248,7 @@ static grpc_error* init_call_elem(grpc_call_element* elem, GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, recv_trailing_metadata_ready, elem, grpc_schedule_on_exec_ctx); + calld->seen_recv_trailing_ready = false; // Create server security context. Set its auth context from channel // data and save it in the call context. grpc_server_security_context* server_ctx = -- cgit v1.2.3 From 30e7b02b5c8bc55c109ed84dfa30663ce90e134b Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Fri, 7 Sep 2018 17:02:57 -0700 Subject: Also initialize closures again --- src/core/ext/filters/http/client/http_client_filter.cc | 3 +++ src/core/ext/filters/http/server/http_server_filter.cc | 3 +++ src/core/ext/filters/message_size/message_size_filter.cc | 3 +++ src/core/lib/security/transport/server_auth_filter.cc | 3 +++ 4 files changed, 12 insertions(+) (limited to 'src') diff --git a/src/core/ext/filters/http/client/http_client_filter.cc b/src/core/ext/filters/http/client/http_client_filter.cc index c7d7f333a5..e4030afcb4 100644 --- a/src/core/ext/filters/http/client/http_client_filter.cc +++ b/src/core/ext/filters/http/client/http_client_filter.cc @@ -175,6 +175,9 @@ static void recv_trailing_metadata_ready(void* user_data, grpc_error* error) { if (calld->original_recv_initial_metadata_ready != nullptr) { calld->recv_trailing_metadata_err = GRPC_ERROR_REF(error); calld->seen_recv_trailing_metadata_ready = true; + GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, + recv_trailing_metadata_ready, elem, + grpc_schedule_on_exec_ctx); GRPC_CALL_COMBINER_STOP(calld->call_combiner, "wait for initial metadata"); return; } diff --git a/src/core/ext/filters/http/server/http_server_filter.cc b/src/core/ext/filters/http/server/http_server_filter.cc index 484ce9c22f..90336103cf 100644 --- a/src/core/ext/filters/http/server/http_server_filter.cc +++ b/src/core/ext/filters/http/server/http_server_filter.cc @@ -335,6 +335,9 @@ static void hs_recv_trailing_metadata_ready(void* user_data, grpc_error* err) { if (calld->original_recv_initial_metadata_ready) { calld->recv_trailing_metadata_ready_error = GRPC_ERROR_REF(err); calld->seen_recv_trailing_metadata_ready = true; + GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, + hs_recv_trailing_metadata_ready, elem, + grpc_schedule_on_exec_ctx); GRPC_CALL_COMBINER_STOP(calld->call_combiner, "wait for initial metadata"); return; } 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 01d483f45e..d85c299abe 100644 --- a/src/core/ext/filters/message_size/message_size_filter.cc +++ b/src/core/ext/filters/message_size/message_size_filter.cc @@ -167,6 +167,9 @@ static void recv_trailing_metadata_ready(void* user_data, grpc_error* error) { if (calld->next_recv_message_ready) { calld->seen_recv_trailing_metadata = true; calld->recv_trailing_metadata_error = GRPC_ERROR_REF(error); + GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, + recv_trailing_metadata_ready, elem, + grpc_schedule_on_exec_ctx); GRPC_CALL_COMBINER_STOP(calld->call_combiner, "wait for recv message"); return; } diff --git a/src/core/lib/security/transport/server_auth_filter.cc b/src/core/lib/security/transport/server_auth_filter.cc index 05dfd09ffb..882f985959 100644 --- a/src/core/lib/security/transport/server_auth_filter.cc +++ b/src/core/lib/security/transport/server_auth_filter.cc @@ -209,6 +209,9 @@ static void recv_trailing_metadata_ready(void* user_data, grpc_error* err) { if (calld->original_recv_initial_metadata_ready) { calld->recv_trailing_metadata_error = GRPC_ERROR_REF(err); calld->seen_recv_trailing_ready = true; + GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, + recv_trailing_metadata_ready, elem, + grpc_schedule_on_exec_ctx); GRPC_CALL_COMBINER_STOP(calld->call_combiner, "wait for initial metadata"); } err = grpc_error_add_child(GRPC_ERROR_REF(err), GRPC_ERROR_REF(calld->error)); -- cgit v1.2.3 From 4009616b581224748b385ffd3d95b024ec4fc1bb Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Fri, 14 Sep 2018 18:02:30 -0700 Subject: Reviewer comments --- .../ext/filters/http/client/http_client_filter.cc | 13 ++++--- .../ext/filters/http/server/http_server_filter.cc | 16 ++++---- .../filters/message_size/message_size_filter.cc | 14 ++++--- .../lib/security/transport/server_auth_filter.cc | 34 +++++++++-------- src/core/lib/surface/server.cc | 43 +++++++++++++++++----- 5 files changed, 76 insertions(+), 44 deletions(-) (limited to 'src') diff --git a/src/core/ext/filters/http/client/http_client_filter.cc b/src/core/ext/filters/http/client/http_client_filter.cc index e4030afcb4..6b41300f91 100644 --- a/src/core/ext/filters/http/client/http_client_filter.cc +++ b/src/core/ext/filters/http/client/http_client_filter.cc @@ -58,6 +58,8 @@ struct call_data { grpc_metadata_batch* recv_trailing_metadata; grpc_closure* original_recv_trailing_metadata_ready; grpc_closure recv_trailing_metadata_ready; + grpc_error* recv_trailing_metadata_error; + bool seen_recv_trailing_metadata_ready; // State for handling send_message ops. grpc_transport_stream_op_batch* send_message_batch; size_t send_message_bytes_read; @@ -67,8 +69,6 @@ struct call_data { grpc_closure on_send_message_next_done; grpc_closure* original_send_message_on_complete; grpc_closure send_message_on_complete; - grpc_error* recv_trailing_metadata_err; - bool seen_recv_trailing_metadata_ready; }; struct channel_data { @@ -164,7 +164,7 @@ static void recv_initial_metadata_ready(void* user_data, grpc_error* error) { if (calld->seen_recv_trailing_metadata_ready) { GRPC_CALL_COMBINER_START( calld->call_combiner, &calld->recv_trailing_metadata_ready, - calld->recv_trailing_metadata_err, "continue recv trailing metadata"); + calld->recv_trailing_metadata_error, "continue recv_trailing_metadata"); } GRPC_CLOSURE_RUN(closure, error); } @@ -173,12 +173,14 @@ static void recv_trailing_metadata_ready(void* user_data, grpc_error* error) { grpc_call_element* elem = static_cast(user_data); call_data* calld = static_cast(elem->call_data); if (calld->original_recv_initial_metadata_ready != nullptr) { - calld->recv_trailing_metadata_err = GRPC_ERROR_REF(error); + calld->recv_trailing_metadata_error = GRPC_ERROR_REF(error); calld->seen_recv_trailing_metadata_ready = true; GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, recv_trailing_metadata_ready, elem, grpc_schedule_on_exec_ctx); - GRPC_CALL_COMBINER_STOP(calld->call_combiner, "wait for initial metadata"); + GRPC_CALL_COMBINER_STOP(calld->call_combiner, + "deferring recv_trailing_metadata_ready until " + "after recv_initial_metadata_ready"); return; } if (error == GRPC_ERROR_NONE) { @@ -445,7 +447,6 @@ static grpc_error* init_call_elem(grpc_call_element* elem, const grpc_call_element_args* args) { call_data* calld = static_cast(elem->call_data); calld->call_combiner = args->call_combiner; - calld->seen_recv_trailing_metadata_ready = false; GRPC_CLOSURE_INIT(&calld->recv_initial_metadata_ready, recv_initial_metadata_ready, elem, grpc_schedule_on_exec_ctx); diff --git a/src/core/ext/filters/http/server/http_server_filter.cc b/src/core/ext/filters/http/server/http_server_filter.cc index cd3cab134a..d5e0663c68 100644 --- a/src/core/ext/filters/http/server/http_server_filter.cc +++ b/src/core/ext/filters/http/server/http_server_filter.cc @@ -63,9 +63,9 @@ struct call_data { grpc_core::OrphanablePtr* recv_message; bool seen_recv_message_ready; + // State for intercepting recv_trailing_metadata grpc_closure recv_trailing_metadata_ready; grpc_closure* original_recv_trailing_metadata_ready; - grpc_error* recv_trailing_metadata_ready_error; bool seen_recv_trailing_metadata_ready; }; @@ -304,15 +304,14 @@ static void hs_recv_initial_metadata_ready(void* user_data, grpc_error* err) { } else { GRPC_ERROR_REF(err); } - grpc_closure* closure = calld->original_recv_initial_metadata_ready; - calld->original_recv_initial_metadata_ready = nullptr; if (calld->seen_recv_trailing_metadata_ready) { GRPC_CALL_COMBINER_START(calld->call_combiner, &calld->recv_trailing_metadata_ready, calld->recv_trailing_metadata_ready_error, - "continue recv trailing metadata"); + "resuming hs_recv_trailing_metadata_ready from " + "hs_recv_initial_metadata_ready"); } - GRPC_CLOSURE_RUN(closure, err); + GRPC_CLOSURE_RUN(calld->original_recv_initial_metadata_ready, err); } static void hs_recv_message_ready(void* user_data, grpc_error* err) { @@ -342,13 +341,15 @@ static void hs_recv_message_ready(void* user_data, grpc_error* err) { static void hs_recv_trailing_metadata_ready(void* user_data, grpc_error* err) { grpc_call_element* elem = static_cast(user_data); call_data* calld = static_cast(elem->call_data); - if (calld->original_recv_initial_metadata_ready) { + if (!calld->seen_recv_initial_metadata_ready) { calld->recv_trailing_metadata_ready_error = GRPC_ERROR_REF(err); calld->seen_recv_trailing_metadata_ready = true; GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, hs_recv_trailing_metadata_ready, elem, grpc_schedule_on_exec_ctx); - GRPC_CALL_COMBINER_STOP(calld->call_combiner, "wait for initial metadata"); + GRPC_CALL_COMBINER_STOP(calld->call_combiner, + "deferring hs_recv_trailing_metadata_ready until " + "ater hs_recv_initial_metadata_ready"); return; } err = grpc_error_add_child( @@ -435,7 +436,6 @@ static grpc_error* hs_init_call_elem(grpc_call_element* elem, const grpc_call_element_args* args) { call_data* calld = static_cast(elem->call_data); calld->call_combiner = args->call_combiner; - calld->seen_recv_trailing_metadata_ready = false; GRPC_CLOSURE_INIT(&calld->recv_initial_metadata_ready, hs_recv_initial_metadata_ready, elem, grpc_schedule_on_exec_ctx); 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 d85c299abe..a32ba21c30 100644 --- a/src/core/ext/filters/message_size/message_size_filter.cc +++ b/src/core/ext/filters/message_size/message_size_filter.cc @@ -152,9 +152,10 @@ static void recv_message_ready(void* user_data, grpc_error* error) { grpc_closure* closure = calld->next_recv_message_ready; calld->next_recv_message_ready = nullptr; if (calld->seen_recv_trailing_metadata) { - GRPC_CALL_COMBINER_START( - calld->call_combiner, &calld->recv_trailing_metadata_ready, - calld->recv_trailing_metadata_error, "continue recv trailing metadata"); + GRPC_CALL_COMBINER_START(calld->call_combiner, + &calld->recv_trailing_metadata_ready, + calld->recv_trailing_metadata_error, + "continue recv_trailing_metadata_ready"); } GRPC_CLOSURE_RUN(closure, error); } @@ -164,13 +165,15 @@ static void recv_message_ready(void* user_data, grpc_error* error) { static void recv_trailing_metadata_ready(void* user_data, grpc_error* error) { grpc_call_element* elem = static_cast(user_data); call_data* calld = static_cast(elem->call_data); - if (calld->next_recv_message_ready) { + if (calld->next_recv_message_ready != nullptr) { calld->seen_recv_trailing_metadata = true; calld->recv_trailing_metadata_error = GRPC_ERROR_REF(error); GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, recv_trailing_metadata_ready, elem, grpc_schedule_on_exec_ctx); - GRPC_CALL_COMBINER_STOP(calld->call_combiner, "wait for recv message"); + GRPC_CALL_COMBINER_STOP(calld->call_combiner, + "deferring recv_trailing_metadata_ready until " + "after recv_message_ready"); return; } error = @@ -227,7 +230,6 @@ static grpc_error* init_call_elem(grpc_call_element* elem, calld->next_recv_message_ready = nullptr; calld->original_recv_trailing_metadata_ready = nullptr; calld->error = GRPC_ERROR_NONE; - calld->seen_recv_trailing_metadata = false; GRPC_CLOSURE_INIT(&calld->recv_message_ready, recv_message_ready, elem, grpc_schedule_on_exec_ctx); GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, diff --git a/src/core/lib/security/transport/server_auth_filter.cc b/src/core/lib/security/transport/server_auth_filter.cc index 882f985959..352355ea67 100644 --- a/src/core/lib/security/transport/server_auth_filter.cc +++ b/src/core/lib/security/transport/server_auth_filter.cc @@ -41,16 +41,16 @@ struct call_data { grpc_transport_stream_op_batch* recv_initial_metadata_batch; grpc_closure* original_recv_initial_metadata_ready; grpc_closure recv_initial_metadata_ready; - grpc_error* error; + grpc_error* recv_initial_metadata_error; grpc_closure recv_trailing_metadata_ready; grpc_closure* original_recv_trailing_metadata_ready; + grpc_error* recv_trailing_metadata_error; + bool seen_recv_trailing_ready; grpc_metadata_array md; const grpc_metadata* consumed_md; size_t num_consumed_md; grpc_closure cancel_closure; gpr_atm state; // async_state - grpc_error* recv_trailing_metadata_error; - bool seen_recv_trailing_ready; }; struct channel_data { @@ -116,13 +116,14 @@ static void on_md_processing_done_inner(grpc_call_element* elem, batch->payload->recv_initial_metadata.recv_initial_metadata, remove_consumed_md, elem, "Response metadata filtering error"); } - calld->error = GRPC_ERROR_REF(error); + calld->recv_initial_metadata_error = GRPC_ERROR_REF(error); grpc_closure* closure = calld->original_recv_initial_metadata_ready; calld->original_recv_initial_metadata_ready = nullptr; if (calld->seen_recv_trailing_ready) { - GRPC_CALL_COMBINER_START( - calld->call_combiner, &calld->recv_trailing_metadata_ready, - calld->recv_trailing_metadata_error, "continue recv trailing metadata"); + GRPC_CALL_COMBINER_START(calld->call_combiner, + &calld->recv_trailing_metadata_ready, + calld->recv_trailing_metadata_error, + "continue recv_trailing_metadata_ready"); } GRPC_CLOSURE_SCHED(closure, error); } @@ -196,9 +197,10 @@ static void recv_initial_metadata_ready(void* arg, grpc_error* error) { grpc_closure* closure = calld->original_recv_initial_metadata_ready; calld->original_recv_initial_metadata_ready = nullptr; if (calld->seen_recv_trailing_ready) { - GRPC_CALL_COMBINER_START( - calld->call_combiner, &calld->recv_trailing_metadata_ready, - calld->recv_trailing_metadata_error, "continue recv trailing metadata"); + GRPC_CALL_COMBINER_START(calld->call_combiner, + &calld->recv_trailing_metadata_ready, + calld->recv_trailing_metadata_error, + "continue recv_trailing_metadata_ready"); } GRPC_CLOSURE_RUN(closure, GRPC_ERROR_REF(error)); } @@ -206,15 +208,18 @@ static void recv_initial_metadata_ready(void* arg, grpc_error* error) { static void recv_trailing_metadata_ready(void* user_data, grpc_error* err) { grpc_call_element* elem = static_cast(user_data); call_data* calld = static_cast(elem->call_data); - if (calld->original_recv_initial_metadata_ready) { + if (calld->original_recv_initial_metadata_ready != nullptr) { calld->recv_trailing_metadata_error = GRPC_ERROR_REF(err); calld->seen_recv_trailing_ready = true; GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, recv_trailing_metadata_ready, elem, grpc_schedule_on_exec_ctx); - GRPC_CALL_COMBINER_STOP(calld->call_combiner, "wait for initial metadata"); + GRPC_CALL_COMBINER_STOP(calld->call_combiner, + "deferring recv_trailing_metadata_ready until " + "after recv_initial_metadata_ready"); } - err = grpc_error_add_child(GRPC_ERROR_REF(err), GRPC_ERROR_REF(calld->error)); + err = grpc_error_add_child( + GRPC_ERROR_REF(err), GRPC_ERROR_REF(calld->recv_initial_metadata_error)); GRPC_CLOSURE_RUN(calld->original_recv_trailing_metadata_ready, err); } @@ -251,7 +256,6 @@ static grpc_error* init_call_elem(grpc_call_element* elem, GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, recv_trailing_metadata_ready, elem, grpc_schedule_on_exec_ctx); - calld->seen_recv_trailing_ready = false; // Create server security context. Set its auth context from channel // data and save it in the call context. grpc_server_security_context* server_ctx = @@ -273,7 +277,7 @@ static void destroy_call_elem(grpc_call_element* elem, const grpc_call_final_info* final_info, grpc_closure* ignored) { call_data* calld = static_cast(elem->call_data); - GRPC_ERROR_UNREF(calld->error); + GRPC_ERROR_UNREF(calld->recv_initial_metadata_error); } /* Constructor for channel_data */ diff --git a/src/core/lib/surface/server.cc b/src/core/lib/surface/server.cc index 5fa58ffdec..1b22db98eb 100644 --- a/src/core/lib/surface/server.cc +++ b/src/core/lib/surface/server.cc @@ -150,12 +150,15 @@ struct call_data { grpc_closure kill_zombie_closure; grpc_closure* on_done_recv_initial_metadata; grpc_closure recv_trailing_metadata_ready; - grpc_error* error; + grpc_error* recv_initial_metadata_error; grpc_closure* original_recv_trailing_metadata_ready; + grpc_error* recv_trailing_metadata_error; + bool seen_recv_trailing_metadata_ready; grpc_closure publish; call_data* pending_next; + grpc_call_combiner* call_combiner; }; struct request_matcher { @@ -730,18 +733,39 @@ static void server_on_recv_initial_metadata(void* ptr, grpc_error* error) { grpc_error* src_error = error; error = GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( "Missing :authority or :path", &error, 1); - GRPC_ERROR_UNREF(src_error); - } - - GRPC_CLOSURE_RUN(calld->on_done_recv_initial_metadata, error); + /* Pass the src_error reference to calld->recv_initial_metadata_error */ + calld->recv_initial_metadata_error = src_error; + } + grpc_closure* closure = calld->on_done_recv_initial_metadata; + calld->on_done_recv_initial_metadata = nullptr; + if (calld->seen_recv_trailing_metadata_ready) { + GRPC_CALL_COMBINER_START(calld->call_combiner, + &calld->recv_trailing_metadata_ready, + calld->recv_trailing_metadata_error, + "continue server_recv_trailing_metadata_ready"); + } + GRPC_CLOSURE_RUN(closure, error); } static void server_recv_trailing_metadata_ready(void* user_data, - grpc_error* err) { + grpc_error* error) { grpc_call_element* elem = static_cast(user_data); call_data* calld = static_cast(elem->call_data); - err = grpc_error_add_child(GRPC_ERROR_REF(err), GRPC_ERROR_REF(calld->error)); - GRPC_CLOSURE_RUN(calld->original_recv_trailing_metadata_ready, err); + if (calld->on_done_recv_initial_metadata != nullptr) { + calld->recv_trailing_metadata_error = GRPC_ERROR_REF(error); + calld->seen_recv_trailing_metadata_ready = true; + GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, + server_recv_trailing_metadata_ready, elem, + grpc_schedule_on_exec_ctx); + GRPC_CALL_COMBINER_STOP(calld->call_combiner, + "deferring server_recv_trailing_metadata_ready " + "until after server_on_recv_initial_metadata"); + return; + } + error = + grpc_error_add_child(GRPC_ERROR_REF(error), + GRPC_ERROR_REF(calld->recv_initial_metadata_error)); + GRPC_CLOSURE_RUN(calld->original_recv_trailing_metadata_ready, error); } static void server_mutate_op(grpc_call_element* elem, @@ -845,6 +869,7 @@ static grpc_error* init_call_elem(grpc_call_element* elem, memset(calld, 0, sizeof(call_data)); calld->deadline = GRPC_MILLIS_INF_FUTURE; calld->call = grpc_call_from_top_element(elem); + calld->call_combiner = args->call_combiner; GRPC_CLOSURE_INIT(&calld->server_on_recv_initial_metadata, server_on_recv_initial_metadata, elem, @@ -863,7 +888,7 @@ static void destroy_call_elem(grpc_call_element* elem, call_data* calld = static_cast(elem->call_data); GPR_ASSERT(calld->state != PENDING); - GRPC_ERROR_UNREF(calld->error); + GRPC_ERROR_UNREF(calld->recv_initial_metadata_error); if (calld->host_set) { grpc_slice_unref_internal(calld->host); } -- cgit v1.2.3 From 98048811a46b107e44f82e02a500e35442079529 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Mon, 17 Sep 2018 13:43:56 -0700 Subject: Reviewer comments --- src/core/ext/filters/http/client/http_client_filter.cc | 3 --- src/core/ext/filters/http/server/http_server_filter.cc | 3 --- src/core/ext/filters/message_size/message_size_filter.cc | 3 --- src/core/lib/security/transport/server_auth_filter.cc | 12 +++++------- src/core/lib/surface/server.cc | 9 ++++----- 5 files changed, 9 insertions(+), 21 deletions(-) (limited to 'src') diff --git a/src/core/ext/filters/http/client/http_client_filter.cc b/src/core/ext/filters/http/client/http_client_filter.cc index 6b41300f91..cd459e47cd 100644 --- a/src/core/ext/filters/http/client/http_client_filter.cc +++ b/src/core/ext/filters/http/client/http_client_filter.cc @@ -175,9 +175,6 @@ static void recv_trailing_metadata_ready(void* user_data, grpc_error* error) { if (calld->original_recv_initial_metadata_ready != nullptr) { calld->recv_trailing_metadata_error = GRPC_ERROR_REF(error); calld->seen_recv_trailing_metadata_ready = true; - GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, - recv_trailing_metadata_ready, elem, - grpc_schedule_on_exec_ctx); GRPC_CALL_COMBINER_STOP(calld->call_combiner, "deferring recv_trailing_metadata_ready until " "after recv_initial_metadata_ready"); diff --git a/src/core/ext/filters/http/server/http_server_filter.cc b/src/core/ext/filters/http/server/http_server_filter.cc index d5e0663c68..436ea09d94 100644 --- a/src/core/ext/filters/http/server/http_server_filter.cc +++ b/src/core/ext/filters/http/server/http_server_filter.cc @@ -344,9 +344,6 @@ static void hs_recv_trailing_metadata_ready(void* user_data, grpc_error* err) { if (!calld->seen_recv_initial_metadata_ready) { calld->recv_trailing_metadata_ready_error = GRPC_ERROR_REF(err); calld->seen_recv_trailing_metadata_ready = true; - GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, - hs_recv_trailing_metadata_ready, elem, - grpc_schedule_on_exec_ctx); GRPC_CALL_COMBINER_STOP(calld->call_combiner, "deferring hs_recv_trailing_metadata_ready until " "ater hs_recv_initial_metadata_ready"); 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 a32ba21c30..6396d65606 100644 --- a/src/core/ext/filters/message_size/message_size_filter.cc +++ b/src/core/ext/filters/message_size/message_size_filter.cc @@ -168,9 +168,6 @@ static void recv_trailing_metadata_ready(void* user_data, grpc_error* error) { if (calld->next_recv_message_ready != nullptr) { calld->seen_recv_trailing_metadata = true; calld->recv_trailing_metadata_error = GRPC_ERROR_REF(error); - GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, - recv_trailing_metadata_ready, elem, - grpc_schedule_on_exec_ctx); GRPC_CALL_COMBINER_STOP(calld->call_combiner, "deferring recv_trailing_metadata_ready until " "after recv_message_ready"); diff --git a/src/core/lib/security/transport/server_auth_filter.cc b/src/core/lib/security/transport/server_auth_filter.cc index 352355ea67..b99fc5e178 100644 --- a/src/core/lib/security/transport/server_auth_filter.cc +++ b/src/core/lib/security/transport/server_auth_filter.cc @@ -45,7 +45,7 @@ struct call_data { grpc_closure recv_trailing_metadata_ready; grpc_closure* original_recv_trailing_metadata_ready; grpc_error* recv_trailing_metadata_error; - bool seen_recv_trailing_ready; + bool seen_recv_trailing_metadata_ready; grpc_metadata_array md; const grpc_metadata* consumed_md; size_t num_consumed_md; @@ -119,7 +119,7 @@ static void on_md_processing_done_inner(grpc_call_element* elem, calld->recv_initial_metadata_error = GRPC_ERROR_REF(error); grpc_closure* closure = calld->original_recv_initial_metadata_ready; calld->original_recv_initial_metadata_ready = nullptr; - if (calld->seen_recv_trailing_ready) { + if (calld->seen_recv_trailing_metadata_ready) { GRPC_CALL_COMBINER_START(calld->call_combiner, &calld->recv_trailing_metadata_ready, calld->recv_trailing_metadata_error, @@ -196,7 +196,7 @@ static void recv_initial_metadata_ready(void* arg, grpc_error* error) { } grpc_closure* closure = calld->original_recv_initial_metadata_ready; calld->original_recv_initial_metadata_ready = nullptr; - if (calld->seen_recv_trailing_ready) { + if (calld->seen_recv_trailing_metadata_ready) { GRPC_CALL_COMBINER_START(calld->call_combiner, &calld->recv_trailing_metadata_ready, calld->recv_trailing_metadata_error, @@ -210,13 +210,11 @@ static void recv_trailing_metadata_ready(void* user_data, grpc_error* err) { call_data* calld = static_cast(elem->call_data); if (calld->original_recv_initial_metadata_ready != nullptr) { calld->recv_trailing_metadata_error = GRPC_ERROR_REF(err); - calld->seen_recv_trailing_ready = true; - GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, - recv_trailing_metadata_ready, elem, - grpc_schedule_on_exec_ctx); + calld->seen_recv_trailing_metadata_ready = true; GRPC_CALL_COMBINER_STOP(calld->call_combiner, "deferring recv_trailing_metadata_ready until " "after recv_initial_metadata_ready"); + return; } err = grpc_error_add_child( GRPC_ERROR_REF(err), GRPC_ERROR_REF(calld->recv_initial_metadata_error)); diff --git a/src/core/lib/surface/server.cc b/src/core/lib/surface/server.cc index 1b22db98eb..ce6446fcaa 100644 --- a/src/core/lib/surface/server.cc +++ b/src/core/lib/surface/server.cc @@ -730,11 +730,10 @@ static void server_on_recv_initial_metadata(void* ptr, grpc_error* error) { if (calld->host_set && calld->path_set) { /* do nothing */ } else { - grpc_error* src_error = error; - error = GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( - "Missing :authority or :path", &error, 1); - /* Pass the src_error reference to calld->recv_initial_metadata_error */ - calld->recv_initial_metadata_error = src_error; + /* Pass the error reference to calld->recv_initial_metadata_error */ + calld->recv_initial_metadata_error = + GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( + "Missing :authority or :path", &error, 1); } grpc_closure* closure = calld->on_done_recv_initial_metadata; calld->on_done_recv_initial_metadata = nullptr; -- cgit v1.2.3 From 23ddadb7ca9cd3d30b2440501fb7d1fd4f8859cb Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Mon, 17 Sep 2018 15:00:03 -0700 Subject: Noob mistake --- src/core/lib/surface/server.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/core/lib/surface/server.cc b/src/core/lib/surface/server.cc index ce6446fcaa..72ddc2648d 100644 --- a/src/core/lib/surface/server.cc +++ b/src/core/lib/surface/server.cc @@ -731,9 +731,11 @@ static void server_on_recv_initial_metadata(void* ptr, grpc_error* error) { /* do nothing */ } else { /* Pass the error reference to calld->recv_initial_metadata_error */ - calld->recv_initial_metadata_error = - GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( - "Missing :authority or :path", &error, 1); + grpc_error* src_error = error; + error = GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( + "Missing :authority or :path", &src_error, 1); + GRPC_ERROR_UNREF(src_error); + calld->recv_initial_metadata_error = GRPC_ERROR_REF(error); } grpc_closure* closure = calld->on_done_recv_initial_metadata; calld->on_done_recv_initial_metadata = nullptr; -- cgit v1.2.3 From 4e7ede8dd21e831362547c1ba46607b9b0f81b76 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Thu, 20 Sep 2018 18:30:42 -0700 Subject: Set seen_recv_trailing_metadata to false --- .../filters/message_size/message_size_filter.cc | 1 + src/core/lib/surface/server.cc | 42 +++++----------------- 2 files changed, 9 insertions(+), 34 deletions(-) (limited to 'src') 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 6396d65606..6a3e14aa13 100644 --- a/src/core/ext/filters/message_size/message_size_filter.cc +++ b/src/core/ext/filters/message_size/message_size_filter.cc @@ -152,6 +152,7 @@ static void recv_message_ready(void* user_data, grpc_error* error) { grpc_closure* closure = calld->next_recv_message_ready; calld->next_recv_message_ready = nullptr; if (calld->seen_recv_trailing_metadata) { + calld->seen_recv_trailing_metadata = false; GRPC_CALL_COMBINER_START(calld->call_combiner, &calld->recv_trailing_metadata_ready, calld->recv_trailing_metadata_error, diff --git a/src/core/lib/surface/server.cc b/src/core/lib/surface/server.cc index 72ddc2648d..5fa58ffdec 100644 --- a/src/core/lib/surface/server.cc +++ b/src/core/lib/surface/server.cc @@ -150,15 +150,12 @@ struct call_data { grpc_closure kill_zombie_closure; grpc_closure* on_done_recv_initial_metadata; grpc_closure recv_trailing_metadata_ready; - grpc_error* recv_initial_metadata_error; + grpc_error* error; grpc_closure* original_recv_trailing_metadata_ready; - grpc_error* recv_trailing_metadata_error; - bool seen_recv_trailing_metadata_ready; grpc_closure publish; call_data* pending_next; - grpc_call_combiner* call_combiner; }; struct request_matcher { @@ -730,43 +727,21 @@ static void server_on_recv_initial_metadata(void* ptr, grpc_error* error) { if (calld->host_set && calld->path_set) { /* do nothing */ } else { - /* Pass the error reference to calld->recv_initial_metadata_error */ grpc_error* src_error = error; error = GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( - "Missing :authority or :path", &src_error, 1); + "Missing :authority or :path", &error, 1); GRPC_ERROR_UNREF(src_error); - calld->recv_initial_metadata_error = GRPC_ERROR_REF(error); } - grpc_closure* closure = calld->on_done_recv_initial_metadata; - calld->on_done_recv_initial_metadata = nullptr; - if (calld->seen_recv_trailing_metadata_ready) { - GRPC_CALL_COMBINER_START(calld->call_combiner, - &calld->recv_trailing_metadata_ready, - calld->recv_trailing_metadata_error, - "continue server_recv_trailing_metadata_ready"); - } - GRPC_CLOSURE_RUN(closure, error); + + GRPC_CLOSURE_RUN(calld->on_done_recv_initial_metadata, error); } static void server_recv_trailing_metadata_ready(void* user_data, - grpc_error* error) { + grpc_error* err) { grpc_call_element* elem = static_cast(user_data); call_data* calld = static_cast(elem->call_data); - if (calld->on_done_recv_initial_metadata != nullptr) { - calld->recv_trailing_metadata_error = GRPC_ERROR_REF(error); - calld->seen_recv_trailing_metadata_ready = true; - GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, - server_recv_trailing_metadata_ready, elem, - grpc_schedule_on_exec_ctx); - GRPC_CALL_COMBINER_STOP(calld->call_combiner, - "deferring server_recv_trailing_metadata_ready " - "until after server_on_recv_initial_metadata"); - return; - } - error = - grpc_error_add_child(GRPC_ERROR_REF(error), - GRPC_ERROR_REF(calld->recv_initial_metadata_error)); - GRPC_CLOSURE_RUN(calld->original_recv_trailing_metadata_ready, error); + err = grpc_error_add_child(GRPC_ERROR_REF(err), GRPC_ERROR_REF(calld->error)); + GRPC_CLOSURE_RUN(calld->original_recv_trailing_metadata_ready, err); } static void server_mutate_op(grpc_call_element* elem, @@ -870,7 +845,6 @@ static grpc_error* init_call_elem(grpc_call_element* elem, memset(calld, 0, sizeof(call_data)); calld->deadline = GRPC_MILLIS_INF_FUTURE; calld->call = grpc_call_from_top_element(elem); - calld->call_combiner = args->call_combiner; GRPC_CLOSURE_INIT(&calld->server_on_recv_initial_metadata, server_on_recv_initial_metadata, elem, @@ -889,7 +863,7 @@ static void destroy_call_elem(grpc_call_element* elem, call_data* calld = static_cast(elem->call_data); GPR_ASSERT(calld->state != PENDING); - GRPC_ERROR_UNREF(calld->recv_initial_metadata_error); + GRPC_ERROR_UNREF(calld->error); if (calld->host_set) { grpc_slice_unref_internal(calld->host); } -- cgit v1.2.3 From a12740f0ae20cbead056df8c2231052c218d0d02 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Fri, 21 Sep 2018 10:59:44 -0700 Subject: Revert the revert to server.cc --- src/core/lib/surface/server.cc | 42 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/core/lib/surface/server.cc b/src/core/lib/surface/server.cc index 5fa58ffdec..72ddc2648d 100644 --- a/src/core/lib/surface/server.cc +++ b/src/core/lib/surface/server.cc @@ -150,12 +150,15 @@ struct call_data { grpc_closure kill_zombie_closure; grpc_closure* on_done_recv_initial_metadata; grpc_closure recv_trailing_metadata_ready; - grpc_error* error; + grpc_error* recv_initial_metadata_error; grpc_closure* original_recv_trailing_metadata_ready; + grpc_error* recv_trailing_metadata_error; + bool seen_recv_trailing_metadata_ready; grpc_closure publish; call_data* pending_next; + grpc_call_combiner* call_combiner; }; struct request_matcher { @@ -727,21 +730,43 @@ static void server_on_recv_initial_metadata(void* ptr, grpc_error* error) { if (calld->host_set && calld->path_set) { /* do nothing */ } else { + /* Pass the error reference to calld->recv_initial_metadata_error */ grpc_error* src_error = error; error = GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( - "Missing :authority or :path", &error, 1); + "Missing :authority or :path", &src_error, 1); GRPC_ERROR_UNREF(src_error); + calld->recv_initial_metadata_error = GRPC_ERROR_REF(error); } - - GRPC_CLOSURE_RUN(calld->on_done_recv_initial_metadata, error); + grpc_closure* closure = calld->on_done_recv_initial_metadata; + calld->on_done_recv_initial_metadata = nullptr; + if (calld->seen_recv_trailing_metadata_ready) { + GRPC_CALL_COMBINER_START(calld->call_combiner, + &calld->recv_trailing_metadata_ready, + calld->recv_trailing_metadata_error, + "continue server_recv_trailing_metadata_ready"); + } + GRPC_CLOSURE_RUN(closure, error); } static void server_recv_trailing_metadata_ready(void* user_data, - grpc_error* err) { + grpc_error* error) { grpc_call_element* elem = static_cast(user_data); call_data* calld = static_cast(elem->call_data); - err = grpc_error_add_child(GRPC_ERROR_REF(err), GRPC_ERROR_REF(calld->error)); - GRPC_CLOSURE_RUN(calld->original_recv_trailing_metadata_ready, err); + if (calld->on_done_recv_initial_metadata != nullptr) { + calld->recv_trailing_metadata_error = GRPC_ERROR_REF(error); + calld->seen_recv_trailing_metadata_ready = true; + GRPC_CLOSURE_INIT(&calld->recv_trailing_metadata_ready, + server_recv_trailing_metadata_ready, elem, + grpc_schedule_on_exec_ctx); + GRPC_CALL_COMBINER_STOP(calld->call_combiner, + "deferring server_recv_trailing_metadata_ready " + "until after server_on_recv_initial_metadata"); + return; + } + error = + grpc_error_add_child(GRPC_ERROR_REF(error), + GRPC_ERROR_REF(calld->recv_initial_metadata_error)); + GRPC_CLOSURE_RUN(calld->original_recv_trailing_metadata_ready, error); } static void server_mutate_op(grpc_call_element* elem, @@ -845,6 +870,7 @@ static grpc_error* init_call_elem(grpc_call_element* elem, memset(calld, 0, sizeof(call_data)); calld->deadline = GRPC_MILLIS_INF_FUTURE; calld->call = grpc_call_from_top_element(elem); + calld->call_combiner = args->call_combiner; GRPC_CLOSURE_INIT(&calld->server_on_recv_initial_metadata, server_on_recv_initial_metadata, elem, @@ -863,7 +889,7 @@ static void destroy_call_elem(grpc_call_element* elem, call_data* calld = static_cast(elem->call_data); GPR_ASSERT(calld->state != PENDING); - GRPC_ERROR_UNREF(calld->error); + GRPC_ERROR_UNREF(calld->recv_initial_metadata_error); if (calld->host_set) { grpc_slice_unref_internal(calld->host); } -- cgit v1.2.3 From cd1992bc04f01279be9e00226633fa5984713cd6 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Fri, 21 Sep 2018 16:50:14 -0700 Subject: Add comment --- src/core/ext/filters/message_size/message_size_filter.cc | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src') 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 6a3e14aa13..2d3b16d992 100644 --- a/src/core/ext/filters/message_size/message_size_filter.cc +++ b/src/core/ext/filters/message_size/message_size_filter.cc @@ -152,6 +152,11 @@ static void recv_message_ready(void* user_data, grpc_error* error) { grpc_closure* closure = calld->next_recv_message_ready; calld->next_recv_message_ready = nullptr; if (calld->seen_recv_trailing_metadata) { + /* We might potentially see another RECV_MESSAGE op. In that case, we do not + * want to run the recv_trailing_metadata_ready closure again. The newer + * RECV_MESSAGE op cannot cause any errors since the transport has already + * invoked the recv_trailing_metadata_ready closure and all further + * RECV_MESSAGE ops will get null payloads. */ calld->seen_recv_trailing_metadata = false; GRPC_CALL_COMBINER_START(calld->call_combiner, &calld->recv_trailing_metadata_ready, -- cgit v1.2.3