Merge pull request #17883 from AspirinSJL/fix_hcc_segv

Unref watcher after releasing lock

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<ConnectedSubchannelStateWatcher*>(arg);
     Subchannel* c = self->subchannel_;
-    MutexLock lock(&c->mu_);
+    {
-    if (self->health_state_ == GRPC_CHANNEL_SHUTDOWN) {
+      MutexLock lock(&c->mu_);
-      self->Unref();
+      if (self->health_state_ != GRPC_CHANNEL_SHUTDOWN) {
-      return;
+        if (self->last_connectivity_state_ == GRPC_CHANNEL_READY) {
-    }
+          grpc_connectivity_state_set(&c->state_and_health_tracker_,
-    if (self->last_connectivity_state_ == GRPC_CHANNEL_READY) {
+                                      self->health_state_,
-      grpc_connectivity_state_set(&c->state_and_health_tracker_,
+                                      GRPC_ERROR_REF(error), "health_changed");
-                                  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_,
+    // Don't unref until we've released the lock, because this might
-                                                     &self->on_health_changed_);
+    // cause the subchannel (which contains the lock) to be destroyed.
+    if (self != nullptr) self->Unref();
   }
 
   Subchannel* subchannel_;