From bec72481b597f8e47c5e624af9639eafa16f0993 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Wed, 10 Apr 2019 16:19:13 -0700 Subject: [PATCH 1/2] Fix thread safety issue --- src/core/ext/transport/cronet/transport/cronet_transport.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.cc b/src/core/ext/transport/cronet/transport/cronet_transport.cc index 9551b4ba496..7ae178612c5 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.cc +++ b/src/core/ext/transport/cronet/transport/cronet_transport.cc @@ -177,7 +177,7 @@ struct op_and_state { bool done = false; struct stream_obj* s; /* Pointer back to the stream object */ /* next op_and_state in the linked list */ - struct op_and_state* next; + struct op_and_state* next = nullptr; }; struct op_storage { @@ -324,7 +324,7 @@ static grpc_error* make_error_with_desc(int error_code, const char* desc) { inline op_and_state::op_and_state(stream_obj* s, const grpc_transport_stream_op_batch& op) - : op(op), state(s->arena), s(s), next(s->storage.head) {} + : op(op), state(s->arena), s(s) {} /* Add a new stream op to op storage. @@ -339,6 +339,7 @@ static void add_to_storage(struct stream_obj* s, // TODO (mxyan): check if this is indeed necessary. new_op->done = false; gpr_mu_lock(&s->mu); + new_op->next = storage->head; storage->head = new_op; storage->num_pending_ops++; if (op->send_message) { From f371fce8871fbf67ed8af0fd3d51919d3bdacf5b Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Wed, 10 Apr 2019 16:20:31 -0700 Subject: [PATCH 2/2] Remove previous fix proposal proved to be wrong --- src/core/ext/transport/cronet/transport/cronet_transport.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.cc b/src/core/ext/transport/cronet/transport/cronet_transport.cc index 7ae178612c5..76a32dc4049 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.cc +++ b/src/core/ext/transport/cronet/transport/cronet_transport.cc @@ -335,9 +335,6 @@ static void add_to_storage(struct stream_obj* s, /* add new op at the beginning of the linked list. The memory is freed in remove_from_storage */ op_and_state* new_op = grpc_core::New(s, *op); - // Pontential fix to crash on GPR_ASSERT(!curr->done) - // TODO (mxyan): check if this is indeed necessary. - new_op->done = false; gpr_mu_lock(&s->mu); new_op->next = storage->head; storage->head = new_op;