Simplify LB policy and resolver shutdown.

src/core/ext/filters/client_channel/client_channel.cc

@@ -1435,10 +1435,8 @@ ChannelData::ChannelData(grpc_channel_element_args* args, grpc_error** error)
       std::move(target_uri), ProcessResolverResultLocked, this, error));
   grpc_channel_args_destroy(new_args);
   if (*error != GRPC_ERROR_NONE) {
-    // Orphan the resolving LB policy and flush the exec_ctx to ensure
-    // that it finishes shutting down.  This ensures that if we are
-    // failing, we destroy the ClientChannelControlHelper (and thus
-    // unref the channel stack) before we return.
+    // Before we return, shut down the resolving LB policy, which destroys
+    // the ClientChannelControlHelper and therefore unrefs the channel stack.
     // TODO(roth): This is not a complete solution, because it only
     // catches the case where channel stack initialization fails in this
     // particular filter.  If there is a failure in a different filter, we
@@ -1446,7 +1444,6 @@ ChannelData::ChannelData(grpc_channel_element_args* args, grpc_error** error)
     // in practice, there are no other filters that can cause failures in
     // channel stack initialization, so this works for now.
     resolving_lb_policy_.reset();
-    ExecCtx::Get()->Flush();
   } else {
     grpc_pollset_set_add_pollset_set(resolving_lb_policy_->interested_parties(),
                                      interested_parties_);

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

@@ -43,23 +43,8 @@ LoadBalancingPolicy::~LoadBalancingPolicy() {
 }
 
 void LoadBalancingPolicy::Orphan() {
-  // Invoke ShutdownAndUnrefLocked() inside of the combiner.
-  // TODO(roth): Is this actually needed?  We should already be in the
-  // combiner here.  Note that if we directly call ShutdownLocked(),
-  // then we can probably remove the hack whereby the helper is
-  // destroyed at shutdown instead of at destruction.
-  GRPC_CLOSURE_SCHED(
-      GRPC_CLOSURE_CREATE(&LoadBalancingPolicy::ShutdownAndUnrefLocked, this,
-                          grpc_combiner_scheduler(combiner_)),
-      GRPC_ERROR_NONE);
-}
-
-void LoadBalancingPolicy::ShutdownAndUnrefLocked(void* arg,
-                                                 grpc_error* ignored) {
-  LoadBalancingPolicy* policy = static_cast<LoadBalancingPolicy*>(arg);
-  policy->ShutdownLocked();
-  policy->channel_control_helper_.reset();
-  policy->Unref();
+  ShutdownLocked();
+  Unref();
 }
 
 //

src/core/ext/filters/client_channel/lb_policy.h

@@ -282,6 +282,7 @@ class LoadBalancingPolicy : public InternallyRefCounted<LoadBalancingPolicy> {
 
   grpc_pollset_set* interested_parties() const { return interested_parties_; }
 
+  // Note: This must be invoked while holding the combiner.
   void Orphan() override;
 
   // A picker that returns PICK_QUEUE for all picks.
@@ -322,7 +323,6 @@ class LoadBalancingPolicy : public InternallyRefCounted<LoadBalancingPolicy> {
 
   // Note: LB policies MUST NOT call any method on the helper from their
   // constructor.
-  // Note: This will return null after ShutdownLocked() has been called.
   ChannelControlHelper* channel_control_helper() const {
     return channel_control_helper_.get();
   }
@@ -331,8 +331,6 @@ class LoadBalancingPolicy : public InternallyRefCounted<LoadBalancingPolicy> {
   virtual void ShutdownLocked() GRPC_ABSTRACT;
 
  private:
-  static void ShutdownAndUnrefLocked(void* arg, grpc_error* ignored);
-
   /// Combiner under which LB policy actions take place.
   grpc_combiner* combiner_;
   /// Owned pointer to interested parties in load balancing decisions.

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

@@ -1989,6 +1989,9 @@ void XdsLb::LocalityMap::LocalityEntry::ShutdownLocked() {
         parent_->interested_parties());
     pending_child_policy_.reset();
   }
+  // Drop our ref to the child's picker, in case it's holding a ref to
+  // the child.
+  picker_ref_.reset();
 }
 
 void XdsLb::LocalityMap::LocalityEntry::ResetBackoffLocked() {

src/core/ext/filters/client_channel/resolver.h

@@ -117,12 +117,10 @@ class Resolver : public InternallyRefCounted<Resolver> {
   /// implementations.  At that point, this method can go away.
   virtual void ResetBackoffLocked() {}
 
+  // Note: This must be invoked while holding the combiner.
   void Orphan() override {
-    // Invoke ShutdownAndUnrefLocked() inside of the combiner.
-    GRPC_CLOSURE_SCHED(
-        GRPC_CLOSURE_CREATE(&Resolver::ShutdownAndUnrefLocked, this,
-                            grpc_combiner_scheduler(combiner_)),
-        GRPC_ERROR_NONE);
+    ShutdownLocked();
+    Unref();
   }
 
   GRPC_ABSTRACT_BASE_CLASS
@@ -147,12 +145,6 @@ class Resolver : public InternallyRefCounted<Resolver> {
   ResultHandler* result_handler() const { return result_handler_.get(); }
 
  private:
-  static void ShutdownAndUnrefLocked(void* arg, grpc_error* ignored) {
-    Resolver* resolver = static_cast<Resolver*>(arg);
-    resolver->ShutdownLocked();
-    resolver->Unref();
-  }
-
   UniquePtr<ResultHandler> result_handler_;
   grpc_combiner* combiner_;
 };