diff --git a/include/envoy/init/init.h b/include/envoy/init/init.h index 824dbd01fac59..d0f48b259da16 100644 --- a/include/envoy/init/init.h +++ b/include/envoy/init/init.h @@ -22,6 +22,11 @@ class Target { * initialization. */ virtual void initialize(std::function callback) PURE; + + /** + * Called when the Manager is no longer interested in this target's initialization. + */ + virtual void cancel() PURE; }; /** diff --git a/source/common/config/config_provider_impl.h b/source/common/config/config_provider_impl.h index b865165cd4f44..d1240bcb8764c 100644 --- a/source/common/config/config_provider_impl.h +++ b/source/common/config/config_provider_impl.h @@ -149,6 +149,8 @@ class ConfigSubscriptionInstanceBase : public Init::Target, start(); } + void cancel() override { initialize_callback_ = nullptr; } + /** * Starts the subscription corresponding to a config source. * A derived class must own the configuration proto specific Envoy::Config::Subscription to be diff --git a/source/common/router/rds_impl.h b/source/common/router/rds_impl.h index 4498eb99bb104..734d72b4bdb12 100644 --- a/source/common/router/rds_impl.h +++ b/source/common/router/rds_impl.h @@ -106,6 +106,7 @@ class RdsRouteConfigSubscription subscription_->start({route_config_name_}, *this); } + void cancel() override { initialize_callback_ = nullptr; } // Config::SubscriptionCallbacks void onConfigUpdate(const ResourceVector& resources, const std::string& version_info) override; void onConfigUpdateFailed(const EnvoyException* e) override; diff --git a/source/common/secret/sds_api.cc b/source/common/secret/sds_api.cc index 2471ad98657f6..2e0600f953177 100644 --- a/source/common/secret/sds_api.cc +++ b/source/common/secret/sds_api.cc @@ -40,6 +40,8 @@ void SdsApi::initialize(std::function callback) { subscription_->start({sds_config_name_}, *this); } +void SdsApi::cancel() { initialize_callback_ = nullptr; } + void SdsApi::onConfigUpdate(const ResourceVector& resources, const std::string&) { if (resources.empty()) { throw EnvoyException( diff --git a/source/common/secret/sds_api.h b/source/common/secret/sds_api.h index b19df9d54023b..291a769869daa 100644 --- a/source/common/secret/sds_api.h +++ b/source/common/secret/sds_api.h @@ -38,6 +38,7 @@ class SdsApi : public Init::Target, // Init::Target void initialize(std::function callback) override; + void cancel() override; // Config::SubscriptionCallbacks void onConfigUpdate(const ResourceVector& resources, const std::string& version_info) override; diff --git a/source/server/init_manager_impl.cc b/source/server/init_manager_impl.cc index 650d284217c1c..59564ed3d0709 100644 --- a/source/server/init_manager_impl.cc +++ b/source/server/init_manager_impl.cc @@ -14,7 +14,15 @@ InitManagerImpl::InitManagerImpl(absl::string_view description) : description_(d TRACE_INIT_MANAGER("constructor"); } -InitManagerImpl::~InitManagerImpl() { TRACE_INIT_MANAGER("destructor"); } +InitManagerImpl::~InitManagerImpl() { + TRACE_INIT_MANAGER("destructor"); + if (state_ == State::Initializing) { + for (auto& target : targets_) { + TRACE_INIT_MANAGER("canceling {}", target.second); + target.first->cancel(); + } + } +} void InitManagerImpl::initialize(std::function callback) { ASSERT(state_ == State::NotInitialized); diff --git a/source/server/lds_api.cc b/source/server/lds_api.cc index 2b5c18629b92d..d881240d67c9b 100644 --- a/source/server/lds_api.cc +++ b/source/server/lds_api.cc @@ -35,6 +35,8 @@ void LdsApiImpl::initialize(std::function callback) { subscription_->start({}, *this); } +void LdsApiImpl::cancel() { initialize_callback_ = nullptr; } + void LdsApiImpl::onConfigUpdate(const ResourceVector& resources, const std::string& version_info) { cm_.adsMux().pause(Config::TypeUrl::get().RouteConfiguration); Cleanup rds_resume([this] { cm_.adsMux().resume(Config::TypeUrl::get().RouteConfiguration); }); diff --git a/source/server/lds_api.h b/source/server/lds_api.h index fefea2e171564..b847c9dc16ae7 100644 --- a/source/server/lds_api.h +++ b/source/server/lds_api.h @@ -32,6 +32,7 @@ class LdsApiImpl : public LdsApi, // Init::Target void initialize(std::function callback) override; + void cancel() override; // Config::SubscriptionCallbacks void onConfigUpdate(const ResourceVector& resources, const std::string& version_info) override; diff --git a/test/integration/ads_integration_test.cc b/test/integration/ads_integration_test.cc index 181027ed4ee99..1ba687e743c46 100644 --- a/test/integration/ads_integration_test.cc +++ b/test/integration/ads_integration_test.cc @@ -605,6 +605,84 @@ TEST_P(AdsIntegrationTest, RdsAfterLdsWithRdsChange) { makeSingleRequest(); } +// Regression test for the use-after-free crash when a listener awaiting an RDS update is destroyed. +TEST_P(AdsIntegrationTest, RdsAfterLdsInvalidated) { + + initialize(); + + // STEP 1: Initial setup + // --------------------- + + // Initial request for any cluster, respond with cluster_0 version 1 + EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Cluster, "", {})); + sendDiscoveryResponse(Config::TypeUrl::get().Cluster, + {buildCluster("cluster_0")}, "1"); + + // Initial request for load assignment for cluster_0, respond with version 1 + EXPECT_TRUE( + compareDiscoveryRequest(Config::TypeUrl::get().ClusterLoadAssignment, "", {"cluster_0"})); + sendDiscoveryResponse( + Config::TypeUrl::get().ClusterLoadAssignment, {buildClusterLoadAssignment("cluster_0")}, "1"); + + // Request for updates to cluster_0 version 1, no response + EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Cluster, "1", {})); + + // Initial request for any listener, respond with listener_0 version 1 + EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Listener, "", {})); + sendDiscoveryResponse( + Config::TypeUrl::get().Listener, {buildListener("listener_0", "route_config_0")}, "1"); + + // Request for updates to load assignment version 1, no response + EXPECT_TRUE( + compareDiscoveryRequest(Config::TypeUrl::get().ClusterLoadAssignment, "1", {"cluster_0"})); + + // Initial request for route_config_0 (referenced by listener_0), respond with version 1 + EXPECT_TRUE( + compareDiscoveryRequest(Config::TypeUrl::get().RouteConfiguration, "", {"route_config_0"})); + sendDiscoveryResponse( + Config::TypeUrl::get().RouteConfiguration, {buildRouteConfig("route_config_0", "cluster_0")}, + "1"); + + // Wait for initial listener to be created successfully. Any subsequent listeners will then use + // the dynamic InitManager (see ListenerImpl::initManager). + test_server_->waitForCounterGe("listener_manager.listener_create_success", 1); + + // STEP 2: Listener with dynamic InitManager + // ----------------------------------------- + + // Request for updates to listener_0 version 1, respond with version 2. Under the hood, this + // invokes RdsRouteConfigSubscription::registerInitTarget with the new ListenerImpl instance. + EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Listener, "1", {})); + sendDiscoveryResponse( + Config::TypeUrl::get().Listener, {buildListener("listener_0", "route_config_1")}, "2"); + + // Request for updates to route_config_0 version 1, and initial request for route_config_1 + // (referenced by listener_0), don't respond yet! + EXPECT_TRUE( + compareDiscoveryRequest(Config::TypeUrl::get().RouteConfiguration, "1", {"route_config_0"})); + EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().RouteConfiguration, "1", + {"route_config_1", "route_config_0"})); + + // STEP 3: "New listener, who dis?" + // -------------------------------- + + // Request for updates to listener_0 version 2, respond with version 3 (updated stats prefix). + // This should blow away the previous ListenerImpl instance, which is still waiting for + // route_config_1... + EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Listener, "2", {})); + sendDiscoveryResponse( + Config::TypeUrl::get().Listener, {buildListener("listener_0", "route_config_1", "omg")}, "3"); + + // Respond to prior request for route_config_1. Under the hood, this invokes + // RdsRouteConfigSubscription::runInitializeCallbackIfAny, which references the defunct + // ListenerImpl instance. We should not crash in this event! + sendDiscoveryResponse( + Config::TypeUrl::get().RouteConfiguration, {buildRouteConfig("route_config_1", "cluster_0")}, + "1"); + + test_server_->waitForCounterGe("listener_manager.listener_create_success", 2); +} + class AdsFailIntegrationTest : public Grpc::GrpcClientIntegrationParamTest, public HttpIntegrationTest { public: diff --git a/test/mocks/init/mocks.cc b/test/mocks/init/mocks.cc index f968ad7c290bc..ff9b6472fd2a5 100644 --- a/test/mocks/init/mocks.cc +++ b/test/mocks/init/mocks.cc @@ -17,6 +17,10 @@ MockTarget::MockTarget() { EXPECT_EQ(nullptr, callback_); callback_ = callback; })); + ON_CALL(*this, cancel()).WillByDefault(Invoke([this]() { + EXPECT_NE(nullptr, callback_); + callback_ = nullptr; + })); } MockTarget::~MockTarget() {} diff --git a/test/mocks/init/mocks.h b/test/mocks/init/mocks.h index e8f6d093a8270..a564a08648dbc 100644 --- a/test/mocks/init/mocks.h +++ b/test/mocks/init/mocks.h @@ -18,6 +18,7 @@ class MockTarget : public Target { ~MockTarget(); MOCK_METHOD1(initialize, void(std::function callback)); + MOCK_METHOD0(cancel, void()); std::function callback_; }; diff --git a/test/server/init_manager_impl_test.cc b/test/server/init_manager_impl_test.cc index 964db18551670..efe4cd4a59c7b 100644 --- a/test/server/init_manager_impl_test.cc +++ b/test/server/init_manager_impl_test.cc @@ -64,6 +64,18 @@ TEST_F(InitManagerImplTest, TargetAfterInitializing) { target1.callback_(); } +TEST_F(InitManagerImplTest, Cancel) { + InitManagerImpl* manager = new InitManagerImpl("test"); + Init::MockTarget target; + + manager->registerTarget(target, ""); + EXPECT_CALL(target, initialize(_)); + manager->initialize([&]() -> void { initialized_.ready(); }); + + EXPECT_CALL(target, cancel()); + delete manager; +} + } // namespace } // namespace Server } // namespace Envoy diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index b10e139e42df5..a57bb369374e3 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -51,7 +51,6 @@ class ListenerHandle { MOCK_METHOD0(onDestroy, void()); - Init::MockTarget target_; MockDrainManager* drain_manager_ = new MockDrainManager(); Configuration::FactoryContext* context_{}; }; @@ -72,20 +71,20 @@ class ListenerManagerImplTest : public testing::Test { * 4) Creates a mock local drain manager for the listener. */ ListenerHandle* expectListenerCreate( - bool need_init, + bool need_init, Init::Target& target, envoy::api::v2::Listener::DrainType drain_type = envoy::api::v2::Listener_DrainType_DEFAULT) { ListenerHandle* raw_listener = new ListenerHandle(); EXPECT_CALL(listener_factory_, createDrainManager_(drain_type)) .WillOnce(Return(raw_listener->drain_manager_)); EXPECT_CALL(listener_factory_, createNetworkFilterFactoryList(_, _)) .WillOnce(Invoke( - [raw_listener, need_init]( + [raw_listener, need_init, &target]( const Protobuf::RepeatedPtrField&, Configuration::FactoryContext& context) -> std::vector { std::shared_ptr notifier(raw_listener); raw_listener->context_ = &context; if (need_init) { - context.initManager().registerTarget(notifier->target_, ""); + context.initManager().registerTarget(target, ""); } return {[notifier](Network::FilterManager&) -> void {}}; })); @@ -505,8 +504,9 @@ TEST_F(ListenerManagerImplTest, ModifyOnlyDrainType) { drain_type: MODIFY_ONLY )EOF"; + Init::MockTarget target_foo; ListenerHandle* listener_foo = - expectListenerCreate(false, envoy::api::v2::Listener_DrainType_MODIFY_ONLY); + expectListenerCreate(false, target_foo, envoy::api::v2::Listener_DrainType_MODIFY_ONLY); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_yaml), "", true)); checkStats(1, 0, 0, 0, 1, 0); @@ -527,7 +527,8 @@ TEST_F(ListenerManagerImplTest, AddListenerAddressNotMatching) { } )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(false); + Init::MockTarget target_foo; + ListenerHandle* listener_foo = expectListenerCreate(false, target_foo); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", true)); checkStats(1, 0, 0, 0, 1, 0); @@ -542,8 +543,9 @@ TEST_F(ListenerManagerImplTest, AddListenerAddressNotMatching) { } )EOF"; - ListenerHandle* listener_foo_different_address = - expectListenerCreate(false, envoy::api::v2::Listener_DrainType_MODIFY_ONLY); + Init::MockTarget target_foo_different_address; + ListenerHandle* listener_foo_different_address = expectListenerCreate( + false, target_foo_different_address, envoy::api::v2::Listener_DrainType_MODIFY_ONLY); EXPECT_CALL(*listener_foo_different_address, onDestroy()); EXPECT_THROW_WITH_MESSAGE( manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_different_address_json), "", @@ -573,7 +575,8 @@ TEST_F(ListenerManagerImplTest, AddListenerOnIpv4OnlySetups) { } )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(false); + Init::MockTarget target_foo; + ListenerHandle* listener_foo = expectListenerCreate(false, target_foo); EXPECT_CALL(os_sys_calls, socket(AF_INET, _, 0)).WillOnce(Return(Api::SysCallIntResult{5, 0})); EXPECT_CALL(os_sys_calls, socket(AF_INET6, _, 0)).WillOnce(Return(Api::SysCallIntResult{-1, 0})); @@ -603,7 +606,8 @@ TEST_F(ListenerManagerImplTest, AddListenerOnIpv6OnlySetups) { } )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(false); + Init::MockTarget target_foo; + ListenerHandle* listener_foo = expectListenerCreate(false, target_foo); EXPECT_CALL(os_sys_calls, socket(AF_INET, _, 0)).WillOnce(Return(Api::SysCallIntResult{-1, 0})); EXPECT_CALL(os_sys_calls, socket(AF_INET6, _, 0)).WillOnce(Return(Api::SysCallIntResult{5, 0})); @@ -630,7 +634,8 @@ TEST_F(ListenerManagerImplTest, UpdateRemoveNotModifiableListener) { } )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(false); + Init::MockTarget target_foo; + ListenerHandle* listener_foo = expectListenerCreate(false, target_foo); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", false)); checkStats(1, 0, 0, 0, 1, 0); @@ -701,7 +706,8 @@ name: "foo" filter_chains: {} )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(false); + Init::MockTarget target_foo; + ListenerHandle* listener_foo = expectListenerCreate(false, target_foo); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_TRUE( manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_yaml), "version1", true)); @@ -743,7 +749,8 @@ per_connection_buffer_limit_bytes: 10 time_system_.setSystemTime(std::chrono::milliseconds(2002002002002)); - ListenerHandle* listener_foo_update1 = expectListenerCreate(false); + Init::MockTarget target_foo_update1; + ListenerHandle* listener_foo_update1 = expectListenerCreate(false, target_foo_update1); EXPECT_CALL(*listener_foo, onDestroy()); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_update1_yaml), "version2", true)); @@ -784,7 +791,8 @@ version_info: version2 // Update foo. Should go into warming, have an immediate warming callback, and start immediate // removal. - ListenerHandle* listener_foo_update2 = expectListenerCreate(false); + Init::MockTarget target_foo_update2; + ListenerHandle* listener_foo_update2 = expectListenerCreate(false, target_foo_update2); EXPECT_CALL(*worker_, addListener(_, _)); EXPECT_CALL(*worker_, stopListener(_)); EXPECT_CALL(*listener_foo_update1->drain_manager_, startDrainSequence(_)); @@ -843,7 +851,8 @@ name: "bar" filter_chains: {} )EOF"; - ListenerHandle* listener_bar = expectListenerCreate(false); + Init::MockTarget target_bar; + ListenerHandle* listener_bar = expectListenerCreate(false, target_bar); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_CALL(*worker_, addListener(_, _)); EXPECT_TRUE( @@ -864,9 +873,10 @@ name: "baz" filter_chains: {} )EOF"; - ListenerHandle* listener_baz = expectListenerCreate(true); + Init::MockTarget target_baz; + ListenerHandle* listener_baz = expectListenerCreate(true, target_baz); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); - EXPECT_CALL(listener_baz->target_, initialize(_)); + EXPECT_CALL(target_baz, initialize(_)); EXPECT_TRUE( manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_baz_yaml), "version5", true)); EXPECT_EQ(2UL, manager_->listeners().size()); @@ -928,12 +938,13 @@ version_info: version5 } )EOF"; - ListenerHandle* listener_baz_update1 = expectListenerCreate(true); - EXPECT_CALL(*listener_baz, onDestroy()).WillOnce(Invoke([listener_baz]() -> void { + Init::MockTarget target_baz_update1; + ListenerHandle* listener_baz_update1 = expectListenerCreate(true, target_baz_update1); + EXPECT_CALL(*listener_baz, onDestroy()).WillOnce(Invoke([&target_baz]() -> void { // Call the initialize callback during destruction like RDS will. - listener_baz->target_.callback_(); + target_baz.callback_(); })); - EXPECT_CALL(listener_baz_update1->target_, initialize(_)); + EXPECT_CALL(target_baz_update1, initialize(_)); EXPECT_TRUE( manager_->addOrUpdateListener(parseListenerFromJson(listener_baz_update1_json), "", true)); EXPECT_EQ(2UL, manager_->listeners().size()); @@ -941,7 +952,7 @@ version_info: version5 // Finish initialization for baz which should make it active. EXPECT_CALL(*worker_, addListener(_, _)); - listener_baz_update1->target_.callback_(); + target_baz_update1.callback_(); EXPECT_EQ(3UL, manager_->listeners().size()); worker_->callAddCompletion(true); checkStats(3, 3, 0, 0, 3, 0); @@ -970,7 +981,8 @@ TEST_F(ListenerManagerImplTest, AddDrainingListener) { new Network::Address::Ipv4Instance("127.0.0.1", 1234)); ON_CALL(*listener_factory_.socket_, localAddress()).WillByDefault(ReturnRef(local_address)); - ListenerHandle* listener_foo = expectListenerCreate(false); + Init::MockTarget target_foo; + ListenerHandle* listener_foo = expectListenerCreate(false, target_foo); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_CALL(*worker_, addListener(_, _)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", true)); @@ -987,7 +999,8 @@ TEST_F(ListenerManagerImplTest, AddDrainingListener) { checkStats(1, 0, 1, 0, 0, 1); // Add foo again. We should use the socket from draining. - ListenerHandle* listener_foo2 = expectListenerCreate(false); + Init::MockTarget target_foo2; + ListenerHandle* listener_foo2 = expectListenerCreate(false, target_foo2); EXPECT_CALL(*worker_, addListener(_, _)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", true)); worker_->callAddCompletion(true); @@ -1014,7 +1027,8 @@ TEST_F(ListenerManagerImplTest, CantBindSocket) { } )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(true); + Init::MockTarget target_foo; + ListenerHandle* listener_foo = expectListenerCreate(true, target_foo); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)) .WillOnce(Throw(EnvoyException("can't bind"))); EXPECT_CALL(*listener_foo, onDestroy()); @@ -1036,7 +1050,8 @@ TEST_F(ListenerManagerImplTest, ListenerDraining) { } )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(false); + Init::MockTarget target_foo; + ListenerHandle* listener_foo = expectListenerCreate(false, target_foo); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_CALL(*worker_, addListener(_, _)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", true)); @@ -1088,27 +1103,30 @@ TEST_F(ListenerManagerImplTest, RemoveListener) { } )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(true); + Init::MockTarget target_foo; + ListenerHandle* listener_foo = expectListenerCreate(true, target_foo); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); - EXPECT_CALL(listener_foo->target_, initialize(_)); + EXPECT_CALL(target_foo, initialize(_)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", true)); EXPECT_EQ(0UL, manager_->listeners().size()); checkStats(1, 0, 0, 1, 0, 0); // Remove foo. EXPECT_CALL(*listener_foo, onDestroy()); + EXPECT_CALL(target_foo, cancel()); EXPECT_TRUE(manager_->removeListener("foo")); EXPECT_EQ(0UL, manager_->listeners().size()); checkStats(1, 0, 1, 0, 0, 0); // Add foo again and initialize it. - listener_foo = expectListenerCreate(true); + listener_foo = expectListenerCreate(true, target_foo); + target_foo.callback_ = nullptr; EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); - EXPECT_CALL(listener_foo->target_, initialize(_)); + EXPECT_CALL(target_foo, initialize(_)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", true)); checkStats(2, 0, 1, 1, 0, 0); EXPECT_CALL(*worker_, addListener(_, _)); - listener_foo->target_.callback_(); + target_foo.callback_(); worker_->callAddCompletion(true); EXPECT_EQ(1UL, manager_->listeners().size()); checkStats(2, 0, 1, 0, 1, 0); @@ -1124,8 +1142,9 @@ TEST_F(ListenerManagerImplTest, RemoveListener) { } )EOF"; - ListenerHandle* listener_foo_update1 = expectListenerCreate(true); - EXPECT_CALL(listener_foo_update1->target_, initialize(_)); + Init::MockTarget target_foo_update1; + ListenerHandle* listener_foo_update1 = expectListenerCreate(true, target_foo_update1); + EXPECT_CALL(target_foo_update1, initialize(_)); EXPECT_TRUE( manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_update1_json), "", true)); EXPECT_EQ(1UL, manager_->listeners().size()); @@ -1133,6 +1152,7 @@ TEST_F(ListenerManagerImplTest, RemoveListener) { // Remove foo which should remove both warming and active. EXPECT_CALL(*listener_foo_update1, onDestroy()); + EXPECT_CALL(target_foo_update1, cancel()); EXPECT_CALL(*worker_, stopListener(_)); EXPECT_CALL(*listener_foo->drain_manager_, startDrainSequence(_)); EXPECT_TRUE(manager_->removeListener("foo")); @@ -1161,7 +1181,8 @@ TEST_F(ListenerManagerImplTest, AddListenerFailure) { } )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(false); + Init::MockTarget target_foo; + ListenerHandle* listener_foo = expectListenerCreate(false, target_foo); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, true)); EXPECT_CALL(*worker_, addListener(_, _)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", true)); @@ -1210,9 +1231,10 @@ TEST_F(ListenerManagerImplTest, DuplicateAddressDontBind) { } )EOF"; - ListenerHandle* listener_foo = expectListenerCreate(true); + Init::MockTarget target_foo; + ListenerHandle* listener_foo = expectListenerCreate(true, target_foo); EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, false)); - EXPECT_CALL(listener_foo->target_, initialize(_)); + EXPECT_CALL(target_foo, initialize(_)); EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromJson(listener_foo_json), "", true)); // Add bar with same non-binding address. Should fail. @@ -1225,7 +1247,8 @@ TEST_F(ListenerManagerImplTest, DuplicateAddressDontBind) { } )EOF"; - ListenerHandle* listener_bar = expectListenerCreate(true); + Init::MockTarget target_bar; + ListenerHandle* listener_bar = expectListenerCreate(true, target_bar); EXPECT_CALL(*listener_bar, onDestroy()); EXPECT_THROW_WITH_MESSAGE( manager_->addOrUpdateListener(parseListenerFromJson(listener_bar_json), "", true), @@ -1234,10 +1257,10 @@ TEST_F(ListenerManagerImplTest, DuplicateAddressDontBind) { // Move foo to active and then try to add again. This should still fail. EXPECT_CALL(*worker_, addListener(_, _)); - listener_foo->target_.callback_(); + target_foo.callback_(); worker_->callAddCompletion(true); - listener_bar = expectListenerCreate(true); + listener_bar = expectListenerCreate(true, target_bar); EXPECT_CALL(*listener_bar, onDestroy()); EXPECT_THROW_WITH_MESSAGE( manager_->addOrUpdateListener(parseListenerFromJson(listener_bar_json), "", true),