From b7439966f01d6ad004b4ec7733e81e353e95ac65 Mon Sep 17 00:00:00 2001
From: "Mark D. Roth" <roth@google.com>
Date: Wed, 4 Sep 2024 16:06:05 +0000
Subject: [PATCH] fix memory leak

---
 src/core/xds/grpc/xds_transport_grpc.cc | 18 ++++++++++++++++--
 src/core/xds/grpc/xds_transport_grpc.h  |  1 +
 src/core/xds/xds_client/xds_transport.h |  3 +++
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/src/core/xds/grpc/xds_transport_grpc.cc b/src/core/xds/grpc/xds_transport_grpc.cc
index 7685b07d079..c065f08ed2a 100644
--- a/src/core/xds/grpc/xds_transport_grpc.cc
+++ b/src/core/xds/grpc/xds_transport_grpc.cc
@@ -41,6 +41,7 @@
 #include "src/core/lib/channel/channel_fwd.h"
 #include "src/core/lib/channel/channel_stack.h"
 #include "src/core/lib/config/core_configuration.h"
+#include "src/core/lib/debug/trace.h"
 #include "src/core/lib/event_engine/default_event_engine.h"
 #include "src/core/lib/gprpp/debug_location.h"
 #include "src/core/lib/gprpp/orphanable.h"
@@ -265,7 +266,13 @@ RefCountedPtr<Channel> CreateXdsChannel(const ChannelArgs& args,
 GrpcXdsTransportFactory::GrpcXdsTransport::GrpcXdsTransport(
     WeakRefCountedPtr<GrpcXdsTransportFactory> factory,
     const XdsBootstrap::XdsServer& server, absl::Status* status)
-    : factory_(std::move(factory)), key_(server.Key()) {
+    : XdsTransport(GRPC_TRACE_FLAG_ENABLED(xds_client_refcount)
+                       ? "GrpcXdsTransport"
+                       : nullptr),
+      factory_(std::move(factory)),
+      key_(server.Key()) {
+  GRPC_TRACE_LOG(xds_client, INFO)
+      << "[GrpcXdsTransport " << this << "] created";
   channel_ = CreateXdsChannel(factory_->args_,
                               static_cast<const GrpcXdsServer&>(server));
   CHECK(channel_ != nullptr);
@@ -274,7 +281,14 @@ GrpcXdsTransportFactory::GrpcXdsTransport::GrpcXdsTransport(
   }
 }
 
+GrpcXdsTransportFactory::GrpcXdsTransport::~GrpcXdsTransport() {
+  GRPC_TRACE_LOG(xds_client, INFO)
+      << "[GrpcXdsTransport " << this << "] destroying";
+}
+
 void GrpcXdsTransportFactory::GrpcXdsTransport::Orphaned() {
+  GRPC_TRACE_LOG(xds_client, INFO)
+      << "[GrpcXdsTransport " << this << "] orphaned";
   {
     MutexLock lock(&factory_->mu_);
     auto it = factory_->transports_.find(key_);
@@ -289,7 +303,7 @@ void GrpcXdsTransportFactory::GrpcXdsTransport::Orphaned() {
       [self = WeakRefAsSubclass<GrpcXdsTransport>()]() mutable {
         ApplicationCallbackExecCtx application_exec_ctx;
         ExecCtx exec_ctx;
-        self.release();
+        self.reset();
       });
 }
 
diff --git a/src/core/xds/grpc/xds_transport_grpc.h b/src/core/xds/grpc/xds_transport_grpc.h
index 48bd47d4129..c771929bc05 100644
--- a/src/core/xds/grpc/xds_transport_grpc.h
+++ b/src/core/xds/grpc/xds_transport_grpc.h
@@ -72,6 +72,7 @@ class GrpcXdsTransportFactory::GrpcXdsTransport final
 
   GrpcXdsTransport(WeakRefCountedPtr<GrpcXdsTransportFactory> factory,
                    const XdsBootstrap::XdsServer& server, absl::Status* status);
+  ~GrpcXdsTransport() override;
 
   void Orphaned() override;
 
diff --git a/src/core/xds/xds_client/xds_transport.h b/src/core/xds/xds_client/xds_transport.h
index 9050224e750..0233c4efa78 100644
--- a/src/core/xds/xds_client/xds_transport.h
+++ b/src/core/xds/xds_client/xds_transport.h
@@ -70,6 +70,9 @@ class XdsTransportFactory : public DualRefCounted<XdsTransportFactory> {
       virtual void OnConnectivityFailure(absl::Status status) = 0;
     };
 
+    explicit XdsTransport(const char* trace = nullptr)
+        : DualRefCounted(trace) {}
+
     // Starts a connectivity failure watcher on the transport.
     virtual void StartConnectivityFailureWatch(
         RefCountedPtr<ConnectivityFailureWatcher> watcher) = 0;