Merge pull request #19389 from markdroth/lb_policy_resolver_shutdown

Second attempt: Simplify LB policy and resolver shutdown
pull/19765/head^2
Mark D. Roth 6 years ago committed by GitHub
commit b8b6df08ae
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 19
      src/core/ext/filters/client_channel/lb_policy.cc
  2. 4
      src/core/ext/filters/client_channel/lb_policy.h
  3. 3
      src/core/ext/filters/client_channel/lb_policy/xds/xds.cc
  4. 14
      src/core/ext/filters/client_channel/resolver.h

@ -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();
}
//

@ -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.

@ -1986,6 +1986,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() {

@ -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_;
};

Loading…
Cancel
Save