From d426d9dfdbd1712b1b241a17452ed15441b502a1 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 9 Mar 2016 09:30:18 -0800 Subject: [PATCH] Fix memory leak if call is already cancelled Obvious in hindsight, if the cas failed, we still created the subchannel call object and then left it dangling. Fixes a leak observed on master in the past 48 hours. --- src/core/channel/subchannel_call_holder.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/core/channel/subchannel_call_holder.c b/src/core/channel/subchannel_call_holder.c index 8f46885a043..9c087dc2a1d 100644 --- a/src/core/channel/subchannel_call_holder.c +++ b/src/core/channel/subchannel_call_holder.c @@ -174,17 +174,15 @@ static void subchannel_ready(grpc_exec_ctx *exec_ctx, void *arg, bool success) { holder->creation_phase = GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING; if (holder->connected_subchannel == NULL) { fail_locked(exec_ctx, holder); + } else if (1 == gpr_atm_acq_load(&holder->subchannel_call)) { + /* already cancelled before subchannel became ready */ + fail_locked(exec_ctx, holder); } else { - 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))) { - 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_atm_rel_store( + &holder->subchannel_call, + (gpr_atm)(uintptr_t)grpc_connected_subchannel_create_call( + exec_ctx, holder->connected_subchannel, holder->pollset)); + retry_waiting_locked(exec_ctx, holder); } gpr_mu_unlock(&holder->mu); GRPC_CALL_STACK_UNREF(exec_ctx, holder->owning_call, "pick_subchannel");