From e3f1f3c8568314bd777f78883fa56520e0b7aee2 Mon Sep 17 00:00:00 2001
From: ncteisen <ncteisen@gmail.com>
Date: Thu, 17 Jan 2019 09:39:23 -0800
Subject: [PATCH] Atomically store uuid of lb channel

---
 .../client_channel/lb_policy/grpclb/grpclb.cc | 25 ++++++++-----------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc
index 78de8b35659..6d46baa08fe 100644
--- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc
+++ b/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");
@@ -1210,12 +1206,8 @@ void GrpcLb::FillChildRefsForChannelz(
   if (rr_policy_ != nullptr) {
     rr_policy_->FillChildRefsForChannelz(child_subchannels, child_channels);
   }
-  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 (lb_channel_uuid_ != 0) {
+    child_channels->push_back(lb_channel_uuid_);
   }
 }
 
@@ -1275,12 +1267,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