From 5b9471da071e350d916762fb18e6641d37a0d8ae Mon Sep 17 00:00:00 2001
From: Yash Tibrewal <yashkt@google.com>
Date: Mon, 15 Mar 2021 15:09:08 -0700
Subject: [PATCH] Follow-up from #24965 (#25683)

* Follow-up from #24965

* Avoid deadlock on XdsClient destruction
---
 src/core/ext/xds/xds_client.cc                | 16 ++++++++++------
 src/core/ext/xds/xds_client.h                 |  6 +++++-
 src/core/ext/xds/xds_server_config_fetcher.cc |  4 ++--
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/src/core/ext/xds/xds_client.cc b/src/core/ext/xds/xds_client.cc
index 52b0f22250f..1d27c344446 100644
--- a/src/core/ext/xds/xds_client.cc
+++ b/src/core/ext/xds/xds_client.cc
@@ -2213,13 +2213,17 @@ void XdsClientGlobalShutdown() {
 }
 
 RefCountedPtr<XdsClient> XdsClient::GetOrCreate(grpc_error** error) {
-  MutexLock lock(g_mu);
-  if (g_xds_client != nullptr) {
-    auto xds_client = g_xds_client->RefIfNonZero();
-    if (xds_client != nullptr) return xds_client;
+  RefCountedPtr<XdsClient> xds_client;
+  {
+    MutexLock lock(g_mu);
+    if (g_xds_client != nullptr) {
+      auto xds_client = g_xds_client->RefIfNonZero();
+      if (xds_client != nullptr) return xds_client;
+    }
+    xds_client = MakeRefCounted<XdsClient>(error);
+    if (*error != GRPC_ERROR_NONE) return nullptr;
+    g_xds_client = xds_client.get();
   }
-  auto xds_client = MakeRefCounted<XdsClient>(error);
-  g_xds_client = xds_client.get();
   return xds_client;
 }
 
diff --git a/src/core/ext/xds/xds_client.h b/src/core/ext/xds/xds_client.h
index 08b86c57766..059e49cab3a 100644
--- a/src/core/ext/xds/xds_client.h
+++ b/src/core/ext/xds/xds_client.h
@@ -88,7 +88,11 @@ class XdsClient : public DualRefCounted<XdsClient> {
   explicit XdsClient(grpc_error** error);
   ~XdsClient() override;
 
-  const XdsBootstrap* bootstrap() const { return bootstrap_.get(); }
+  const XdsBootstrap& bootstrap() const {
+    // bootstrap_ is guaranteed to be non-null since XdsClient::GetOrCreate()
+    // would return a null object if bootstrap_ was null.
+    return *bootstrap_;
+  }
 
   CertificateProviderStore& certificate_provider_store() {
     return *certificate_provider_store_;
diff --git a/src/core/ext/xds/xds_server_config_fetcher.cc b/src/core/ext/xds/xds_server_config_fetcher.cc
index b1f4c73ca2b..7e6d34ee82b 100644
--- a/src/core/ext/xds/xds_server_config_fetcher.cc
+++ b/src/core/ext/xds/xds_server_config_fetcher.cc
@@ -51,7 +51,7 @@ class XdsServerConfigFetcher : public grpc_server_config_fetcher {
         listening_address);
     auto* listener_watcher_ptr = listener_watcher.get();
     listening_address = absl::StrReplaceAll(
-        xds_client_->bootstrap()->server_listener_resource_name_template(),
+        xds_client_->bootstrap().server_listener_resource_name_template(),
         {{"%s", listening_address}});
     xds_client_->WatchListenerData(listening_address,
                                    std::move(listener_watcher));
@@ -328,7 +328,7 @@ grpc_server_config_fetcher* grpc_server_config_fetcher_xds_create(
     return nullptr;
   }
   if (xds_client->bootstrap()
-          ->server_listener_resource_name_template()
+          .server_listener_resource_name_template()
           .empty()) {
     gpr_log(GPR_ERROR,
             "server_listener_resource_name_template not provided in bootstrap "