From a4f8534a98748a0d29a8cfd93edcf8af2481f106 Mon Sep 17 00:00:00 2001 From: Juanli Shen Date: Thu, 31 Jan 2019 10:45:02 -0800 Subject: [PATCH] Unref watcher after releasing lock --- .../ext/filters/client_channel/subchannel.cc | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc index 1d188a655f8..4276df3067f 100644 --- a/src/core/ext/filters/client_channel/subchannel.cc +++ b/src/core/ext/filters/client_channel/subchannel.cc @@ -405,18 +405,22 @@ class Subchannel::ConnectedSubchannelStateWatcher static void OnHealthChanged(void* arg, grpc_error* error) { auto* self = static_cast(arg); Subchannel* c = self->subchannel_; - MutexLock lock(&c->mu_); - if (self->health_state_ == GRPC_CHANNEL_SHUTDOWN) { - self->Unref(); - return; - } - if (self->last_connectivity_state_ == GRPC_CHANNEL_READY) { - grpc_connectivity_state_set(&c->state_and_health_tracker_, - self->health_state_, GRPC_ERROR_REF(error), - "health_changed"); + { + MutexLock lock(&c->mu_); + if (self->health_state_ != GRPC_CHANNEL_SHUTDOWN) { + if (self->last_connectivity_state_ == GRPC_CHANNEL_READY) { + grpc_connectivity_state_set(&c->state_and_health_tracker_, + self->health_state_, + GRPC_ERROR_REF(error), "health_changed"); + } + self->health_check_client_->NotifyOnHealthChange( + &self->health_state_, &self->on_health_changed_); + self = nullptr; // So we don't unref below. + } } - self->health_check_client_->NotifyOnHealthChange(&self->health_state_, - &self->on_health_changed_); + // Don't unref until we've released the lock, because this might + // cause the subchannel (which contains the lock) to be destroyed. + if (self != nullptr) self->Unref(); } Subchannel* subchannel_;