Merge pull request #17738 from ncteisen/channelz-lock

Remove Unneeded Lock

src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc

@@ -296,9 +296,8 @@ class GrpcLb : public LoadBalancingPolicy {
 
   // The channel for communicating with the LB server.
   grpc_channel* lb_channel_ = nullptr;
-  // Mutex to protect the channel to the LB server. This is used when
-  // processing a channelz request.
-  gpr_mu lb_channel_mu_;
+  // Uuid of the lb channel. Used for channelz.
+  gpr_atm lb_channel_uuid_ = 0;
   grpc_connectivity_state lb_channel_connectivity_;
   grpc_closure lb_channel_on_connectivity_changed_;
   // Are we already watching the LB channel's connectivity?
@@ -986,7 +985,6 @@ GrpcLb::GrpcLb(const LoadBalancingPolicy::Args& args)
               .set_max_backoff(GRPC_GRPCLB_RECONNECT_MAX_BACKOFF_SECONDS *
                                1000)) {
   // Initialization.
-  gpr_mu_init(&lb_channel_mu_);
   GRPC_CLOSURE_INIT(&lb_channel_on_connectivity_changed_,
                     &GrpcLb::OnBalancerChannelConnectivityChangedLocked, this,
                     grpc_combiner_scheduler(args.combiner));
@@ -1023,7 +1021,6 @@ GrpcLb::GrpcLb(const LoadBalancingPolicy::Args& args)
 
 GrpcLb::~GrpcLb() {
   GPR_ASSERT(pending_picks_ == nullptr);
-  gpr_mu_destroy(&lb_channel_mu_);
   gpr_free((void*)server_name_);
   grpc_channel_args_destroy(args_);
   grpc_connectivity_state_destroy(&state_tracker_);
@@ -1049,10 +1046,9 @@ void GrpcLb::ShutdownLocked() {
   // OnBalancerChannelConnectivityChangedLocked(), and we need to be
   // alive when that callback is invoked.
   if (lb_channel_ != nullptr) {
-    gpr_mu_lock(&lb_channel_mu_);
     grpc_channel_destroy(lb_channel_);
     lb_channel_ = nullptr;
-    gpr_mu_unlock(&lb_channel_mu_);
+    gpr_atm_no_barrier_store(&lb_channel_uuid_, 0);
   }
   grpc_connectivity_state_set(&state_tracker_, GRPC_CHANNEL_SHUTDOWN,
                               GRPC_ERROR_REF(error), "grpclb_shutdown");
@@ -1207,14 +1203,12 @@ void GrpcLb::FillChildRefsForChannelz(
     channelz::ChildRefsList* child_subchannels,
     channelz::ChildRefsList* child_channels) {
   // delegate to the RoundRobin to fill the children subchannels.
-  rr_policy_->FillChildRefsForChannelz(child_subchannels, child_channels);
-  MutexLock lock(&lb_channel_mu_);
-  if (lb_channel_ != nullptr) {
-    grpc_core::channelz::ChannelNode* channel_node =
-        grpc_channel_get_channelz_node(lb_channel_);
-    if (channel_node != nullptr) {
-      child_channels->push_back(channel_node->uuid());
-    }
+  if (rr_policy_ != nullptr) {
+    rr_policy_->FillChildRefsForChannelz(child_subchannels, child_channels);
+  }
+  gpr_atm uuid = gpr_atm_no_barrier_load(&lb_channel_uuid_);
+  if (uuid != 0) {
+    child_channels->push_back(uuid);
   }
 }
 
@@ -1274,12 +1268,15 @@ void GrpcLb::ProcessChannelArgsLocked(const grpc_channel_args& args) {
   if (lb_channel_ == nullptr) {
     char* uri_str;
     gpr_asprintf(&uri_str, "fake:///%s", server_name_);
-    gpr_mu_lock(&lb_channel_mu_);
     lb_channel_ = grpc_client_channel_factory_create_channel(
         client_channel_factory(), uri_str,
         GRPC_CLIENT_CHANNEL_TYPE_LOAD_BALANCING, lb_channel_args);
-    gpr_mu_unlock(&lb_channel_mu_);
     GPR_ASSERT(lb_channel_ != nullptr);
+    grpc_core::channelz::ChannelNode* channel_node =
+        grpc_channel_get_channelz_node(lb_channel_);
+    if (channel_node != nullptr) {
+      gpr_atm_no_barrier_store(&lb_channel_uuid_, channel_node->uuid());
+    }
     gpr_free(uri_str);
   }
   // Propagate updates to the LB channel (pick_first) through the fake