Potential fix for race condition exposed by Node

narzolliam · Mar 7, 2016 · f880e62 · f880e62
1 parent 6537ab9
commit f880e62
Showing 1 changed file with 9 additions and 6 deletions.
diff --git a/src/core/channel/subchannel_call_holder.c b/src/core/channel/subchannel_call_holder.c
@@ -168,21 +168,24 @@ void grpc_subchannel_call_holder_perform_op(grpc_exec_ctx *exec_ctx,
 
 static void subchannel_ready(grpc_exec_ctx *exec_ctx, void *arg, bool success) {
  grpc_subchannel_call_holder *holder = arg;
- grpc_subchannel_call *call;
  gpr_mu_lock(&holder->mu);
  GPR_ASSERT(holder->creation_phase ==
  GRPC_SUBCHANNEL_CALL_HOLDER_PICKING_SUBCHANNEL);
- call = GET_CALL(holder);
- GPR_ASSERT(call == NULL || call == CANCELLED_CALL);
  holder->creation_phase = GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING;
  if (holder->connected_subchannel == NULL) {
  fail_locked(exec_ctx, holder);
  } else {
- gpr_atm_rel_store(
+ if (!gpr_atm_rel_cas(
  &holder->subchannel_call,
+ 0,
  (gpr_atm)(uintptr_t)grpc_connected_subchannel_create_call(
- exec_ctx, holder->connected_subchannel, holder->pollset));
- retry_waiting_locked(exec_ctx, holder);
+ exec_ctx, holder->connected_subchannel, holder->pollset))) {
+ GPR_ASSERT(gpr_atm_acq_load(&holder->subchannel_call) == 1);
+ /* if this cas fails, the call was cancelled before the pick completed */
+ fail_locked(exec_ctx, holder);
+ } else {
+ retry_waiting_locked(exec_ctx, holder);
+ }
  }
  gpr_mu_unlock(&holder->mu);
  GRPC_CALL_STACK_UNREF(exec_ctx, holder->owning_call, "pick_subchannel");