diff --git a/source/common/upstream/health_checker_base_impl.cc b/source/common/upstream/health_checker_base_impl.cc index 21e68d6b518b7..5858fdbadc31e 100644 --- a/source/common/upstream/health_checker_base_impl.cc +++ b/source/common/upstream/health_checker_base_impl.cc @@ -40,8 +40,11 @@ HealthCheckerImplBase::HealthCheckerImplBase(const Cluster& cluster, } HealthCheckerImplBase::~HealthCheckerImplBase() { - // Make sure that any sessions that were deferred deleted are cleared before we destruct. - dispatcher_.clearDeferredDeleteList(); + // ASSERTs inside the session destructor check to make sure we have been previously deferred + // deleted. Unify that logic here before actual destruction happens. + for (auto& session : active_sessions_) { + session.second->onDeferredDeleteBase(); + } } void HealthCheckerImplBase::decHealthy() { @@ -226,12 +229,9 @@ HealthCheckerImplBase::ActiveHealthCheckSession::ActiveHealthCheckSession( } HealthCheckerImplBase::ActiveHealthCheckSession::~ActiveHealthCheckSession() { - if (!host_->healthFlagGet(Host::HealthFlag::FAILED_ACTIVE_HC)) { - parent_.decHealthy(); - } - if (host_->healthFlagGet(Host::HealthFlag::DEGRADED_ACTIVE_HC)) { - parent_.decDegraded(); - } + // Make sure onDeferredDeleteBase() has been called. We should not reference our parent at this + // point since we may have been deferred deleted. + ASSERT(interval_timer_ == nullptr && timeout_timer_ == nullptr); } void HealthCheckerImplBase::ActiveHealthCheckSession::onDeferredDeleteBase() { @@ -239,6 +239,12 @@ void HealthCheckerImplBase::ActiveHealthCheckSession::onDeferredDeleteBase() { // implementation specific state is destroyed. interval_timer_.reset(); timeout_timer_.reset(); + if (!host_->healthFlagGet(Host::HealthFlag::FAILED_ACTIVE_HC)) { + parent_.decHealthy(); + } + if (host_->healthFlagGet(Host::HealthFlag::DEGRADED_ACTIVE_HC)) { + parent_.decDegraded(); + } onDeferredDelete(); } diff --git a/source/common/upstream/health_checker_impl.cc b/source/common/upstream/health_checker_impl.cc index 9acc50bca9bf8..a5f93d4ccc803 100644 --- a/source/common/upstream/health_checker_impl.cc +++ b/source/common/upstream/health_checker_impl.cc @@ -154,7 +154,6 @@ HttpHealthCheckerImpl::HttpActiveHealthCheckSession::HttpActiveHealthCheckSessio local_address_(std::make_shared("127.0.0.1")) {} HttpHealthCheckerImpl::HttpActiveHealthCheckSession::~HttpActiveHealthCheckSession() { - onDeferredDelete(); ASSERT(client_ == nullptr); } @@ -379,7 +378,6 @@ TcpHealthCheckerImpl::TcpHealthCheckerImpl(const Cluster& cluster, receive_bytes_(TcpHealthCheckMatcher::loadProtoBytes(config.tcp_health_check().receive())) {} TcpHealthCheckerImpl::TcpActiveHealthCheckSession::~TcpActiveHealthCheckSession() { - onDeferredDelete(); ASSERT(client_ == nullptr); } @@ -487,7 +485,6 @@ GrpcHealthCheckerImpl::GrpcActiveHealthCheckSession::GrpcActiveHealthCheckSessio : ActiveHealthCheckSession(parent, host), parent_(parent) {} GrpcHealthCheckerImpl::GrpcActiveHealthCheckSession::~GrpcActiveHealthCheckSession() { - onDeferredDelete(); ASSERT(client_ == nullptr); } diff --git a/source/extensions/health_checkers/redis/redis.cc b/source/extensions/health_checkers/redis/redis.cc index b6342316499ae..9f130824639c8 100644 --- a/source/extensions/health_checkers/redis/redis.cc +++ b/source/extensions/health_checkers/redis/redis.cc @@ -25,7 +25,6 @@ RedisHealthChecker::RedisActiveHealthCheckSession::RedisActiveHealthCheckSession : ActiveHealthCheckSession(parent, host), parent_(parent) {} RedisHealthChecker::RedisActiveHealthCheckSession::~RedisActiveHealthCheckSession() { - onDeferredDelete(); ASSERT(current_request_ == nullptr); ASSERT(client_ == nullptr); } diff --git a/test/common/upstream/health_checker_impl_test.cc b/test/common/upstream/health_checker_impl_test.cc index 2d86d68c3b699..7ea5c824b92aa 100644 --- a/test/common/upstream/health_checker_impl_test.cc +++ b/test/common/upstream/health_checker_impl_test.cc @@ -81,7 +81,6 @@ TEST(HealthCheckerFactoryTest, CreateGrpc) { Event::MockDispatcher dispatcher; AccessLog::MockAccessLogManager log_manager; - EXPECT_CALL(dispatcher, clearDeferredDeleteList()); EXPECT_NE(nullptr, dynamic_cast( HealthCheckerFactory::create(createGrpcHealthCheckConfig(), cluster, runtime, random, dispatcher, log_manager) diff --git a/test/extensions/health_checkers/redis/config_test.cc b/test/extensions/health_checkers/redis/config_test.cc index 0985aa90a92a9..b05edf269d830 100644 --- a/test/extensions/health_checkers/redis/config_test.cc +++ b/test/extensions/health_checkers/redis/config_test.cc @@ -107,7 +107,6 @@ TEST(HealthCheckerFactoryTest, CreateRedisViaUpstreamHealthCheckerFactory) { Event::MockDispatcher dispatcher; AccessLog::MockAccessLogManager log_manager; - EXPECT_CALL(dispatcher, clearDeferredDeleteList()); EXPECT_NE(nullptr, dynamic_cast( Upstream::HealthCheckerFactory::create( Upstream::parseHealthCheckFromV2Yaml(yaml), cluster, runtime, random,