From 0a56dcabb910b331cad0ab424c9c5ed8c550f39d Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Mon, 6 Apr 2020 21:14:21 -0700 Subject: [PATCH 1/2] Fix cronet transport crash --- .../cronet/transport/cronet_transport.cc | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.cc b/src/core/ext/transport/cronet/transport/cronet_transport.cc index acdc3fc1ff1..f2c4639b3d1 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.cc +++ b/src/core/ext/transport/cronet/transport/cronet_transport.cc @@ -1072,9 +1072,15 @@ static enum e_op_result execute_stream_op(struct op_and_state* oas) { op_can_be_run(stream_op, s, &oas->state, OP_SEND_MESSAGE)) { CRONET_LOG(GPR_DEBUG, "running: %p OP_SEND_MESSAGE", oas); stream_state->pending_send_message = false; - if (stream_state->state_callback_received[OP_FAILED]) { + if (stream_state->state_op_done[OP_CANCEL_ERROR]) { + result = NO_ACTION_POSSIBLE; + CRONET_LOG(GPR_DEBUG, "Stream is cancelled."); + } else if (stream_state->state_callback_received[OP_FAILED]) { + result = NO_ACTION_POSSIBLE; + CRONET_LOG(GPR_DEBUG, "Stream failed."); + } else if (stream_state->state_callback_received[OP_SUCCEEDED]) { result = NO_ACTION_POSSIBLE; - CRONET_LOG(GPR_DEBUG, "Stream is either cancelled or failed."); + CRONET_LOG(GPR_DEBUG, "Stream is already finished."); } else { grpc_slice_buffer write_slice_buffer; grpc_slice slice; @@ -1131,9 +1137,15 @@ static enum e_op_result execute_stream_op(struct op_and_state* oas) { op_can_be_run(stream_op, s, &oas->state, OP_SEND_TRAILING_METADATA)) { CRONET_LOG(GPR_DEBUG, "running: %p OP_SEND_TRAILING_METADATA", oas); - if (stream_state->state_callback_received[OP_FAILED]) { + if (stream_state->state_op_done[OP_CANCEL_ERROR]) { + result = NO_ACTION_POSSIBLE; + CRONET_LOG(GPR_DEBUG, "Stream is cancelled."); + } else if (stream_state->state_callback_received[OP_FAILED]) { + result = NO_ACTION_POSSIBLE; + CRONET_LOG(GPR_DEBUG, "Stream failed."); + } else if (stream_state->state_callback_received[OP_SUCCEEDED]) { result = NO_ACTION_POSSIBLE; - CRONET_LOG(GPR_DEBUG, "Stream is either cancelled or failed."); + CRONET_LOG(GPR_DEBUG, "Stream is already done."); } else { CRONET_LOG(GPR_DEBUG, "bidirectional_stream_write (%p, 0)", s->cbs); stream_state->state_callback_received[OP_SEND_MESSAGE] = false; From d94f41c40ed63360f37948e1509dfd127301c88f Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Mon, 6 Apr 2020 22:19:33 -0700 Subject: [PATCH 2/2] Fix clang error --- .../cronet/transport/cronet_transport.cc | 24 +++++++------------ 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.cc b/src/core/ext/transport/cronet/transport/cronet_transport.cc index f2c4639b3d1..b3a0c401dbc 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.cc +++ b/src/core/ext/transport/cronet/transport/cronet_transport.cc @@ -1072,15 +1072,11 @@ static enum e_op_result execute_stream_op(struct op_and_state* oas) { op_can_be_run(stream_op, s, &oas->state, OP_SEND_MESSAGE)) { CRONET_LOG(GPR_DEBUG, "running: %p OP_SEND_MESSAGE", oas); stream_state->pending_send_message = false; - if (stream_state->state_op_done[OP_CANCEL_ERROR]) { - result = NO_ACTION_POSSIBLE; - CRONET_LOG(GPR_DEBUG, "Stream is cancelled."); - } else if (stream_state->state_callback_received[OP_FAILED]) { - result = NO_ACTION_POSSIBLE; - CRONET_LOG(GPR_DEBUG, "Stream failed."); - } else if (stream_state->state_callback_received[OP_SUCCEEDED]) { + if (stream_state->state_op_done[OP_CANCEL_ERROR] || + stream_state->state_callback_received[OP_FAILED] || + stream_state->state_callback_received[OP_SUCCEEDED]) { result = NO_ACTION_POSSIBLE; - CRONET_LOG(GPR_DEBUG, "Stream is already finished."); + CRONET_LOG(GPR_DEBUG, "Stream is either cancelled, failed or finished"); } else { grpc_slice_buffer write_slice_buffer; grpc_slice slice; @@ -1137,15 +1133,11 @@ static enum e_op_result execute_stream_op(struct op_and_state* oas) { op_can_be_run(stream_op, s, &oas->state, OP_SEND_TRAILING_METADATA)) { CRONET_LOG(GPR_DEBUG, "running: %p OP_SEND_TRAILING_METADATA", oas); - if (stream_state->state_op_done[OP_CANCEL_ERROR]) { - result = NO_ACTION_POSSIBLE; - CRONET_LOG(GPR_DEBUG, "Stream is cancelled."); - } else if (stream_state->state_callback_received[OP_FAILED]) { - result = NO_ACTION_POSSIBLE; - CRONET_LOG(GPR_DEBUG, "Stream failed."); - } else if (stream_state->state_callback_received[OP_SUCCEEDED]) { + if (stream_state->state_op_done[OP_CANCEL_ERROR] || + stream_state->state_callback_received[OP_FAILED] || + stream_state->state_callback_received[OP_SUCCEEDED]) { result = NO_ACTION_POSSIBLE; - CRONET_LOG(GPR_DEBUG, "Stream is already done."); + CRONET_LOG(GPR_DEBUG, "Stream is either cancelled, failed or finished"); } else { CRONET_LOG(GPR_DEBUG, "bidirectional_stream_write (%p, 0)", s->cbs); stream_state->state_callback_received[OP_SEND_MESSAGE] = false;