From f757028ea41b7d6ddb46de4da6e3030da0bdbceb Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Tue, 2 Jul 2019 11:54:57 -0700 Subject: [PATCH 1/2] Sched combiner closures instead of running to avoid data races --- .../transport/chttp2/transport/chttp2_transport.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index 48c3d002bbd..b05a48a6a0c 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -1232,10 +1232,10 @@ static grpc_closure* add_closure_barrier(grpc_closure* closure) { return closure; } -static void null_then_run_closure(grpc_closure** closure, grpc_error* error) { +static void null_then_sched_closure(grpc_closure** closure, grpc_error* error) { grpc_closure* c = *closure; *closure = nullptr; - GRPC_CLOSURE_RUN(c, error); + GRPC_CLOSURE_SCHED(c, error); } void grpc_chttp2_complete_closure_step(grpc_chttp2_transport* t, @@ -1902,7 +1902,7 @@ void grpc_chttp2_maybe_complete_recv_initial_metadata(grpc_chttp2_transport* t, } grpc_chttp2_incoming_metadata_buffer_publish(&s->metadata_buffer[0], s->recv_initial_metadata); - null_then_run_closure(&s->recv_initial_metadata_ready, GRPC_ERROR_NONE); + null_then_sched_closure(&s->recv_initial_metadata_ready, GRPC_ERROR_NONE); } } @@ -1982,10 +1982,10 @@ void grpc_chttp2_maybe_complete_recv_message(grpc_chttp2_transport* t, s->unprocessed_incoming_frames_buffer_cached_length = s->unprocessed_incoming_frames_buffer.length; if (error == GRPC_ERROR_NONE && *s->recv_message != nullptr) { - null_then_run_closure(&s->recv_message_ready, GRPC_ERROR_NONE); + null_then_sched_closure(&s->recv_message_ready, GRPC_ERROR_NONE); } else if (s->published_metadata[1] != GRPC_METADATA_NOT_PUBLISHED) { *s->recv_message = nullptr; - null_then_run_closure(&s->recv_message_ready, GRPC_ERROR_NONE); + null_then_sched_closure(&s->recv_message_ready, GRPC_ERROR_NONE); } GRPC_ERROR_UNREF(error); } @@ -2051,8 +2051,8 @@ void grpc_chttp2_maybe_complete_recv_trailing_metadata(grpc_chttp2_transport* t, s->collecting_stats = nullptr; grpc_chttp2_incoming_metadata_buffer_publish(&s->metadata_buffer[1], s->recv_trailing_metadata); - null_then_run_closure(&s->recv_trailing_metadata_finished, - GRPC_ERROR_NONE); + null_then_sched_closure(&s->recv_trailing_metadata_finished, + GRPC_ERROR_NONE); } } } From 7e3a9b6b236174000e673771cfbae22a42d05621 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Wed, 10 Jul 2019 14:54:44 -0700 Subject: [PATCH 2/2] Remove error arg - reviewer comment --- .../transport/chttp2/transport/chttp2_transport.cc | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index b05a48a6a0c..6254369ef69 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -1232,10 +1232,10 @@ static grpc_closure* add_closure_barrier(grpc_closure* closure) { return closure; } -static void null_then_sched_closure(grpc_closure** closure, grpc_error* error) { +static void null_then_sched_closure(grpc_closure** closure) { grpc_closure* c = *closure; *closure = nullptr; - GRPC_CLOSURE_SCHED(c, error); + GRPC_CLOSURE_SCHED(c, GRPC_ERROR_NONE); } void grpc_chttp2_complete_closure_step(grpc_chttp2_transport* t, @@ -1902,7 +1902,7 @@ void grpc_chttp2_maybe_complete_recv_initial_metadata(grpc_chttp2_transport* t, } grpc_chttp2_incoming_metadata_buffer_publish(&s->metadata_buffer[0], s->recv_initial_metadata); - null_then_sched_closure(&s->recv_initial_metadata_ready, GRPC_ERROR_NONE); + null_then_sched_closure(&s->recv_initial_metadata_ready); } } @@ -1982,10 +1982,10 @@ void grpc_chttp2_maybe_complete_recv_message(grpc_chttp2_transport* t, s->unprocessed_incoming_frames_buffer_cached_length = s->unprocessed_incoming_frames_buffer.length; if (error == GRPC_ERROR_NONE && *s->recv_message != nullptr) { - null_then_sched_closure(&s->recv_message_ready, GRPC_ERROR_NONE); + null_then_sched_closure(&s->recv_message_ready); } else if (s->published_metadata[1] != GRPC_METADATA_NOT_PUBLISHED) { *s->recv_message = nullptr; - null_then_sched_closure(&s->recv_message_ready, GRPC_ERROR_NONE); + null_then_sched_closure(&s->recv_message_ready); } GRPC_ERROR_UNREF(error); } @@ -2051,8 +2051,7 @@ void grpc_chttp2_maybe_complete_recv_trailing_metadata(grpc_chttp2_transport* t, s->collecting_stats = nullptr; grpc_chttp2_incoming_metadata_buffer_publish(&s->metadata_buffer[1], s->recv_trailing_metadata); - null_then_sched_closure(&s->recv_trailing_metadata_finished, - GRPC_ERROR_NONE); + null_then_sched_closure(&s->recv_trailing_metadata_finished); } } }