diff --git a/source/exe/main_common.cc b/source/exe/main_common.cc index ed76b4381847..c91f5144bd03 100644 --- a/source/exe/main_common.cc +++ b/source/exe/main_common.cc @@ -77,9 +77,9 @@ MainCommonBase::MainCommonBase(const OptionsImpl& options, Event::TimeSystem& ti stats_store_ = std::make_unique(stats_allocator_); server_ = std::make_unique( - options_, time_system, local_address, listener_hooks, *restarter_, *stats_store_, - access_log_lock, component_factory, std::move(random_generator), *tls_, thread_factory_, - file_system_, std::move(process_context)); + *init_manager_, options_, time_system, local_address, listener_hooks, *restarter_, + *stats_store_, access_log_lock, component_factory, std::move(random_generator), *tls_, + thread_factory_, file_system_, std::move(process_context)); break; } diff --git a/source/exe/main_common.h b/source/exe/main_common.h index 72d4e041374d..d2141061854a 100644 --- a/source/exe/main_common.h +++ b/source/exe/main_common.h @@ -82,6 +82,7 @@ class MainCommonBase { std::unique_ptr restarter_; std::unique_ptr stats_store_; std::unique_ptr logging_context_; + std::unique_ptr init_manager_{std::make_unique("Server")}; std::unique_ptr server_; private: diff --git a/source/server/server.cc b/source/server/server.cc index 1afeff023cfe..8aba537de9b2 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -47,16 +47,14 @@ namespace Envoy { namespace Server { -InstanceImpl::InstanceImpl(const Options& options, Event::TimeSystem& time_system, - Network::Address::InstanceConstSharedPtr local_address, - ListenerHooks& hooks, HotRestart& restarter, Stats::StoreRoot& store, - Thread::BasicLockable& access_log_lock, - ComponentFactory& component_factory, - Runtime::RandomGeneratorPtr&& random_generator, - ThreadLocal::Instance& tls, Thread::ThreadFactory& thread_factory, - Filesystem::Instance& file_system, - std::unique_ptr process_context) - : workers_started_(false), shutdown_(false), options_(options), +InstanceImpl::InstanceImpl( + Init::Manager& init_manager, const Options& options, Event::TimeSystem& time_system, + Network::Address::InstanceConstSharedPtr local_address, ListenerHooks& hooks, + HotRestart& restarter, Stats::StoreRoot& store, Thread::BasicLockable& access_log_lock, + ComponentFactory& component_factory, Runtime::RandomGeneratorPtr&& random_generator, + ThreadLocal::Instance& tls, Thread::ThreadFactory& thread_factory, + Filesystem::Instance& file_system, std::unique_ptr process_context) + : init_manager_(init_manager), workers_started_(false), shutdown_(false), options_(options), validation_context_(options_.allowUnknownStaticFields(), !options.rejectUnknownDynamicFields()), time_source_(time_system), restarter_(restarter), start_time_(time(nullptr)), diff --git a/source/server/server.h b/source/server/server.h index 2ed83f1df2eb..1993e2276f01 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -144,14 +144,14 @@ class RunHelper : Logger::Loggable { /** * This is the actual full standalone server which stitches together various common components. */ -class InstanceImpl : Logger::Loggable, - public Instance, - public ServerLifecycleNotifier { +class InstanceImpl final : Logger::Loggable, + public Instance, + public ServerLifecycleNotifier { public: /** * @throw EnvoyException if initialization fails. */ - InstanceImpl(const Options& options, Event::TimeSystem& time_system, + InstanceImpl(Init::Manager& init_manager, const Options& options, Event::TimeSystem& time_system, Network::Address::InstanceConstSharedPtr local_address, ListenerHooks& hooks, HotRestart& restarter, Stats::StoreRoot& store, Thread::BasicLockable& access_log_lock, ComponentFactory& component_factory, @@ -227,7 +227,7 @@ class InstanceImpl : Logger::Loggable, // init_manager_ must come before any member that participates in initialization, and destructed // only after referencing members are gone, since initialization continuation can potentially // occur at any point during member lifetime. This init manager is populated with LdsApi targets. - Init::ManagerImpl init_manager_{"Server"}; + Init::Manager& init_manager_; // secret_manager_ must come before listener_manager_, config_ and dispatcher_, and destructed // only after these members can no longer reference it, since: // - There may be active filter chains referencing it in listener_manager_. diff --git a/test/integration/server.cc b/test/integration/server.cc index 83777e19db1e..052b8a94f707 100644 --- a/test/integration/server.cc +++ b/test/integration/server.cc @@ -188,6 +188,7 @@ void IntegrationTestServerImpl::createAndRunEnvoyServer( Runtime::RandomGeneratorPtr&& random_generator, absl::optional> process_object) { { + Init::ManagerImpl init_manager{"Server"}; Stats::SymbolTablePtr symbol_table = Stats::SymbolTableCreator::makeSymbolTable(); Server::HotRestartNopImpl restarter; ThreadLocal::InstanceImpl tls; @@ -197,9 +198,9 @@ void IntegrationTestServerImpl::createAndRunEnvoyServer( if (process_object.has_value()) { process_context = std::make_unique(process_object->get()); } - Server::InstanceImpl server(options, time_system, local_address, hooks, restarter, stat_store, - access_log_lock, component_factory, std::move(random_generator), - tls, Thread::threadFactoryForTest(), + Server::InstanceImpl server(init_manager, options, time_system, local_address, hooks, restarter, + stat_store, access_log_lock, component_factory, + std::move(random_generator), tls, Thread::threadFactoryForTest(), Filesystem::fileSystemForTest(), std::move(process_context)); // This is technically thread unsafe (assigning to a shared_ptr accessed // across threads), but because we synchronize below through serverReady(), the only diff --git a/test/server/server_fuzz_test.cc b/test/server/server_fuzz_test.cc index b47f1aed9622..df47293d7cea 100644 --- a/test/server/server_fuzz_test.cc +++ b/test/server/server_fuzz_test.cc @@ -72,6 +72,7 @@ DEFINE_PROTO_FUZZER(const envoy::config::bootstrap::v2::Bootstrap& input) { ThreadLocal::InstanceImpl thread_local_instance; DangerousDeprecatedTestTime test_time; Fuzz::PerTestEnvironment test_env; + Init::ManagerImpl init_manager{"Server"}; { const std::string bootstrap_path = test_env.temporaryPath("bootstrap.pb_text"); @@ -84,7 +85,7 @@ DEFINE_PROTO_FUZZER(const envoy::config::bootstrap::v2::Bootstrap& input) { std::unique_ptr server; try { server = std::make_unique( - options, test_time.timeSystem(), + init_manager, options, test_time.timeSystem(), std::make_shared("127.0.0.1"), hooks, restart, stats_store, fakelock, component_factory, std::make_unique(), thread_local_instance, Thread::threadFactoryForTest(), Filesystem::fileSystemForTest(), diff --git a/test/server/server_test.cc b/test/server/server_test.cc index b5688754eaa8..1abb1ba977b8 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -141,27 +141,6 @@ class InitializingInitManager : public Init::ManagerImpl { State state() const override { return State::Initializing; } }; -class InitializingInstanceImpl : public InstanceImpl { -private: - InitializingInitManager init_manager_{"Server"}; - -public: - InitializingInstanceImpl(const Options& options, Event::TimeSystem& time_system, - Network::Address::InstanceConstSharedPtr local_address, - ListenerHooks& hooks, HotRestart& restarter, Stats::StoreRoot& store, - Thread::BasicLockable& access_log_lock, - ComponentFactory& component_factory, - Runtime::RandomGeneratorPtr&& random_generator, - ThreadLocal::Instance& tls, Thread::ThreadFactory& thread_factory, - Filesystem::Instance& file_system, - std::unique_ptr process_context) - : InstanceImpl(options, time_system, local_address, hooks, restarter, store, access_log_lock, - component_factory, std::move(random_generator), tls, thread_factory, - file_system, std::move(process_context)) {} - - Init::Manager& initManager() override { return init_manager_; } -}; - // Class creates minimally viable server instance for testing. class ServerInstanceImplTestBase { protected: @@ -176,25 +155,16 @@ class ServerInstanceImplTestBase { if (process_object_ != nullptr) { process_context_ = std::make_unique(*process_object_); } - if (use_intializing_instance) { - server_ = std::make_unique( - options_, test_time_.timeSystem(), - Network::Address::InstanceConstSharedPtr(new Network::Address::Ipv4Instance("127.0.0.1")), - hooks_, restart_, stats_store_, fakelock_, component_factory_, - std::make_unique>(), *thread_local_, - Thread::threadFactoryForTest(), Filesystem::fileSystemForTest(), - std::move(process_context_)); - - } else { - server_ = std::make_unique( - options_, test_time_.timeSystem(), - Network::Address::InstanceConstSharedPtr(new Network::Address::Ipv4Instance("127.0.0.1")), - hooks_, restart_, stats_store_, fakelock_, component_factory_, - std::make_unique>(), *thread_local_, - Thread::threadFactoryForTest(), Filesystem::fileSystemForTest(), - std::move(process_context_)); - } + init_manager_ = use_intializing_instance ? std::make_unique("Server") + : std::make_unique("Server"); + server_ = std::make_unique( + *init_manager_, options_, test_time_.timeSystem(), + Network::Address::InstanceConstSharedPtr(new Network::Address::Ipv4Instance("127.0.0.1")), + hooks_, restart_, stats_store_, fakelock_, component_factory_, + std::make_unique>(), *thread_local_, + Thread::threadFactoryForTest(), Filesystem::fileSystemForTest(), + std::move(process_context_)); EXPECT_TRUE(server_->api().fileSystem().fileExists("/dev/null")); } @@ -206,8 +176,9 @@ class ServerInstanceImplTestBase { {"health_check_interval", fmt::format("{}", interval).c_str()}}, TestEnvironment::PortMap{}, version_); thread_local_ = std::make_unique(); + init_manager_ = std::make_unique("Server"); server_ = std::make_unique( - options_, test_time_.timeSystem(), + *init_manager_, options_, test_time_.timeSystem(), Network::Address::InstanceConstSharedPtr(new Network::Address::Ipv4Instance("127.0.0.1")), hooks_, restart_, stats_store_, fakelock_, component_factory_, std::make_unique>(), *thread_local_, @@ -256,6 +227,8 @@ class ServerInstanceImplTestBase { DangerousDeprecatedTestTime test_time_; ProcessObject* process_object_ = nullptr; std::unique_ptr process_context_; + std::unique_ptr init_manager_; + std::unique_ptr server_; }; @@ -781,9 +754,10 @@ TEST_P(ServerInstanceImplTest, LogToFileError) { // an empty config. TEST_P(ServerInstanceImplTest, NoOptionsPassed) { thread_local_ = std::make_unique(); + init_manager_ = std::make_unique("Server"); EXPECT_THROW_WITH_MESSAGE( server_.reset(new InstanceImpl( - options_, test_time_.timeSystem(), + *init_manager_, options_, test_time_.timeSystem(), Network::Address::InstanceConstSharedPtr(new Network::Address::Ipv4Instance("127.0.0.1")), hooks_, restart_, stats_store_, fakelock_, component_factory_, std::make_unique>(), *thread_local_,