diff --git a/mobile/library/common/BUILD b/mobile/library/common/BUILD index 1990b609dc0d..bca2df8d0a2f 100644 --- a/mobile/library/common/BUILD +++ b/mobile/library/common/BUILD @@ -25,6 +25,7 @@ envoy_cc_library( deps = [ ":engine_common_lib", ":engine_types_lib", + ":mobile_process_wide_lib", "//library/common/bridge:utility_lib", "//library/common/event:provisional_dispatcher_lib", "//library/common/http:client_lib", @@ -91,3 +92,19 @@ envoy_cc_library( "@envoy//source/common/http:header_map_lib", ], ) + +envoy_cc_library( + name = "mobile_process_wide_lib", + srcs = [ + "mobile_process_wide.cc", + ], + hdrs = [ + "mobile_process_wide.h", + ], + repository = "@envoy", + deps = [ + "@envoy//source/common/common:minimal_logger_lib", + "@envoy//source/common/common:thread_lib", + "@envoy//source/server:options_base", + ], +) diff --git a/mobile/library/common/engine_common.cc b/mobile/library/common/engine_common.cc index 0f0746c431f7..d882c3c5944c 100644 --- a/mobile/library/common/engine_common.cc +++ b/mobile/library/common/engine_common.cc @@ -98,12 +98,11 @@ EngineCommon::EngineCommon(std::shared_ptr options) : op server->initialize(local_address, component_factory); return server; }; - // `set_new_handler` is false because the application using Envoy Mobile should decide how to - // handle `new` memory allocation failures. - base_ = std::make_unique( - *options_, real_time_system_, default_listener_hooks_, prod_component_factory_, - std::make_unique(), std::make_unique(), nullptr, - create_instance, /*set_new_handler=*/false); + auto random_generator = std::make_unique(); + base_ = std::make_unique(*options_, prod_component_factory_, + std::make_unique(), *random_generator); + base_->init(real_time_system_, default_listener_hooks_, std::move(random_generator), nullptr, + create_instance); // Disabling signal handling in the options makes it so that the server's event dispatcher _does // not_ listen for termination signals such as SIGTERM, SIGINT, etc // (https://github.com/envoyproxy/envoy/blob/048f4231310fbbead0cbe03d43ffb4307fff0517/source/server/server.cc#L519). diff --git a/mobile/library/common/internal_engine.cc b/mobile/library/common/internal_engine.cc index 53e9f673317e..24b547efe1d0 100644 --- a/mobile/library/common/internal_engine.cc +++ b/mobile/library/common/internal_engine.cc @@ -10,6 +10,7 @@ #include "source/common/runtime/runtime_features.h" #include "absl/synchronization/notification.h" +#include "library/common/mobile_process_wide.h" #include "library/common/network/proxy_api.h" #include "library/common/stats/utility.h" @@ -19,6 +20,11 @@ constexpr absl::Duration ENGINE_RUNNING_TIMEOUT = absl::Seconds(30); // Google DNS address used for IPv6 probes. constexpr absl::string_view IPV6_PROBE_ADDRESS = "2001:4860:4860::8888"; constexpr uint32_t IPV6_PROBE_PORT = 53; + +// There is only one shared MobileProcessWide instance for all Envoy Mobile engines. +MobileProcessWide& initOnceMobileProcessWide(const OptionsImplBase& options) { + MUTABLE_CONSTRUCT_ON_FIRST_USE(MobileProcessWide, options); +} } // namespace static std::atomic current_stream_handle_{0}; @@ -90,7 +96,8 @@ envoy_status_t InternalEngine::cancelStream(envoy_stream_t stream) { // This function takes a `std::shared_ptr` instead of `std::unique_ptr` because `std::function` is a // copy-constructible type, so it's not possible to move capture `std::unique_ptr` with // `std::function`. -envoy_status_t InternalEngine::run(std::shared_ptr options) { +envoy_status_t InternalEngine::run(std::shared_ptr options) { + initOnceMobileProcessWide(*options); Thread::Options thread_options; thread_options.priority_ = thread_priority_; main_thread_ = thread_factory_->createThread([this, options]() mutable -> void { main(options); }, @@ -98,7 +105,7 @@ envoy_status_t InternalEngine::run(std::shared_ptr optio return (main_thread_ != nullptr) ? ENVOY_SUCCESS : ENVOY_FAILURE; } -envoy_status_t InternalEngine::main(std::shared_ptr options) { +envoy_status_t InternalEngine::main(std::shared_ptr options) { // Using unique_ptr ensures main_common's lifespan is strictly scoped to this function. std::unique_ptr main_common; { diff --git a/mobile/library/common/internal_engine.h b/mobile/library/common/internal_engine.h index eec47a4916d9..1382ec672bf2 100644 --- a/mobile/library/common/internal_engine.h +++ b/mobile/library/common/internal_engine.h @@ -41,7 +41,7 @@ class InternalEngine : public Logger::Loggable { * Run the engine with the provided options. * @param options, the Envoy options, including the Bootstrap configuration and log level. */ - envoy_status_t run(std::shared_ptr options); + envoy_status_t run(std::shared_ptr options); /** * Immediately terminate the engine, if running. Calling this function when @@ -170,7 +170,7 @@ class InternalEngine : public Logger::Loggable { std::unique_ptr event_tracker, absl::optional thread_priority, Thread::PosixThreadFactoryPtr thread_factory); - envoy_status_t main(std::shared_ptr options); + envoy_status_t main(std::shared_ptr options); static void logInterfaces(absl::string_view event, std::vector& interfaces); /** Returns true if there is IPv6 connectivity. */ diff --git a/mobile/library/common/mobile_process_wide.cc b/mobile/library/common/mobile_process_wide.cc new file mode 100644 index 000000000000..f0d3c4ceaf95 --- /dev/null +++ b/mobile/library/common/mobile_process_wide.cc @@ -0,0 +1,13 @@ +#include "library/common/mobile_process_wide.h" + +namespace Envoy { + +MobileProcessWide::MobileProcessWide(const OptionsImplBase& options) { + logging_context_ = std::make_unique(options.logLevel(), options.logFormat(), + log_lock_, options.logFormatEscaped(), + options.enableFineGrainLogging()); +} + +MobileProcessWide::~MobileProcessWide() { logging_context_.reset(nullptr); } + +} // namespace Envoy diff --git a/mobile/library/common/mobile_process_wide.h b/mobile/library/common/mobile_process_wide.h new file mode 100644 index 000000000000..94b954d693fe --- /dev/null +++ b/mobile/library/common/mobile_process_wide.h @@ -0,0 +1,21 @@ +#pragma once + +#include "source/common/common/logger.h" +#include "source/common/common/thread.h" +#include "source/server/options_impl_base.h" + +namespace Envoy { + +// Process-wide lifecycle events for global state for Envoy Mobile. There should only ever be a +// singleton of this class. +class MobileProcessWide { +public: + explicit MobileProcessWide(const OptionsImplBase& options); + ~MobileProcessWide(); + +private: + Thread::MutexBasicLockable log_lock_; + std::unique_ptr logging_context_; +}; + +} // namespace Envoy diff --git a/mobile/test/common/integration/client_integration_test.cc b/mobile/test/common/integration/client_integration_test.cc index 433ba7f8062b..c103cb75409c 100644 --- a/mobile/test/common/integration/client_integration_test.cc +++ b/mobile/test/common/integration/client_integration_test.cc @@ -78,6 +78,7 @@ class ClientIntegrationTest } void initialize() override { + builder_.setLogLevel(Logger::Logger::debug); builder_.addRuntimeGuard("dns_cache_set_ip_version_to_remove", true); builder_.addRuntimeGuard("quic_no_tcp_delay", true); diff --git a/source/exe/main_common.cc b/source/exe/main_common.cc index 87b3342f3335..75099c6ee0c0 100644 --- a/source/exe/main_common.cc +++ b/source/exe/main_common.cc @@ -55,14 +55,23 @@ MainCommonBase::MainCommonBase(const Server::Options& options, Event::TimeSystem std::unique_ptr platform_impl, std::unique_ptr&& random_generator, std::unique_ptr process_context) - : StrippedMainBase(options, time_system, listener_hooks, component_factory, - std::move(platform_impl), std::move(random_generator), - std::move(process_context), createFunction()) + : StrippedMainBase(options, component_factory, std::move(platform_impl), *random_generator) #ifdef ENVOY_ADMIN_FUNCTIONALITY , shared_response_set_(std::make_shared()) #endif { + if (options.mode() == Server::Mode::Serve) { + // Provide consistent behavior for out-of-memory, regardless of whether it occurs in a + // try/catch block or not. + std::set_new_handler([]() { PANIC("out of memory"); }); + } + + logging_context_ = std::make_unique( + options_.logLevel(), options_.logFormat(), restarter_->logLock(), options_.logFormatEscaped(), + options_.mode() == Server::Mode::Validate ? false : options_.enableFineGrainLogging()); + init(time_system, listener_hooks, std::move(random_generator), std::move(process_context), + createFunction()); } bool MainCommonBase::run() { diff --git a/source/exe/stripped_main_base.cc b/source/exe/stripped_main_base.cc index 6d6321f89049..fac588bf4a78 100644 --- a/source/exe/stripped_main_base.cc +++ b/source/exe/stripped_main_base.cc @@ -41,18 +41,15 @@ Runtime::LoaderPtr ProdComponentFactory::createRuntime(Server::Instance& server, return Server::InstanceUtil::createRuntime(server, config); } -StrippedMainBase::StrippedMainBase(const Server::Options& options, Event::TimeSystem& time_system, - ListenerHooks& listener_hooks, +StrippedMainBase::StrippedMainBase(const Server::Options& options, Server::ComponentFactory& component_factory, std::unique_ptr platform_impl, - std::unique_ptr&& random_generator, - std::unique_ptr process_context, - CreateInstanceFunction create_instance, bool set_new_handler) + Random::RandomGenerator& random_generator) : platform_impl_(std::move(platform_impl)), options_(options), component_factory_(component_factory), stats_allocator_(symbol_table_) { // Process the option to disable extensions as early as possible, // before we do any configuration loading. - OptionsImplBase::disableExtensions(options.disabledExtensions()); + OptionsImplBase::disableExtensions(options_.disabledExtensions()); // Enable core dumps as early as possible. if (options_.coreDumpEnabled()) { @@ -66,37 +63,33 @@ StrippedMainBase::StrippedMainBase(const Server::Options& options, Event::TimeSy switch (options_.mode()) { case Server::Mode::InitOnly: - case Server::Mode::Serve: { - configureHotRestarter(*random_generator); - + case Server::Mode::Serve: + configureHotRestarter(random_generator); tls_ = std::make_unique(); - Thread::BasicLockable& log_lock = restarter_->logLock(); - Thread::BasicLockable& access_log_lock = restarter_->accessLogLock(); - logging_context_ = std::make_unique(options_.logLevel(), options_.logFormat(), - log_lock, options_.logFormatEscaped(), - options_.enableFineGrainLogging()); + stats_store_ = std::make_unique(stats_allocator_); + break; + case Server::Mode::Validate: + restarter_ = std::make_unique(); + break; + } +} +void StrippedMainBase::init(Event::TimeSystem& time_system, ListenerHooks& listener_hooks, + std::unique_ptr&& random_generator, + std::unique_ptr process_context, + CreateInstanceFunction create_instance) { + switch (options_.mode()) { + case Server::Mode::InitOnly: + case Server::Mode::Serve: { configureComponentLogLevels(); - if (set_new_handler) { - // Provide consistent behavior for out-of-memory, regardless of whether it occurs in a - // try/catch block or not. - std::set_new_handler([]() { PANIC("out of memory"); }); - } - - stats_store_ = std::make_unique(stats_allocator_); - server_ = create_instance(*init_manager_, options_, time_system, listener_hooks, *restarter_, - *stats_store_, access_log_lock, component_factory, + *stats_store_, restarter_->accessLogLock(), component_factory_, std::move(random_generator), *tls_, platform_impl_->threadFactory(), platform_impl_->fileSystem(), std::move(process_context), nullptr); break; } case Server::Mode::Validate: - restarter_ = std::make_unique(); - logging_context_ = - std::make_unique(options_.logLevel(), options_.logFormat(), - restarter_->logLock(), options_.logFormatEscaped()); process_context_ = std::move(process_context); break; } diff --git a/source/exe/stripped_main_base.h b/source/exe/stripped_main_base.h index eb6a632b4ad4..2a067e88a6f5 100644 --- a/source/exe/stripped_main_base.h +++ b/source/exe/stripped_main_base.h @@ -47,13 +47,18 @@ class StrippedMainBase { static std::string hotRestartVersion(bool hot_restart_enabled); // Consumer must guarantee that all passed references are alive until this object is - // destructed. - StrippedMainBase(const Server::Options& options, Event::TimeSystem& time_system, - ListenerHooks& listener_hooks, Server::ComponentFactory& component_factory, + // destructed, except for `random_generator` which only needs to be alive until the constructor + // completes execution. + StrippedMainBase(const Server::Options& options, Server::ComponentFactory& component_factory, std::unique_ptr platform_impl, - std::unique_ptr&& random_generator, - std::unique_ptr process_context, - CreateInstanceFunction create_instance, bool set_new_handler = true); + Random::RandomGenerator& random_generator); + + // Initialize the Envoy server instance. Must be called prior to any other method to use the + // server. Separated from the constructor to allow for globals to be initialized first. + void init(Event::TimeSystem& time_system, ListenerHooks& listener_hooks, + std::unique_ptr&& random_generator, + std::unique_ptr process_context, + CreateInstanceFunction create_instance); void runServer() { ASSERT(options_.mode() == Server::Mode::Serve); @@ -78,6 +83,10 @@ class StrippedMainBase { ThreadLocal::InstanceImplPtr tls_; std::unique_ptr restarter_; Stats::ThreadLocalStoreImplPtr stats_store_; + // logging_context_ is only used in (some) subclasses, but it must be declared here so that it's + // destructed after the server_. This is necessary because the Server and its threads may log + // during destruction, causing data race issues with the Context's destruction and activation of + // the saved context. std::unique_ptr logging_context_; std::unique_ptr init_manager_{std::make_unique("Server")}; std::unique_ptr server_;