XdsClient: refcount tracing improvements (#30277)

* XdsClient: improve ref-count trace logging

* better message for GetOrCreate()

* add more tracing

* Automated change: Fix sanity tests

Co-authored-by: markdroth <markdroth@users.noreply.github.com>

BUILD

@@ -3166,6 +3166,7 @@ grpc_cc_library(
     deps = [
         "avl",
         "channel_stack_type",
+        "debug_location",
         "dual_ref_counted",
         "gpr_base",
         "grpc_codegen",
@@ -4027,6 +4028,7 @@ grpc_cc_library(
         "channel_args_preconditioning",
         "channel_fwd",
         "config",
+        "debug_location",
         "exec_ctx",
         "gpr_base",
         "grpc_base",

src/core/ext/filters/client_channel/lb_policy/xds/cds.cc

@@ -714,7 +714,8 @@ class CdsLbFactory : public LoadBalancingPolicyFactory {
  public:
   OrphanablePtr<LoadBalancingPolicy> CreateLoadBalancingPolicy(
       LoadBalancingPolicy::Args args) const override {
-    auto xds_client = args.args.GetObjectRef<GrpcXdsClient>();
+    auto xds_client =
+        args.args.GetObjectRef<GrpcXdsClient>(DEBUG_LOCATION, "CdsLb");
     if (xds_client == nullptr) {
       gpr_log(GPR_ERROR,
               "XdsClient not present in channel args -- cannot instantiate "

src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_impl.cc

@@ -466,7 +466,7 @@ void XdsClusterImplLb::ShutdownLocked() {
   // the child.
   picker_.reset();
   drop_stats_.reset();
-  xds_client_.reset();
+  xds_client_.reset(DEBUG_LOCATION, "XdsClusterImpl");
 }
 
 void XdsClusterImplLb::ExitIdleLocked() {
@@ -691,7 +691,8 @@ class XdsClusterImplLbFactory : public LoadBalancingPolicyFactory {
  public:
   OrphanablePtr<LoadBalancingPolicy> CreateLoadBalancingPolicy(
       LoadBalancingPolicy::Args args) const override {
-    auto xds_client = args.args.GetObjectRef<GrpcXdsClient>();
+    auto xds_client = args.args.GetObjectRef<GrpcXdsClient>(DEBUG_LOCATION,
+                                                            "XdsClusterImplLb");
     if (xds_client == nullptr) {
       gpr_log(GPR_ERROR,
               "XdsClient not present in channel args -- cannot instantiate "

src/core/ext/filters/client_channel/lb_policy/xds/xds_cluster_resolver.cc

@@ -1059,7 +1059,8 @@ class XdsClusterResolverLbFactory : public LoadBalancingPolicyFactory {
  public:
   OrphanablePtr<LoadBalancingPolicy> CreateLoadBalancingPolicy(
       LoadBalancingPolicy::Args args) const override {
-    auto xds_client = args.args.GetObjectRef<GrpcXdsClient>();
+    auto xds_client = args.args.GetObjectRef<GrpcXdsClient>(
+        DEBUG_LOCATION, "XdsClusterResolverLbFactory");
     if (xds_client == nullptr) {
       gpr_log(GPR_ERROR,
               "XdsClient not present in channel args -- cannot instantiate "
@@ -1281,6 +1282,10 @@ class XdsClusterResolverLbFactory : public LoadBalancingPolicyFactory {
                              &grpc_lb_xds_cluster_resolver_trace),
           xds_client_(std::move(xds_client)) {}
 
+    ~XdsClusterResolverChildHandler() override {
+      xds_client_.reset(DEBUG_LOCATION, "XdsClusterResolverChildHandler");
+    }
+
     bool ConfigChangeRequiresNewPolicyInstance(
         LoadBalancingPolicy::Config* old_config,
         LoadBalancingPolicy::Config* new_config) const override {
@@ -1296,7 +1301,9 @@ class XdsClusterResolverLbFactory : public LoadBalancingPolicyFactory {
 
     OrphanablePtr<LoadBalancingPolicy> CreateLoadBalancingPolicy(
         const char* /*name*/, LoadBalancingPolicy::Args args) const override {
-      return MakeOrphanable<XdsClusterResolverLb>(xds_client_, std::move(args));
+      return MakeOrphanable<XdsClusterResolverLb>(
+          xds_client_->Ref(DEBUG_LOCATION, "XdsClusterResolverLb"),
+          std::move(args));
     }
 
    private:

src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc

@@ -787,24 +787,22 @@ ConfigSelector::CallConfig XdsResolver::XdsConfigSelector::GetCallConfig(
 
 void XdsResolver::StartLocked() {
   grpc_error_handle error = GRPC_ERROR_NONE;
-  xds_client_ = GrpcXdsClient::GetOrCreate(args_, &error);
-  if (!GRPC_ERROR_IS_NONE(error)) {
+  auto xds_client = GrpcXdsClient::GetOrCreate(args_, "xds resolver");
+  if (!xds_client.ok()) {
     gpr_log(GPR_ERROR,
             "Failed to create xds client -- channel will remain in "
             "TRANSIENT_FAILURE: %s",
-            grpc_error_std_string(error).c_str());
-    std::string error_message;
-    grpc_error_get_str(error, GRPC_ERROR_STR_DESCRIPTION, &error_message);
-    absl::Status status = absl::UnavailableError(
-        absl::StrCat("Failed to create XdsClient: ", error_message));
+            xds_client.status().ToString().c_str());
+    absl::Status status = absl::UnavailableError(absl::StrCat(
+        "Failed to create XdsClient: ", xds_client.status().message()));
     Result result;
     result.addresses = status;
     result.service_config = std::move(status);
     result.args = args_;
     result_handler_->ReportResult(std::move(result));
-    GRPC_ERROR_UNREF(error);
     return;
   }
+  xds_client_ = std::move(*xds_client);
   std::string resource_name_fragment(absl::StripPrefix(uri_.path(), "/"));
   if (!uri_.authority().empty()) {
     // target_uri.authority is set case
@@ -876,7 +874,7 @@ void XdsResolver::ShutdownLocked() {
     grpc_pollset_set_del_pollset_set(
         static_cast<GrpcXdsClient*>(xds_client_.get())->interested_parties(),
         interested_parties_);
-    xds_client_.reset();
+    xds_client_.reset(DEBUG_LOCATION, "xds resolver");
   }
 }
 
@@ -974,7 +972,11 @@ void XdsResolver::OnError(absl::string_view context, absl::Status status) {
   Result result;
   result.addresses = status;
   result.service_config = std::move(status);
-  result.args = args_.SetObject(xds_client_);
+  // Need to explicitly convert to the right RefCountedPtr<> type for
+  // use with ChannelArgs::SetObject().
+  RefCountedPtr<GrpcXdsClient> xds_client =
+      xds_client_->Ref(DEBUG_LOCATION, "xds resolver result");
+  result.args = args_.SetObject(std::move(xds_client));
   result_handler_->ReportResult(std::move(result));
 }
 
@@ -1066,7 +1068,12 @@ void XdsResolver::GenerateResult() {
                 ? std::string((*result.service_config)->json_string()).c_str()
                 : result.service_config.status().ToString().c_str());
   }
-  result.args = args_.SetObject(xds_client_).SetObject(config_selector);
+  // Need to explicitly convert to the right RefCountedPtr<> type for
+  // use with ChannelArgs::SetObject().
+  RefCountedPtr<GrpcXdsClient> xds_client =
+      xds_client_->Ref(DEBUG_LOCATION, "xds resolver result");
+  result.args =
+      args_.SetObject(std::move(xds_client)).SetObject(config_selector);
   result_handler_->ReportResult(std::move(result));
 }
 

src/core/ext/xds/xds_client.cc

@@ -1426,10 +1426,10 @@ void XdsClient::Orphan() {
 }
 
 RefCountedPtr<XdsClient::ChannelState> XdsClient::GetOrCreateChannelStateLocked(
-    const XdsBootstrap::XdsServer& server) {
+    const XdsBootstrap::XdsServer& server, const char* reason) {
   auto it = xds_server_channel_map_.find(server);
   if (it != xds_server_channel_map_.end()) {
-    return it->second->Ref(DEBUG_LOCATION, "Authority");
+    return it->second->Ref(DEBUG_LOCATION, reason);
   }
   // Channel not found, so create a new one.
   auto channel_state = MakeRefCounted<ChannelState>(
@@ -1507,7 +1507,7 @@ void XdsClient::WatchResource(const XdsResourceType* type,
     // needed.
     if (authority_state.channel_state == nullptr) {
       authority_state.channel_state =
-          GetOrCreateChannelStateLocked(*xds_server);
+          GetOrCreateChannelStateLocked(*xds_server, "start watch");
     }
     authority_state.channel_state->SubscribeLocked(type, *resource_name);
   }
@@ -1641,8 +1641,8 @@ RefCountedPtr<XdsClusterDropStats> XdsClient::AddClusterDropStats(
         xds_load_report_server_map_.emplace(xds_server, LoadReportServer())
             .first;
     if (server_it->second.channel_state == nullptr) {
-      server_it->second.channel_state =
-          GetOrCreateChannelStateLocked(xds_server);
+      server_it->second.channel_state = GetOrCreateChannelStateLocked(
+          xds_server, "load report map (drop stats)");
     }
     auto load_report_it = server_it->second.load_report_map
                               .emplace(std::move(key), LoadReportState())
@@ -1707,8 +1707,8 @@ RefCountedPtr<XdsClusterLocalityStats> XdsClient::AddClusterLocalityStats(
         xds_load_report_server_map_.emplace(xds_server, LoadReportServer())
             .first;
     if (server_it->second.channel_state == nullptr) {
-      server_it->second.channel_state =
-          GetOrCreateChannelStateLocked(xds_server);
+      server_it->second.channel_state = GetOrCreateChannelStateLocked(
+          xds_server, "load report map (locality stats)");
     }
     auto load_report_it = server_it->second.load_report_map
                               .emplace(std::move(key), LoadReportState())

src/core/ext/xds/xds_client.h

@@ -297,7 +297,8 @@ class XdsClient : public DualRefCounted<XdsClient> {
       const std::set<std::string>& clusters) ABSL_EXCLUSIVE_LOCKS_REQUIRED(mu_);
 
   RefCountedPtr<ChannelState> GetOrCreateChannelStateLocked(
-      const XdsBootstrap::XdsServer& server) ABSL_EXCLUSIVE_LOCKS_REQUIRED(mu_);
+      const XdsBootstrap::XdsServer& server, const char* reason)
+      ABSL_EXCLUSIVE_LOCKS_REQUIRED(mu_);
 
   std::unique_ptr<XdsBootstrap> bootstrap_;
   OrphanablePtr<XdsTransportFactory> transport_factory_;

src/core/ext/xds/xds_client_grpc.cc

@@ -24,6 +24,7 @@
 #include <utility>
 
 #include "absl/base/thread_annotations.h"
+#include "absl/status/status.h"
 #include "absl/strings/string_view.h"
 #include "absl/types/optional.h"
 
@@ -42,15 +43,18 @@
 #include "src/core/lib/channel/channel_args.h"
 #include "src/core/lib/debug/trace.h"
 #include "src/core/lib/gpr/env.h"
+#include "src/core/lib/gprpp/debug_location.h"
 #include "src/core/lib/gprpp/memory.h"
 #include "src/core/lib/gprpp/orphanable.h"
 #include "src/core/lib/gprpp/ref_counted_ptr.h"
 #include "src/core/lib/gprpp/sync.h"
 #include "src/core/lib/gprpp/time.h"
+#include "src/core/lib/iomgr/error.h"
 #include "src/core/lib/iomgr/exec_ctx.h"
 #include "src/core/lib/iomgr/load_file.h"
 #include "src/core/lib/slice/slice_internal.h"
 #include "src/core/lib/slice/slice_refcount.h"
+#include "src/core/lib/transport/error_utils.h"
 
 namespace grpc_core {
 
@@ -58,7 +62,7 @@ namespace {
 
 Mutex* g_mu = nullptr;
 const grpc_channel_args* g_channel_args ABSL_GUARDED_BY(*g_mu) = nullptr;
-XdsClient* g_xds_client ABSL_GUARDED_BY(*g_mu) = nullptr;
+GrpcXdsClient* g_xds_client ABSL_GUARDED_BY(*g_mu) = nullptr;
 char* g_fallback_bootstrap_config ABSL_GUARDED_BY(*g_mu) = nullptr;
 
 }  // namespace
@@ -82,8 +86,7 @@ void XdsClientGlobalShutdown() ABSL_NO_THREAD_SAFETY_ANALYSIS {
 
 namespace {
 
-std::string GetBootstrapContents(const char* fallback_config,
-                                 grpc_error_handle* error) {
+absl::StatusOr<std::string> GetBootstrapContents(const char* fallback_config) {
   // First, try GRPC_XDS_BOOTSTRAP env var.
   UniquePtr<char> path(gpr_getenv("GRPC_XDS_BOOTSTRAP"));
   if (path != nullptr) {
@@ -94,9 +97,9 @@ std::string GetBootstrapContents(const char* fallback_config,
               path.get());
     }
     grpc_slice contents;
-    *error =
+    grpc_error_handle error =
         grpc_load_file(path.get(), /*add_null_terminator=*/true, &contents);
-    if (!GRPC_ERROR_IS_NONE(*error)) return "";
+    if (!GRPC_ERROR_IS_NONE(error)) return grpc_error_to_absl_status(error);
     std::string contents_str(StringViewFromSlice(contents));
     grpc_slice_unref_internal(contents);
     return contents_str;
@@ -119,56 +122,51 @@ std::string GetBootstrapContents(const char* fallback_config,
     return fallback_config;
   }
   // No bootstrap config found.
-  *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING(
+  return absl::FailedPreconditionError(
       "Environment variables GRPC_XDS_BOOTSTRAP or GRPC_XDS_BOOTSTRAP_CONFIG "
       "not defined");
-  return "";
 }
 
 }  // namespace
 
-RefCountedPtr<XdsClient> GrpcXdsClient::GetOrCreate(const ChannelArgs& args,
-                                                    grpc_error_handle* error) {
-  RefCountedPtr<XdsClient> xds_client;
+absl::StatusOr<RefCountedPtr<GrpcXdsClient>> GrpcXdsClient::GetOrCreate(
+    const ChannelArgs& args, const char* reason) {
   // If getting bootstrap from channel args, create a local XdsClient
   // instance for the channel or server instead of using the global instance.
   absl::optional<absl::string_view> bootstrap_config = args.GetString(
       GRPC_ARG_TEST_ONLY_DO_NOT_USE_IN_PROD_XDS_BOOTSTRAP_CONFIG);
   if (bootstrap_config.has_value()) {
+    grpc_error_handle error = GRPC_ERROR_NONE;
     std::unique_ptr<XdsBootstrap> bootstrap =
-        XdsBootstrap::Create(*bootstrap_config, error);
-    if (GRPC_ERROR_IS_NONE(*error)) {
-      grpc_channel_args* xds_channel_args = args.GetPointer<grpc_channel_args>(
-          GRPC_ARG_TEST_ONLY_DO_NOT_USE_IN_PROD_XDS_CLIENT_CHANNEL_ARGS);
-      return MakeRefCounted<GrpcXdsClient>(
-          std::move(bootstrap), ChannelArgs::FromC(xds_channel_args));
-    }
-    return nullptr;
+        XdsBootstrap::Create(*bootstrap_config, &error);
+    if (!GRPC_ERROR_IS_NONE(error)) return grpc_error_to_absl_status(error);
+    grpc_channel_args* xds_channel_args = args.GetPointer<grpc_channel_args>(
+        GRPC_ARG_TEST_ONLY_DO_NOT_USE_IN_PROD_XDS_CLIENT_CHANNEL_ARGS);
+    return MakeRefCounted<GrpcXdsClient>(std::move(bootstrap),
+                                         ChannelArgs::FromC(xds_channel_args));
   }
   // Otherwise, use the global instance.
-  {
-    MutexLock lock(g_mu);
-    if (g_xds_client != nullptr) {
-      auto xds_client = g_xds_client->RefIfNonZero();
-      if (xds_client != nullptr) return xds_client;
-    }
-    // Find bootstrap contents.
-    std::string bootstrap_contents =
-        GetBootstrapContents(g_fallback_bootstrap_config, error);
-    if (!GRPC_ERROR_IS_NONE(*error)) return nullptr;
-    if (GRPC_TRACE_FLAG_ENABLED(grpc_xds_client_trace)) {
-      gpr_log(GPR_INFO, "xDS bootstrap contents: %s",
-              bootstrap_contents.c_str());
-    }
-    // Parse bootstrap.
-    std::unique_ptr<XdsBootstrap> bootstrap =
-        XdsBootstrap::Create(bootstrap_contents, error);
-    if (!GRPC_ERROR_IS_NONE(*error)) return nullptr;
-    // Instantiate XdsClient.
-    xds_client = MakeRefCounted<GrpcXdsClient>(
-        std::move(bootstrap), ChannelArgs::FromC(g_channel_args));
-    g_xds_client = xds_client.get();
+  MutexLock lock(g_mu);
+  if (g_xds_client != nullptr) {
+    auto xds_client = g_xds_client->RefIfNonZero(DEBUG_LOCATION, reason);
+    if (xds_client != nullptr) return xds_client;
   }
+  // Find bootstrap contents.
+  auto bootstrap_contents = GetBootstrapContents(g_fallback_bootstrap_config);
+  if (!bootstrap_contents.ok()) return bootstrap_contents.status();
+  if (GRPC_TRACE_FLAG_ENABLED(grpc_xds_client_trace)) {
+    gpr_log(GPR_INFO, "xDS bootstrap contents: %s",
+            bootstrap_contents->c_str());
+  }
+  // Parse bootstrap.
+  grpc_error_handle error = GRPC_ERROR_NONE;
+  std::unique_ptr<XdsBootstrap> bootstrap =
+      XdsBootstrap::Create(*bootstrap_contents, &error);
+  if (!GRPC_ERROR_IS_NONE(error)) return grpc_error_to_absl_status(error);
+  // Instantiate XdsClient.
+  auto xds_client = MakeRefCounted<GrpcXdsClient>(
+      std::move(bootstrap), ChannelArgs::FromC(g_channel_args));
+  g_xds_client = xds_client.get();
   return xds_client;
 }
 
@@ -218,12 +216,11 @@ grpc_slice grpc_dump_xds_configs(void) {
   grpc_core::ApplicationCallbackExecCtx callback_exec_ctx;
   grpc_core::ExecCtx exec_ctx;
   grpc_error_handle error = GRPC_ERROR_NONE;
-  auto xds_client =
-      grpc_core::GrpcXdsClient::GetOrCreate(grpc_core::ChannelArgs(), &error);
-  if (!GRPC_ERROR_IS_NONE(error)) {
+  auto xds_client = grpc_core::GrpcXdsClient::GetOrCreate(
+      grpc_core::ChannelArgs(), "grpc_dump_xds_configs()");
+  if (!xds_client.ok()) {
     // If we aren't using xDS, just return an empty string.
-    GRPC_ERROR_UNREF(error);
     return grpc_empty_slice();
   }
-  return grpc_slice_from_cpp_string(xds_client->DumpClientConfigBinary());
+  return grpc_slice_from_cpp_string((*xds_client)->DumpClientConfigBinary());
 }

src/core/ext/xds/xds_client_grpc.h

@@ -21,6 +21,7 @@
 
 #include <memory>
 
+#include "absl/status/statusor.h"
 #include "absl/strings/string_view.h"
 
 #include <grpc/impl/codegen/grpc_types.h>
@@ -30,7 +31,6 @@
 #include "src/core/lib/channel/channel_args.h"
 #include "src/core/lib/gpr/useful.h"
 #include "src/core/lib/gprpp/ref_counted_ptr.h"
-#include "src/core/lib/iomgr/error.h"
 #include "src/core/lib/iomgr/iomgr_fwd.h"
 
 namespace grpc_core {
@@ -38,10 +38,8 @@ namespace grpc_core {
 class GrpcXdsClient : public XdsClient {
  public:
   // Factory function to get or create the global XdsClient instance.
-  // If *error is not GRPC_ERROR_NONE upon return, then there was
-  // an error initializing the client.
-  static RefCountedPtr<XdsClient> GetOrCreate(const ChannelArgs& args,
-                                              grpc_error_handle* error);
+  static absl::StatusOr<RefCountedPtr<GrpcXdsClient>> GetOrCreate(
+      const ChannelArgs& args, const char* reason);
 
   // Do not instantiate directly -- use GetOrCreate() instead.
   GrpcXdsClient(std::unique_ptr<XdsBootstrap> bootstrap,

src/core/ext/xds/xds_server_config_fetcher.cc

@@ -54,7 +54,6 @@
 #include "src/core/ext/xds/xds_bootstrap.h"
 #include "src/core/ext/xds/xds_certificate_provider.h"
 #include "src/core/ext/xds/xds_channel_stack_modifier.h"
-#include "src/core/ext/xds/xds_client.h"
 #include "src/core/ext/xds/xds_client_grpc.h"
 #include "src/core/ext/xds/xds_common_types.h"
 #include "src/core/ext/xds/xds_http_filters.h"
@@ -69,6 +68,7 @@
 #include "src/core/lib/channel/channel_fwd.h"
 #include "src/core/lib/config/core_configuration.h"
 #include "src/core/lib/debug/trace.h"
+#include "src/core/lib/gprpp/debug_location.h"
 #include "src/core/lib/gprpp/host_port.h"
 #include "src/core/lib/gprpp/ref_counted_ptr.h"
 #include "src/core/lib/gprpp/sync.h"
@@ -102,9 +102,13 @@ TraceFlag grpc_xds_server_config_fetcher_trace(false,
 // listeners from the xDS control plane.
 class XdsServerConfigFetcher : public grpc_server_config_fetcher {
  public:
-  XdsServerConfigFetcher(RefCountedPtr<XdsClient> xds_client,
+  XdsServerConfigFetcher(RefCountedPtr<GrpcXdsClient> xds_client,
                          grpc_server_xds_status_notifier notifier);
 
+  ~XdsServerConfigFetcher() override {
+    xds_client_.reset(DEBUG_LOCATION, "XdsServerConfigFetcher");
+  }
+
   void StartWatch(std::string listening_address,
                   std::unique_ptr<grpc_server_config_fetcher::WatcherInterface>
                       watcher) override;
@@ -114,13 +118,13 @@ class XdsServerConfigFetcher : public grpc_server_config_fetcher {
 
   // Return the interested parties from the xds client so that it can be polled.
   grpc_pollset_set* interested_parties() override {
-    return static_cast<GrpcXdsClient*>(xds_client_.get())->interested_parties();
+    return xds_client_->interested_parties();
   }
 
  private:
   class ListenerWatcher;
 
-  const RefCountedPtr<XdsClient> xds_client_;
+  RefCountedPtr<GrpcXdsClient> xds_client_;
   const grpc_server_xds_status_notifier serving_status_notifier_;
   Mutex mu_;
   std::map<grpc_server_config_fetcher::WatcherInterface*, ListenerWatcher*>
@@ -140,12 +144,16 @@ class XdsServerConfigFetcher : public grpc_server_config_fetcher {
 class XdsServerConfigFetcher::ListenerWatcher
     : public XdsListenerResourceType::WatcherInterface {
  public:
-  ListenerWatcher(RefCountedPtr<XdsClient> xds_client,
+  ListenerWatcher(RefCountedPtr<GrpcXdsClient> xds_client,
                   std::unique_ptr<grpc_server_config_fetcher::WatcherInterface>
                       server_config_watcher,
                   grpc_server_xds_status_notifier serving_status_notifier,
                   std::string listening_address);
 
+  ~ListenerWatcher() override {
+    xds_client_.reset(DEBUG_LOCATION, "ListenerWatcher");
+  }
+
   void OnResourceChanged(XdsListenerResource listener) override;
 
   void OnError(absl::Status status) override;
@@ -172,7 +180,7 @@ class XdsServerConfigFetcher::ListenerWatcher
       FilterChainMatchManager* filter_chain_match_manager)
       ABSL_EXCLUSIVE_LOCKS_REQUIRED(&mu_);
 
-  const RefCountedPtr<XdsClient> xds_client_;
+  RefCountedPtr<GrpcXdsClient> xds_client_;
   const std::unique_ptr<grpc_server_config_fetcher::WatcherInterface>
       server_config_watcher_;
   const grpc_server_xds_status_notifier serving_status_notifier_;
@@ -192,11 +200,15 @@ class XdsServerConfigFetcher::ListenerWatcher
 class XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager
     : public grpc_server_config_fetcher::ConnectionManager {
  public:
-  FilterChainMatchManager(RefCountedPtr<XdsClient> xds_client,
+  FilterChainMatchManager(RefCountedPtr<GrpcXdsClient> xds_client,
                           XdsListenerResource::FilterChainMap filter_chain_map,
                           absl::optional<XdsListenerResource::FilterChainData>
                               default_filter_chain);
 
+  ~FilterChainMatchManager() override {
+    xds_client_.reset(DEBUG_LOCATION, "FilterChainMatchManager");
+  }
+
   absl::StatusOr<ChannelArgs> UpdateChannelArgsForConnection(
       const ChannelArgs& args, grpc_endpoint* tcp) override;
 
@@ -246,7 +258,7 @@ class XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager
   void OnError(const std::string& resource_name, absl::Status status);
   void OnResourceDoesNotExist(const std::string& resource_name);
 
-  RefCountedPtr<XdsClient> xds_client_;
+  RefCountedPtr<GrpcXdsClient> xds_client_;
   // This ref is only kept around till the FilterChainMatchManager becomes
   // ready.
   RefCountedPtr<ListenerWatcher> listener_watcher_;
@@ -402,11 +414,15 @@ class XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager::
     : public ServerConfigSelectorProvider {
  public:
   DynamicXdsServerConfigSelectorProvider(
-      RefCountedPtr<XdsClient> xds_client, std::string resource_name,
+      RefCountedPtr<GrpcXdsClient> xds_client, std::string resource_name,
       absl::StatusOr<XdsRouteConfigResource> initial_resource,
       std::vector<XdsListenerResource::HttpConnectionManager::HttpFilter>
           http_filters);
 
+  ~DynamicXdsServerConfigSelectorProvider() override {
+    xds_client_.reset(DEBUG_LOCATION, "DynamicXdsServerConfigSelectorProvider");
+  }
+
   void Orphan() override;
 
   absl::StatusOr<RefCountedPtr<ServerConfigSelector>> Watch(
@@ -421,7 +437,7 @@ class XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager::
   void OnError(absl::Status status);
   void OnResourceDoesNotExist();
 
-  RefCountedPtr<XdsClient> xds_client_;
+  RefCountedPtr<GrpcXdsClient> xds_client_;
   std::string resource_name_;
   std::vector<XdsListenerResource::HttpConnectionManager::HttpFilter>
       http_filters_;
@@ -459,7 +475,7 @@ class XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager::
 //
 
 XdsServerConfigFetcher::XdsServerConfigFetcher(
-    RefCountedPtr<XdsClient> xds_client,
+    RefCountedPtr<GrpcXdsClient> xds_client,
     grpc_server_xds_status_notifier notifier)
     : xds_client_(std::move(xds_client)), serving_status_notifier_(notifier) {
   GPR_ASSERT(xds_client_ != nullptr);
@@ -481,8 +497,8 @@ void XdsServerConfigFetcher::StartWatch(
     std::unique_ptr<grpc_server_config_fetcher::WatcherInterface> watcher) {
   grpc_server_config_fetcher::WatcherInterface* watcher_ptr = watcher.get();
   auto listener_watcher = MakeRefCounted<ListenerWatcher>(
-      xds_client_, std::move(watcher), serving_status_notifier_,
-      listening_address);
+      xds_client_->Ref(DEBUG_LOCATION, "ListenerWatcher"), std::move(watcher),
+      serving_status_notifier_, listening_address);
   auto* listener_watcher_ptr = listener_watcher.get();
   XdsListenerResourceType::StartWatch(
       xds_client_.get(),
@@ -515,7 +531,7 @@ void XdsServerConfigFetcher::CancelWatch(
 //
 
 XdsServerConfigFetcher::ListenerWatcher::ListenerWatcher(
-    RefCountedPtr<XdsClient> xds_client,
+    RefCountedPtr<GrpcXdsClient> xds_client,
     std::unique_ptr<grpc_server_config_fetcher::WatcherInterface>
         server_config_watcher,
     grpc_server_xds_status_notifier serving_status_notifier,
@@ -539,7 +555,8 @@ void XdsServerConfigFetcher::ListenerWatcher::OnResourceChanged(
     return;
   }
   auto new_filter_chain_match_manager = MakeRefCounted<FilterChainMatchManager>(
-      xds_client_, std::move(listener.filter_chain_map),
+      xds_client_->Ref(DEBUG_LOCATION, "FilterChainMatchManager"),
+      std::move(listener.filter_chain_map),
       std::move(listener.default_filter_chain));
   MutexLock lock(&mu_);
   if (filter_chain_match_manager_ == nullptr ||
@@ -642,7 +659,7 @@ void XdsServerConfigFetcher::ListenerWatcher::
 
 XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager::
     FilterChainMatchManager(
-        RefCountedPtr<XdsClient> xds_client,
+        RefCountedPtr<GrpcXdsClient> xds_client,
         XdsListenerResource::FilterChainMap filter_chain_map,
         absl::optional<XdsListenerResource::FilterChainData>
             default_filter_chain)
@@ -1083,7 +1100,8 @@ absl::StatusOr<ChannelArgs> XdsServerConfigFetcher::ListenerWatcher::
       }
       server_config_selector_provider =
           MakeRefCounted<DynamicXdsServerConfigSelectorProvider>(
-              xds_client_,
+              xds_client_->Ref(DEBUG_LOCATION,
+                               "DynamicXdsServerConfigSelectorProvider"),
               filter_chain->http_connection_manager.route_config_name,
               std::move(initial_resource),
               filter_chain->http_connection_manager.http_filters);
@@ -1226,7 +1244,7 @@ ServerConfigSelector::CallConfig XdsServerConfigFetcher::ListenerWatcher::
 XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager::
     DynamicXdsServerConfigSelectorProvider::
         DynamicXdsServerConfigSelectorProvider(
-            RefCountedPtr<XdsClient> xds_client, std::string resource_name,
+            RefCountedPtr<GrpcXdsClient> xds_client, std::string resource_name,
             absl::StatusOr<XdsRouteConfigResource> initial_resource,
             std::vector<XdsListenerResource::HttpConnectionManager::HttpFilter>
                 http_filters)
@@ -1330,16 +1348,15 @@ grpc_server_config_fetcher* grpc_server_config_fetcher_xds_create(
       "grpc_server_config_fetcher_xds_create(notifier={on_serving_status_"
       "update=%p, user_data=%p}, args=%p)",
       3, (notifier.on_serving_status_update, notifier.user_data, args));
-  grpc_error_handle error = GRPC_ERROR_NONE;
-  grpc_core::RefCountedPtr<grpc_core::XdsClient> xds_client =
-      grpc_core::GrpcXdsClient::GetOrCreate(channel_args, &error);
-  if (!GRPC_ERROR_IS_NONE(error)) {
+  auto xds_client = grpc_core::GrpcXdsClient::GetOrCreate(
+      channel_args, "XdsServerConfigFetcher");
+  if (!xds_client.ok()) {
     gpr_log(GPR_ERROR, "Failed to create xds client: %s",
-            grpc_error_std_string(error).c_str());
-    GRPC_ERROR_UNREF(error);
+            xds_client.status().ToString().c_str());
     return nullptr;
   }
-  if (xds_client->bootstrap()
+  if ((*xds_client)
+          ->bootstrap()
           .server_listener_resource_name_template()
           .empty()) {
     gpr_log(GPR_ERROR,
@@ -1347,5 +1364,6 @@ grpc_server_config_fetcher* grpc_server_config_fetcher_xds_create(
             "file.");
     return nullptr;
   }
-  return new grpc_core::XdsServerConfigFetcher(std::move(xds_client), notifier);
+  return new grpc_core::XdsServerConfigFetcher(std::move(*xds_client),
+                                               notifier);
 }

src/core/lib/channel/channel_args.h

@@ -39,6 +39,7 @@
 
 #include "src/core/lib/avl/avl.h"
 #include "src/core/lib/gpr/useful.h"
+#include "src/core/lib/gprpp/debug_location.h"
 #include "src/core/lib/gprpp/dual_ref_counted.h"
 #include "src/core/lib/gprpp/ref_counted.h"
 #include "src/core/lib/gprpp/ref_counted_ptr.h"
@@ -86,11 +87,16 @@ struct ChannelArgTypeTraits<
     static const grpc_arg_pointer_vtable tbl = {
         // copy
         [](void* p) -> void* {
-          return p == nullptr ? nullptr : static_cast<T*>(p)->Ref().release();
+          return p == nullptr ? nullptr
+                              : static_cast<T*>(p)
+                                    ->Ref(DEBUG_LOCATION, "ChannelArgs copy")
+                                    .release();
         },
         // destroy
         [](void* p) {
-          if (p != nullptr) static_cast<T*>(p)->Unref();
+          if (p != nullptr) {
+            static_cast<T*>(p)->Unref(DEBUG_LOCATION, "ChannelArgs destroy");
+          }
         },
         // compare
         [](void* p1, void* p2) {
@@ -257,7 +263,14 @@ class ChannelArgs {
   RefCountedPtr<T> GetObjectRef() const {
     auto* p = GetObject<T>();
     if (p == nullptr) return nullptr;
-    return p->Ref();
+    return p->Ref(DEBUG_LOCATION, "ChannelArgs GetObjectRef()");
+  }
+  template <typename T>
+  RefCountedPtr<T> GetObjectRef(const DebugLocation& location,
+                                const char* reason) const {
+    auto* p = GetObject<T>();
+    if (p == nullptr) return nullptr;
+    return p->Ref(location, reason);
   }
 
   bool operator!=(const ChannelArgs& other) const;