diff --git a/.github/workflows/mobile-perf.yml b/.github/workflows/mobile-perf.yml index 8fc3ce3af503..2e92469c9640 100644 --- a/.github/workflows/mobile-perf.yml +++ b/.github/workflows/mobile-perf.yml @@ -30,7 +30,7 @@ jobs: steps: - uses: actions/checkout@v4 - name: Ensure files don't leak back into the main binary - run: rm source/extensions/listener_managers/listener_manager/listener_manager_impl.h source/server/overload_manager_impl.cc source/common/network/listen_socket_impl.h source/common/network/tcp_listener_impl.h + run: rm source/extensions/listener_managers/listener_manager/listener_manager_impl.h source/server/overload_manager_impl.cc source/common/network/listen_socket_impl.h source/common/network/tcp_listener_impl.h source/server/guarddog_impl.h source/server/watchdog_impl.h - name: Add safe directory run: git config --global --add safe.directory /__w/envoy/envoy - name: 'Build test binary' diff --git a/envoy/server/listener_manager.h b/envoy/server/listener_manager.h index f3cfadc5a468..db6cefe41232 100644 --- a/envoy/server/listener_manager.h +++ b/envoy/server/listener_manager.h @@ -221,10 +221,10 @@ class ListenerManager { /** * Start all workers accepting new connections on all added listeners. - * @param guard_dog supplies the guard dog to use for thread watching. + * @param guard_dog supplies the optional guard dog to use for thread watching. * @param callback supplies the callback to complete server initialization. */ - virtual void startWorkers(GuardDog& guard_dog, std::function callback) PURE; + virtual void startWorkers(OptRef guard_dog, std::function callback) PURE; /** * Stop all listeners from accepting new connections without actually removing any of them. This diff --git a/envoy/server/worker.h b/envoy/server/worker.h index aa782c0c43ea..74e24d01f017 100644 --- a/envoy/server/worker.h +++ b/envoy/server/worker.h @@ -45,10 +45,10 @@ class Worker { /** * Start the worker thread. - * @param guard_dog supplies the guard dog to use for thread watching. + * @param guard_dog supplies the optional guard dog to use for thread watching. * @param cb a callback to run when the worker thread starts running. */ - virtual void start(GuardDog& guard_dog, const std::function& cb) PURE; + virtual void start(OptRef guard_dog, const std::function& cb) PURE; /** * Initialize stats for this worker's dispatcher, if available. The worker will output diff --git a/mobile/library/common/engine_common.cc b/mobile/library/common/engine_common.cc index ef89ca070b11..53a90c7cadc2 100644 --- a/mobile/library/common/engine_common.cc +++ b/mobile/library/common/engine_common.cc @@ -62,6 +62,9 @@ class ServerLite : public Server::InstanceBase { std::unique_ptr createOverloadManager() override { return std::make_unique(threadLocal(), true); } + std::unique_ptr maybeCreateGuardDog(absl::string_view) override { + return nullptr; + } }; EngineCommon::EngineCommon(std::unique_ptr&& options) diff --git a/mobile/library/common/extensions/listener_managers/api_listener_manager/api_listener_manager.h b/mobile/library/common/extensions/listener_managers/api_listener_manager/api_listener_manager.h index 0f152df9e502..493beb8dec44 100644 --- a/mobile/library/common/extensions/listener_managers/api_listener_manager/api_listener_manager.h +++ b/mobile/library/common/extensions/listener_managers/api_listener_manager/api_listener_manager.h @@ -29,7 +29,7 @@ class ApiListenerManagerImpl : public ListenerManager, Logger::Loggable callback) override { callback(); } + void startWorkers(OptRef, std::function callback) override { callback(); } void stopListeners(StopListenersType, const Network::ExtraShutdownListenerOptions&) override {} void stopWorkers() override {} void beginListenerUpdate() override {} diff --git a/source/extensions/listener_managers/listener_manager/listener_manager_impl.cc b/source/extensions/listener_managers/listener_manager/listener_manager_impl.cc index 8b1c0485ca7e..b6e6caccad7e 100644 --- a/source/extensions/listener_managers/listener_manager/listener_manager_impl.cc +++ b/source/extensions/listener_managers/listener_manager/listener_manager_impl.cc @@ -919,7 +919,7 @@ bool ListenerManagerImpl::removeListenerInternal(const std::string& name, return true; } -void ListenerManagerImpl::startWorkers(GuardDog& guard_dog, std::function callback) { +void ListenerManagerImpl::startWorkers(OptRef guard_dog, std::function callback) { ENVOY_LOG(info, "all dependencies initialized. starting workers"); ASSERT(!workers_started_); workers_started_ = true; diff --git a/source/extensions/listener_managers/listener_manager/listener_manager_impl.h b/source/extensions/listener_managers/listener_manager/listener_manager_impl.h index 56bd78050836..172f832ff775 100644 --- a/source/extensions/listener_managers/listener_manager/listener_manager_impl.h +++ b/source/extensions/listener_managers/listener_manager/listener_manager_impl.h @@ -222,7 +222,7 @@ class ListenerManagerImpl : public ListenerManager, Logger::Loggable callback) override; + void startWorkers(OptRef guard_dog, std::function callback) override; void stopListeners(StopListenersType stop_listeners_type, const Network::ExtraShutdownListenerOptions& options) override; void stopWorkers() override; diff --git a/source/server/BUILD b/source/server/BUILD index 95a4b4b65d6a..a3c9717e223a 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -395,7 +395,6 @@ envoy_cc_library( deps = [ ":api_listener_lib", ":configuration_lib", - ":guarddog_lib", ":listener_hooks_lib", ":listener_manager_factory_lib", ":regex_engine_lib", @@ -457,6 +456,7 @@ envoy_cc_library( srcs = ["instance_impl.cc"], hdrs = ["instance_impl.h"], deps = [ + ":guarddog_lib", ":server_base_lib", "//source/common/memory:heap_shrinker_lib", "//source/server:overload_manager_lib", diff --git a/source/server/instance_impl.cc b/source/server/instance_impl.cc index 2d66b785edc4..82e1817f6f6c 100644 --- a/source/server/instance_impl.cc +++ b/source/server/instance_impl.cc @@ -1,5 +1,6 @@ #include "source/server/instance_impl.h" +#include "source/server/guarddog_impl.h" #include "source/server/overload_manager_impl.h" namespace Envoy { @@ -15,5 +16,10 @@ std::unique_ptr InstanceImpl::createOverloadManager() { messageValidationContext().staticValidationVisitor(), api(), options()); } +std::unique_ptr InstanceImpl::maybeCreateGuardDog(absl::string_view name) { + return std::make_unique(*stats().rootScope(), + config().mainThreadWatchdogConfig(), api(), name); +} + } // namespace Server } // namespace Envoy diff --git a/source/server/instance_impl.h b/source/server/instance_impl.h index 04cd4fdd2331..c21d8b17ef2b 100644 --- a/source/server/instance_impl.h +++ b/source/server/instance_impl.h @@ -14,6 +14,7 @@ class InstanceImpl : public InstanceBase { protected: void maybeCreateHeapShrinker() override; std::unique_ptr createOverloadManager() override; + std::unique_ptr maybeCreateGuardDog(absl::string_view name) override; private: std::unique_ptr heap_shrinker_; diff --git a/source/server/server.cc b/source/server/server.cc index 257bce927557..cb3f3c9df87b 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -51,7 +51,6 @@ #include "source/common/upstream/cluster_manager_impl.h" #include "source/common/version/version.h" #include "source/server/configuration_impl.h" -#include "source/server/guarddog_impl.h" #include "source/server/listener_hooks.h" #include "source/server/listener_manager_factory.h" #include "source/server/regex_engine.h" @@ -778,10 +777,8 @@ void InstanceBase::initializeOrThrow(Network::Address::InstanceConstSharedPtr lo // GuardDog (deadlock detection) object and thread setup before workers are // started and before our own run() loop runs. - main_thread_guard_dog_ = std::make_unique( - *stats_store_.rootScope(), config_.mainThreadWatchdogConfig(), *api_, "main_thread"); - worker_guard_dog_ = std::make_unique( - *stats_store_.rootScope(), config_.workerWatchdogConfig(), *api_, "workers"); + main_thread_guard_dog_ = maybeCreateGuardDog("main_thread"); + worker_guard_dog_ = maybeCreateGuardDog("workers"); } void InstanceBase::onClusterManagerPrimaryInitializationComplete() { @@ -823,7 +820,7 @@ void InstanceBase::onRuntimeReady() { void InstanceBase::startWorkers() { // The callback will be called after workers are started. - listener_manager_->startWorkers(*worker_guard_dog_, [this]() { + listener_manager_->startWorkers(makeOptRefFromPtr(worker_guard_dog_.get()), [this]() { if (isShutdown()) { return; } @@ -950,12 +947,17 @@ void InstanceBase::run() { // Run the main dispatch loop waiting to exit. ENVOY_LOG(info, "starting main dispatch loop"); - auto watchdog = main_thread_guard_dog_->createWatchDog(api_->threadFactory().currentThreadId(), - "main_thread", *dispatcher_); + WatchDogSharedPtr watchdog; + if (main_thread_guard_dog_) { + watchdog = main_thread_guard_dog_->createWatchDog(api_->threadFactory().currentThreadId(), + "main_thread", *dispatcher_); + } dispatcher_->post([this] { notifyCallbacksForStage(Stage::Startup); }); dispatcher_->run(Event::Dispatcher::RunType::Block); ENVOY_LOG(info, "main dispatch loop exited"); - main_thread_guard_dog_->stopWatching(watchdog); + if (main_thread_guard_dog_) { + main_thread_guard_dog_->stopWatching(watchdog); + } watchdog.reset(); terminate(); diff --git a/source/server/server.h b/source/server/server.h index efec23b440a6..369693d75b41 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -249,6 +249,7 @@ class InstanceBase : Logger::Loggable, virtual void maybeCreateHeapShrinker() PURE; virtual std::unique_ptr createOverloadManager() PURE; + virtual std::unique_ptr maybeCreateGuardDog(absl::string_view name) PURE; void run() override; @@ -314,6 +315,9 @@ class InstanceBase : Logger::Loggable, ServerLifecycleNotifier::HandlePtr registerCallback(Stage stage, StageCallbackWithCompletion callback) override; +protected: + const Configuration::MainImpl& config() { return config_; } + private: Network::DnsResolverSharedPtr getOrCreateDnsResolver(); diff --git a/source/server/worker_impl.cc b/source/server/worker_impl.cc index 504e7f963e82..dcdd526b27e0 100644 --- a/source/server/worker_impl.cc +++ b/source/server/worker_impl.cc @@ -98,7 +98,7 @@ void WorkerImpl::removeFilterChains(uint64_t listener_tag, }); } -void WorkerImpl::start(GuardDog& guard_dog, const std::function& cb) { +void WorkerImpl::start(OptRef guard_dog, const std::function& cb) { ASSERT(!thread_); // In posix, thread names are limited to 15 characters, so contrive to make @@ -113,7 +113,7 @@ void WorkerImpl::start(GuardDog& guard_dog, const std::function& cb) { // architecture is centralized, resulting in clearer names. Thread::Options options{absl::StrCat("wrk:", dispatcher_->name())}; thread_ = api_.threadFactory().createThread( - [this, &guard_dog, cb]() -> void { threadRoutine(guard_dog, cb); }, options); + [this, guard_dog, cb]() -> void { threadRoutine(guard_dog, cb); }, options); } void WorkerImpl::initializeStats(Stats::Scope& scope) { dispatcher_->initializeStats(scope); } @@ -139,18 +139,22 @@ void WorkerImpl::stopListener(Network::ListenerConfig& listener, }); } -void WorkerImpl::threadRoutine(GuardDog& guard_dog, const std::function& cb) { +void WorkerImpl::threadRoutine(OptRef guard_dog, const std::function& cb) { ENVOY_LOG(debug, "worker entering dispatch loop"); // The watch dog must be created after the dispatcher starts running and has post events flushed, // as this is when TLS stat scopes start working. dispatcher_->post([this, &guard_dog, cb]() { cb(); - watch_dog_ = guard_dog.createWatchDog(api_.threadFactory().currentThreadId(), - dispatcher_->name(), *dispatcher_); + if (guard_dog.has_value()) { + watch_dog_ = guard_dog->createWatchDog(api_.threadFactory().currentThreadId(), + dispatcher_->name(), *dispatcher_); + } }); dispatcher_->run(Event::Dispatcher::RunType::Block); ENVOY_LOG(debug, "worker exited dispatch loop"); - guard_dog.stopWatching(watch_dog_); + if (guard_dog.has_value()) { + guard_dog->stopWatching(watch_dog_); + } dispatcher_->shutdown(); // We must close all active connections before we actually exit the thread. This prevents any diff --git a/source/server/worker_impl.h b/source/server/worker_impl.h index 0fb45a0918a7..a1baa6ec5ff4 100644 --- a/source/server/worker_impl.h +++ b/source/server/worker_impl.h @@ -61,7 +61,7 @@ class WorkerImpl : public Worker, Logger::Loggable { void removeFilterChains(uint64_t listener_tag, const std::list& filter_chains, std::function completion) override; - void start(GuardDog& guard_dog, const std::function& cb) override; + void start(OptRef guard_dog, const std::function& cb) override; void initializeStats(Stats::Scope& scope) override; void stop() override; void stopListener(Network::ListenerConfig& listener, @@ -69,7 +69,7 @@ class WorkerImpl : public Worker, Logger::Loggable { std::function completion) override; private: - void threadRoutine(GuardDog& guard_dog, const std::function& cb); + void threadRoutine(OptRef guard_dog, const std::function& cb); void stopAcceptingConnectionsCb(OverloadActionState state); void rejectIncomingConnectionsCb(OverloadActionState state); void resetStreamsUsingExcessiveMemory(OverloadActionState state); diff --git a/test/mocks/server/listener_manager.h b/test/mocks/server/listener_manager.h index 7285ff78eaa1..2d35d1a1df5d 100644 --- a/test/mocks/server/listener_manager.h +++ b/test/mocks/server/listener_manager.h @@ -21,7 +21,7 @@ class MockListenerManager : public ListenerManager { (ListenerState state)); MOCK_METHOD(uint64_t, numConnections, (), (const)); MOCK_METHOD(bool, removeListener, (const std::string& listener_name)); - MOCK_METHOD(void, startWorkers, (GuardDog & guard_dog, std::function callback)); + MOCK_METHOD(void, startWorkers, (OptRef guard_dog, std::function callback)); MOCK_METHOD(void, stopListeners, (StopListenersType listeners_type, const Network::ExtraShutdownListenerOptions& options)); diff --git a/test/mocks/server/worker.cc b/test/mocks/server/worker.cc index 2d70e824d24f..2990be7ed5cd 100644 --- a/test/mocks/server/worker.cc +++ b/test/mocks/server/worker.cc @@ -47,7 +47,8 @@ MockWorker::MockWorker() { })); ON_CALL(*this, start(_, _)) - .WillByDefault(Invoke([](GuardDog&, const std::function& cb) -> void { cb(); })); + .WillByDefault( + Invoke([](OptRef, const std::function& cb) -> void { cb(); })); } MockWorker::~MockWorker() = default; diff --git a/test/mocks/server/worker.h b/test/mocks/server/worker.h index d2ccd9fb1ee9..a52da20a81d5 100644 --- a/test/mocks/server/worker.h +++ b/test/mocks/server/worker.h @@ -37,7 +37,7 @@ class MockWorker : public Worker { MOCK_METHOD(uint64_t, numConnections, (), (const)); MOCK_METHOD(void, removeListener, (Network::ListenerConfig & listener, std::function completion)); - MOCK_METHOD(void, start, (GuardDog & guard_dog, const std::function& cb)); + MOCK_METHOD(void, start, (OptRef guard_dog, const std::function& cb)); MOCK_METHOD(void, initializeStats, (Stats::Scope & scope)); MOCK_METHOD(void, stop, ()); MOCK_METHOD(void, stopListener,