diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 81160b941eb0..b3f89cb6213c 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -95,7 +95,12 @@ void ClusterManagerInitHelper::maybeFinishInitialize() { if (!started_secondary_initialize_) { log().info("cm init: initializing secondary clusters"); started_secondary_initialize_ = true; - for (Cluster* cluster : secondary_init_clusters_) { + // Cluster::initialize() method can modify the list of secondary_init_clusters_ to remove + // the item currently being initialized, so we eschew range-based-for and do this complicated + // dance to increment the iterator before calling initialize. + for (auto iter = secondary_init_clusters_.begin(); iter != secondary_init_clusters_.end();) { + Cluster* cluster = *iter; + ++iter; cluster->initialize(); } } diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index a48e67f374ab..53a274bdcc03 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -838,4 +838,25 @@ TEST(ClusterManagerInitHelper, AddSecondaryAfterSecondaryInit) { cluster2.initialize_callback_(); } +TEST(ClusterManagerInitHelper, RemoveClusterWithinInitLoop) { + // Tests the scenario encountered in Issue 903: The cluster was removed from + // the secondary init list while traversing the list. + + InSequence s; + ClusterManagerInitHelper init_helper; + NiceMock cluster; + ON_CALL(cluster, initializePhase()).WillByDefault(Return(Cluster::InitializePhase::Secondary)); + init_helper.addCluster(cluster); + + // Set up the scenario seen in Issue 903 where initialize() ultimately results + // in the removeCluster() call. In the real bug this was a long and complex call + // chain. + EXPECT_CALL(cluster, initialize()) + .WillOnce(Invoke([&]() -> void { init_helper.removeCluster(cluster); })); + + // Now call onStaticLoadComplete which will exercise maybeFinishInitialize() + // which calls initialize() on the members of the secondary init list. + init_helper.onStaticLoadComplete(); +} + } // Upstream