From db1c09ad49840ae9e030c84e0fabd2e5c4ebddd8 Mon Sep 17 00:00:00 2001
From: Juanli Shen <juanlishen@google.com>
Date: Thu, 7 Feb 2019 12:51:25 -0800
Subject: [PATCH] Fix subchannel call destruction

---
 .../ext/filters/client_channel/subchannel.cc  | 36 ++++++++++---------
 .../ext/filters/client_channel/subchannel.h   |  6 ++--
 2 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc
index 35225b0d5c3..1a07edad09c 100644
--- a/src/core/ext/filters/client_channel/subchannel.cc
+++ b/src/core/ext/filters/client_channel/subchannel.cc
@@ -116,21 +116,6 @@ void ConnectedSubchannel::Ping(grpc_closure* on_initiate,
   elem->filter->start_transport_op(elem, op);
 }
 
-namespace {
-
-void SubchannelCallDestroy(void* arg, grpc_error* error) {
-  GPR_TIMER_SCOPE("subchannel_call_destroy", 0);
-  SubchannelCall* call = static_cast<SubchannelCall*>(arg);
-  grpc_closure* after_call_stack_destroy = call->after_call_stack_destroy();
-  call->~SubchannelCall();
-  // This should be the last step to destroy the subchannel call, because
-  // call->after_call_stack_destroy(), if not null, will free the call arena.
-  grpc_call_stack_destroy(SUBCHANNEL_CALL_TO_CALL_STACK(call), nullptr,
-                          after_call_stack_destroy);
-}
-
-}  // namespace
-
 RefCountedPtr<SubchannelCall> ConnectedSubchannel::CreateCall(
     const CallArgs& args, grpc_error** error) {
   const size_t allocation_size =
@@ -149,7 +134,7 @@ RefCountedPtr<SubchannelCall> ConnectedSubchannel::CreateCall(
       args.arena,        /* arena */
       args.call_combiner /* call_combiner */
   };
-  *error = grpc_call_stack_init(channel_stack_, 1, SubchannelCallDestroy,
+  *error = grpc_call_stack_init(channel_stack_, 1, SubchannelCall::Destroy,
                                 call.get(), &call_args);
   if (GPR_UNLIKELY(*error != GRPC_ERROR_NONE)) {
     const char* error_string = grpc_error_string(*error);
@@ -226,6 +211,25 @@ void SubchannelCall::Unref(const DebugLocation& location, const char* reason) {
   GRPC_CALL_STACK_UNREF(SUBCHANNEL_CALL_TO_CALL_STACK(this), reason);
 }
 
+void SubchannelCall::Destroy(void* arg, grpc_error* error) {
+  GPR_TIMER_SCOPE("subchannel_call_destroy", 0);
+  SubchannelCall* self = static_cast<SubchannelCall*>(arg);
+  // Keep some members before destroying the subchannel call.
+  grpc_closure* after_call_stack_destroy = self->after_call_stack_destroy_;
+  RefCountedPtr<ConnectedSubchannel> connected_subchannel =
+      std::move(self->connected_subchannel_);
+  // Destroy the subchannel call.
+  self->~SubchannelCall();
+  // Destroy the call stack. This should be after destroying the subchannel
+  // call, because call->after_call_stack_destroy(), if not null, will free the
+  // call arena.
+  grpc_call_stack_destroy(SUBCHANNEL_CALL_TO_CALL_STACK(self), nullptr,
+                          after_call_stack_destroy);
+  // Automatically reset connected_subchannel. This should be after destroying
+  // the call stack, because destroying call stack needs access to the channel
+  // stack.
+}
+
 void SubchannelCall::MaybeInterceptRecvTrailingMetadata(
     grpc_transport_stream_op_batch* batch) {
   // only intercept payloads with recv trailing.
diff --git a/src/core/ext/filters/client_channel/subchannel.h b/src/core/ext/filters/client_channel/subchannel.h
index 47c21ff8680..bb8e45bf965 100644
--- a/src/core/ext/filters/client_channel/subchannel.h
+++ b/src/core/ext/filters/client_channel/subchannel.h
@@ -131,10 +131,6 @@ class SubchannelCall {
   // Returns the call stack of the subchannel call.
   grpc_call_stack* GetCallStack();
 
-  grpc_closure* after_call_stack_destroy() const {
-    return after_call_stack_destroy_;
-  }
-
   // Sets the 'then_schedule_closure' argument for call stack destruction.
   // Must be called once per call.
   void SetAfterCallStackDestroy(grpc_closure* closure);
@@ -148,6 +144,8 @@ class SubchannelCall {
   void Unref();
   void Unref(const DebugLocation& location, const char* reason);
 
+  static void Destroy(void* arg, grpc_error* error);
+
  private:
   // Allow RefCountedPtr<> to access IncrementRefCount().
   template <typename T>