diff --git a/include/envoy/thread/thread.h b/include/envoy/thread/thread.h index f7197b34635e6..92f6063fafdb2 100644 --- a/include/envoy/thread/thread.h +++ b/include/envoy/thread/thread.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include "envoy/common/pure.h" @@ -21,6 +22,20 @@ class Thread { typedef std::unique_ptr ThreadPtr; +/** + * Interface providing a mechanism for creating threads. + */ +class ThreadFactory { +public: + virtual ~ThreadFactory() {} + + /** + * Create a thread. + * @param thread_routine supplies the function to invoke in the thread. + */ + virtual ThreadPtr createThread(std::function thread_routine) PURE; +}; + /** * Like the C++11 "basic lockable concept" but a pure virtual interface vs. a template, and * with thread annotations. diff --git a/source/common/api/api_impl.cc b/source/common/api/api_impl.cc index 5a068a8dd450d..2c7708a10fccd 100644 --- a/source/common/api/api_impl.cc +++ b/source/common/api/api_impl.cc @@ -10,13 +10,14 @@ namespace Envoy { namespace Api { +Impl::Impl(std::chrono::milliseconds file_flush_interval_msec, + Thread::ThreadFactory& thread_factory) + : file_flush_interval_msec_(file_flush_interval_msec), thread_factory_(thread_factory) {} + Event::DispatcherPtr Impl::allocateDispatcher(Event::TimeSystem& time_system) { return Event::DispatcherPtr{new Event::DispatcherImpl(time_system)}; } -Impl::Impl(std::chrono::milliseconds file_flush_interval_msec) - : file_flush_interval_msec_(file_flush_interval_msec) {} - Filesystem::FileSharedPtr Impl::createFile(const std::string& path, Event::Dispatcher& dispatcher, Thread::BasicLockable& lock, Stats::Store& stats_store) { return std::make_shared(path, dispatcher, lock, stats_store, *this, @@ -28,7 +29,7 @@ bool Impl::fileExists(const std::string& path) { return Filesystem::fileExists(p std::string Impl::fileReadToEnd(const std::string& path) { return Filesystem::fileReadToEnd(path); } Thread::ThreadPtr Impl::createThread(std::function thread_routine) { - return std::make_unique(thread_routine); + return thread_factory_.createThread(thread_routine); } } // namespace Api diff --git a/source/common/api/api_impl.h b/source/common/api/api_impl.h index 34b1cf7c5bcd9..d2343d8e69007 100644 --- a/source/common/api/api_impl.h +++ b/source/common/api/api_impl.h @@ -6,6 +6,7 @@ #include "envoy/api/api.h" #include "envoy/event/timer.h" #include "envoy/filesystem/filesystem.h" +#include "envoy/thread/thread.h" namespace Envoy { namespace Api { @@ -15,7 +16,7 @@ namespace Api { */ class Impl : public Api::Api { public: - Impl(std::chrono::milliseconds file_flush_interval_msec = std::chrono::milliseconds(1000)); + Impl(std::chrono::milliseconds file_flush_interval_msec, Thread::ThreadFactory& thread_factory); // Api::Api Event::DispatcherPtr allocateDispatcher(Event::TimeSystem& time_system) override; @@ -28,6 +29,7 @@ class Impl : public Api::Api { private: std::chrono::milliseconds file_flush_interval_msec_; + Thread::ThreadFactory& thread_factory_; }; } // namespace Api diff --git a/source/common/common/thread.cc b/source/common/common/thread.cc index a2feafab411db..9f3ef5e9f6600 100644 --- a/source/common/common/thread.cc +++ b/source/common/common/thread.cc @@ -14,20 +14,35 @@ namespace Envoy { namespace Thread { -ThreadImpl::ThreadImpl(std::function thread_routine) : thread_routine_(thread_routine) { - RELEASE_ASSERT(Logger::Registry::initialized(), ""); - int rc = pthread_create(&thread_id_, nullptr, - [](void* arg) -> void* { - static_cast(arg)->thread_routine_(); - return nullptr; - }, - this); - RELEASE_ASSERT(rc == 0, ""); -} - -void ThreadImpl::join() { - int rc = pthread_join(thread_id_, nullptr); - RELEASE_ASSERT(rc == 0, ""); +/** + * Wrapper for a pthread thread. We don't use std::thread because it eats exceptions and leads to + * unusable stack traces. + */ +class ThreadImpl : public Thread { +public: + ThreadImpl(std::function thread_routine) : thread_routine_(thread_routine) { + RELEASE_ASSERT(Logger::Registry::initialized(), ""); + int rc = pthread_create(&thread_id_, nullptr, + [](void* arg) -> void* { + static_cast(arg)->thread_routine_(); + return nullptr; + }, + this); + RELEASE_ASSERT(rc == 0, ""); + } + + void join() override { + int rc = pthread_join(thread_id_, nullptr); + RELEASE_ASSERT(rc == 0, ""); + } + +private: + std::function thread_routine_; + pthread_t thread_id_; +}; + +ThreadPtr ThreadFactoryImpl::createThread(std::function thread_routine) { + return std::make_unique(thread_routine); } int32_t currentThreadId() { diff --git a/source/common/common/thread.h b/source/common/common/thread.h index 54f9fc954aea9..191500ab3e419 100644 --- a/source/common/common/thread.h +++ b/source/common/common/thread.h @@ -18,21 +18,13 @@ typedef int32_t ThreadId; ThreadId currentThreadId(); /** - * Wrapper for a pthread thread. We don't use std::thread because it eats exceptions and leads to - * unusable stack traces. + * Implementation of ThreadFactory */ -class ThreadImpl : public Thread { +class ThreadFactoryImpl : public ThreadFactory { public: - ThreadImpl(std::function thread_routine); + ThreadFactoryImpl() {} - /** - * Join on thread exit. - */ - void join() override; - -private: - std::function thread_routine_; - pthread_t thread_id_; + ThreadPtr createThread(std::function thread_routine) override; }; /** diff --git a/source/exe/main_common.cc b/source/exe/main_common.cc index 95ac108f14d1e..9569eb4f919e6 100644 --- a/source/exe/main_common.cc +++ b/source/exe/main_common.cc @@ -42,8 +42,9 @@ Runtime::LoaderPtr ProdComponentFactory::createRuntime(Server::Instance& server, MainCommonBase::MainCommonBase(OptionsImpl& options, Event::TimeSystem& time_system, TestHooks& test_hooks, Server::ComponentFactory& component_factory, - std::unique_ptr&& random_generator) - : options_(options), component_factory_(component_factory) { + std::unique_ptr&& random_generator, + Thread::ThreadFactory& thread_factory) + : options_(options), component_factory_(component_factory), thread_factory_(thread_factory) { ares_library_init(ARES_LIB_INIT_ALL); Event::Libevent::Global::initialize(); RELEASE_ASSERT(Envoy::Server::validateProtoDescriptors(), ""); @@ -74,7 +75,7 @@ MainCommonBase::MainCommonBase(OptionsImpl& options, Event::TimeSystem& time_sys server_ = std::make_unique( options_, time_system, local_address, test_hooks, *restarter_, *stats_store_, - access_log_lock, component_factory, std::move(random_generator), *tls_); + access_log_lock, component_factory, std::move(random_generator), *tls_, thread_factory); break; } @@ -103,7 +104,7 @@ bool MainCommonBase::run() { return true; case Server::Mode::Validate: { auto local_address = Network::Utility::getLocalAddress(options_.localAddressIpVersion()); - return Server::validateConfig(options_, local_address, component_factory_); + return Server::validateConfig(options_, local_address, component_factory_, thread_factory_); } case Server::Mode::InitOnly: PERF_DUMP(); @@ -127,7 +128,7 @@ void MainCommonBase::adminRequest(absl::string_view path_and_query, absl::string MainCommon::MainCommon(int argc, const char* const* argv) : options_(argc, argv, &MainCommon::hotRestartVersion, spdlog::level::info), base_(options_, real_time_system_, default_test_hooks_, prod_component_factory_, - std::make_unique()) {} + std::make_unique(), thread_factory_) {} std::string MainCommon::hotRestartVersion(uint64_t max_num_stats, uint64_t max_stat_name_len, bool hot_restart_enabled) { @@ -152,9 +153,10 @@ int main_common(OptionsImpl& options) { Event::RealTimeSystem real_time_system_; DefaultTestHooks default_test_hooks_; ProdComponentFactory prod_component_factory_; + Thread::ThreadFactoryImpl thread_factory_; MainCommonBase main_common(options, real_time_system_, default_test_hooks_, prod_component_factory_, - std::make_unique()); + std::make_unique(), thread_factory_); return main_common.run() ? EXIT_SUCCESS : EXIT_FAILURE; } catch (EnvoyException& e) { return EXIT_FAILURE; diff --git a/source/exe/main_common.h b/source/exe/main_common.h index e4bd84ed9768a..9d8a49cb5e402 100644 --- a/source/exe/main_common.h +++ b/source/exe/main_common.h @@ -3,6 +3,7 @@ #include "envoy/event/timer.h" #include "envoy/runtime/runtime.h" +#include "common/common/thread.h" #include "common/event/real_time_system.h" #include "common/stats/thread_local_store.h" #include "common/thread_local/thread_local_impl.h" @@ -32,7 +33,8 @@ class MainCommonBase { // destructed. MainCommonBase(OptionsImpl& options, Event::TimeSystem& time_system, TestHooks& test_hooks, Server::ComponentFactory& component_factory, - std::unique_ptr&& random_generator); + std::unique_ptr&& random_generator, + Thread::ThreadFactory& thread_factory); ~MainCommonBase(); bool run(); @@ -62,6 +64,7 @@ class MainCommonBase { Envoy::OptionsImpl& options_; Server::ComponentFactory& component_factory_; + Thread::ThreadFactory& thread_factory_; std::unique_ptr tls_; std::unique_ptr restarter_; @@ -107,6 +110,7 @@ class MainCommon { Event::RealTimeSystem real_time_system_; DefaultTestHooks default_test_hooks_; ProdComponentFactory prod_component_factory_; + Thread::ThreadFactoryImpl thread_factory_; MainCommonBase base_; }; diff --git a/source/server/config_validation/api.cc b/source/server/config_validation/api.cc index 227ed06f4aeb6..c04ab0e77cec4 100644 --- a/source/server/config_validation/api.cc +++ b/source/server/config_validation/api.cc @@ -5,8 +5,9 @@ namespace Envoy { namespace Api { -ValidationImpl::ValidationImpl(std::chrono::milliseconds file_flush_interval_msec) - : Impl(file_flush_interval_msec) {} +ValidationImpl::ValidationImpl(std::chrono::milliseconds file_flush_interval_msec, + Thread::ThreadFactory& thread_factory) + : Impl(file_flush_interval_msec, thread_factory) {} Event::DispatcherPtr ValidationImpl::allocateDispatcher(Event::TimeSystem& time_system) { return Event::DispatcherPtr{new Event::ValidationDispatcher(time_system)}; diff --git a/source/server/config_validation/api.h b/source/server/config_validation/api.h index ef2f71dcbd85d..62378934d7732 100644 --- a/source/server/config_validation/api.h +++ b/source/server/config_validation/api.h @@ -15,7 +15,8 @@ namespace Api { */ class ValidationImpl : public Impl { public: - ValidationImpl(std::chrono::milliseconds file_flush_interval_msec); + ValidationImpl(std::chrono::milliseconds file_flush_interval_msec, + Thread::ThreadFactory& thread_factory); Event::DispatcherPtr allocateDispatcher(Event::TimeSystem&) override; }; diff --git a/source/server/config_validation/server.cc b/source/server/config_validation/server.cc index 824d73f8d2f7d..a6a71c5d577df 100644 --- a/source/server/config_validation/server.cc +++ b/source/server/config_validation/server.cc @@ -20,14 +20,14 @@ namespace Envoy { namespace Server { bool validateConfig(Options& options, Network::Address::InstanceConstSharedPtr local_address, - ComponentFactory& component_factory) { + ComponentFactory& component_factory, Thread::ThreadFactory& thread_factory) { Thread::MutexBasicLockable access_log_lock; Stats::IsolatedStoreImpl stats_store; try { Event::RealTimeSystem time_system; ValidationInstance server(options, time_system, local_address, stats_store, access_log_lock, - component_factory); + component_factory, thread_factory); std::cout << "configuration '" << options.configPath() << "' OK" << std::endl; server.shutdown(); return true; @@ -40,9 +40,10 @@ ValidationInstance::ValidationInstance(Options& options, Event::TimeSystem& time Network::Address::InstanceConstSharedPtr local_address, Stats::IsolatedStoreImpl& store, Thread::BasicLockable& access_log_lock, - ComponentFactory& component_factory) + ComponentFactory& component_factory, + Thread::ThreadFactory& thread_factory) : options_(options), time_system_(time_system), stats_store_(store), - api_(new Api::ValidationImpl(options.fileFlushIntervalMsec())), + api_(new Api::ValidationImpl(options.fileFlushIntervalMsec(), thread_factory)), dispatcher_(api_->allocateDispatcher(time_system)), singleton_manager_(new Singleton::ManagerImpl()), access_log_manager_(*api_, *dispatcher_, access_log_lock, store), mutex_tracer_(nullptr) { diff --git a/source/server/config_validation/server.h b/source/server/config_validation/server.h index 3812d78197845..b77d77f30b549 100644 --- a/source/server/config_validation/server.h +++ b/source/server/config_validation/server.h @@ -34,7 +34,7 @@ namespace Server { * the config is valid, false if invalid. */ bool validateConfig(Options& options, Network::Address::InstanceConstSharedPtr local_address, - ComponentFactory& component_factory); + ComponentFactory& component_factory, Thread::ThreadFactory& thread_factory); /** * ValidationInstance does the bulk of the work for config-validation runs of Envoy. It implements @@ -56,7 +56,7 @@ class ValidationInstance : Logger::Loggable, ValidationInstance(Options& options, Event::TimeSystem& time_system, Network::Address::InstanceConstSharedPtr local_address, Stats::IsolatedStoreImpl& store, Thread::BasicLockable& access_log_lock, - ComponentFactory& component_factory); + ComponentFactory& component_factory, Thread::ThreadFactory& thread_factory); // Server::Instance Admin& admin() override { return admin_; } diff --git a/source/server/server.cc b/source/server/server.cc index 80b8f31d60067..554cd202cd92d 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -50,10 +50,10 @@ InstanceImpl::InstanceImpl(Options& options, Event::TimeSystem& time_system, Thread::BasicLockable& access_log_lock, ComponentFactory& component_factory, Runtime::RandomGeneratorPtr&& random_generator, - ThreadLocal::Instance& tls) + ThreadLocal::Instance& tls, Thread::ThreadFactory& thread_factory) : shutdown_(false), options_(options), time_system_(time_system), restarter_(restarter), start_time_(time(nullptr)), original_start_time_(start_time_), stats_store_(store), - thread_local_(tls), api_(new Api::Impl(options.fileFlushIntervalMsec())), + thread_local_(tls), api_(new Api::Impl(options.fileFlushIntervalMsec(), thread_factory)), secret_manager_(std::make_unique()), dispatcher_(api_->allocateDispatcher(time_system)), singleton_manager_(new Singleton::ManagerImpl()), diff --git a/source/server/server.h b/source/server/server.h index 57081205f3294..1758d41658d59 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -137,7 +137,8 @@ class InstanceImpl : Logger::Loggable, public Instance { Network::Address::InstanceConstSharedPtr local_address, TestHooks& hooks, HotRestart& restarter, Stats::StoreRoot& store, Thread::BasicLockable& access_log_lock, ComponentFactory& component_factory, - Runtime::RandomGeneratorPtr&& random_generator, ThreadLocal::Instance& tls); + Runtime::RandomGeneratorPtr&& random_generator, ThreadLocal::Instance& tls, + Thread::ThreadFactory& thread_factory); ~InstanceImpl() override; diff --git a/test/common/api/BUILD b/test/common/api/BUILD index 0144845cbf774..087fbabf2b7af 100644 --- a/test/common/api/BUILD +++ b/test/common/api/BUILD @@ -14,5 +14,6 @@ envoy_cc_test( deps = [ "//source/common/api:api_lib", "//test/test_common:environment_lib", + "//test/test_common:utility_lib", ], ) diff --git a/test/common/api/api_impl_test.cc b/test/common/api/api_impl_test.cc index f06330d2bd1dc..e6001f482d60b 100644 --- a/test/common/api/api_impl_test.cc +++ b/test/common/api/api_impl_test.cc @@ -4,6 +4,7 @@ #include "common/api/api_impl.h" #include "test/test_common/environment.h" +#include "test/test_common/utility.h" #include "gtest/gtest.h" @@ -11,7 +12,7 @@ namespace Envoy { namespace Api { TEST(ApiImplTest, readFileToEnd) { - Impl api(std::chrono::milliseconds(10000)); + Impl api(std::chrono::milliseconds(1000), Thread::threadFactoryForTest()); const std::string data = "test read To End\nWith new lines."; const std::string file_path = TestEnvironment::writeStringToFileForTest("test_api_envoy", data); @@ -20,7 +21,7 @@ TEST(ApiImplTest, readFileToEnd) { } TEST(ApiImplTest, fileExists) { - Impl api(std::chrono::milliseconds(10000)); + Impl api(std::chrono::milliseconds(1000), Thread::threadFactoryForTest()); EXPECT_TRUE(api.fileExists("/dev/null")); EXPECT_FALSE(api.fileExists("/dev/blahblahblah")); diff --git a/test/common/event/BUILD b/test/common/event/BUILD index 3f196ba90c5cd..26464d75d3dc8 100644 --- a/test/common/event/BUILD +++ b/test/common/event/BUILD @@ -17,6 +17,7 @@ envoy_cc_test( "//source/common/event:dispatcher_lib", "//test/mocks:common_lib", "//test/test_common:test_time_lib", + "//test/test_common:utility_lib", ], ) @@ -46,5 +47,6 @@ envoy_cc_test( "//test/mocks/server:server_mocks", "//test/mocks/stats:stats_mocks", "//test/test_common:test_time_lib", + "//test/test_common:utility_lib", ], ) diff --git a/test/common/event/dispatched_thread_impl_test.cc b/test/common/event/dispatched_thread_impl_test.cc index ed1d8ba29a33f..8842a5110cc26 100644 --- a/test/common/event/dispatched_thread_impl_test.cc +++ b/test/common/event/dispatched_thread_impl_test.cc @@ -10,6 +10,7 @@ #include "test/mocks/server/mocks.h" #include "test/mocks/stats/mocks.h" #include "test/test_common/test_time.h" +#include "test/test_common/utility.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -23,15 +24,15 @@ namespace Event { class DispatchedThreadTest : public testing::Test { protected: DispatchedThreadTest() - : config_(1000, 1000, 1000, 1000), api_(std::chrono::milliseconds(1000)), - thread_(api_, test_time_.timeSystem()), - guard_dog_(fakestats_, config_, test_time_.timeSystem(), api_) {} + : config_(1000, 1000, 1000, 1000), api_(Api::createApiForTest()), + thread_(*api_, test_time_.timeSystem()), + guard_dog_(fakestats_, config_, test_time_.timeSystem(), *api_) {} void SetUp() { thread_.start(guard_dog_); } NiceMock config_; NiceMock fakestats_; DangerousDeprecatedTestTime test_time_; - Api::Impl api_; + Api::ApiPtr api_; DispatchedThreadImpl thread_; Envoy::Server::GuardDogImpl guard_dog_; }; diff --git a/test/common/event/dispatcher_impl_test.cc b/test/common/event/dispatcher_impl_test.cc index f818d78331507..89d1b2eeda277 100644 --- a/test/common/event/dispatcher_impl_test.cc +++ b/test/common/event/dispatcher_impl_test.cc @@ -8,6 +8,7 @@ #include "test/mocks/common.h" #include "test/test_common/test_time.h" +#include "test/test_common/utility.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -62,7 +63,7 @@ class DispatcherImplTest : public ::testing::Test { DispatcherImplTest() : dispatcher_(std::make_unique(test_time_.timeSystem())), work_finished_(false) { - dispatcher_thread_ = api_.createThread([this]() { + dispatcher_thread_ = Thread::threadFactoryForTest().createThread([this]() { // Must create a keepalive timer to keep the dispatcher from exiting. std::chrono::milliseconds time_interval(500); keepalive_timer_ = dispatcher_->createTimer( @@ -80,7 +81,6 @@ class DispatcherImplTest : public ::testing::Test { DangerousDeprecatedTestTime test_time_; - Api::Impl api_; Thread::ThreadPtr dispatcher_thread_; DispatcherPtr dispatcher_; Thread::MutexBasicLockable mu_; diff --git a/test/common/filesystem/BUILD b/test/common/filesystem/BUILD index 0c127b59c02e9..c797d6c72f927 100644 --- a/test/common/filesystem/BUILD +++ b/test/common/filesystem/BUILD @@ -25,6 +25,7 @@ envoy_cc_test( "//test/mocks/filesystem:filesystem_mocks", "//test/test_common:environment_lib", "//test/test_common:threadsafe_singleton_injector_lib", + "//test/test_common:utility_lib", ], ) diff --git a/test/common/filesystem/filesystem_impl_test.cc b/test/common/filesystem/filesystem_impl_test.cc index 1ed281635ad43..eb7da18c69868 100644 --- a/test/common/filesystem/filesystem_impl_test.cc +++ b/test/common/filesystem/filesystem_impl_test.cc @@ -14,6 +14,7 @@ #include "test/mocks/filesystem/mocks.h" #include "test/test_common/environment.h" #include "test/test_common/threadsafe_singleton_injector.h" +#include "test/test_common/utility.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -31,9 +32,9 @@ namespace Envoy { class FileSystemImplTest : public ::testing::Test { protected: - FileSystemImplTest() {} + FileSystemImplTest() : api_(Api::createApiForTest()) {} - Api::Impl api_; + Api::ApiPtr api_; }; TEST_F(FileSystemImplTest, BadFile) { @@ -42,7 +43,7 @@ TEST_F(FileSystemImplTest, BadFile) { Stats::IsolatedStoreImpl store; EXPECT_CALL(dispatcher, createTimer_(_)); EXPECT_THROW( - Filesystem::FileImpl("", dispatcher, lock, store, api_, std::chrono::milliseconds(10000)), + Filesystem::FileImpl("", dispatcher, lock, store, *api_, std::chrono::milliseconds(10000)), EnvoyException); } @@ -125,7 +126,7 @@ TEST_F(FileSystemImplTest, flushToLogFilePeriodically) { TestThreadsafeSingletonInjector os_calls(&os_sys_calls); EXPECT_CALL(os_sys_calls, open_(_, _, _)).WillOnce(Return(5)); - Filesystem::FileImpl file("", dispatcher, mutex, stats_store, api_, + Filesystem::FileImpl file("", dispatcher, mutex, stats_store, *api_, std::chrono::milliseconds(40)); EXPECT_CALL(*timer, enableTimer(std::chrono::milliseconds(40))); @@ -179,7 +180,7 @@ TEST_F(FileSystemImplTest, flushToLogFileOnDemand) { TestThreadsafeSingletonInjector os_calls(&os_sys_calls); EXPECT_CALL(os_sys_calls, open_(_, _, _)).WillOnce(Return(5)); - Filesystem::FileImpl file("", dispatcher, mutex, stats_store, api_, + Filesystem::FileImpl file("", dispatcher, mutex, stats_store, *api_, std::chrono::milliseconds(40)); EXPECT_CALL(*timer, enableTimer(std::chrono::milliseconds(40))); @@ -254,7 +255,7 @@ TEST_F(FileSystemImplTest, reopenFile) { Sequence sq; EXPECT_CALL(os_sys_calls, open_(_, _, _)).InSequence(sq).WillOnce(Return(5)); - Filesystem::FileImpl file("", dispatcher, mutex, stats_store, api_, + Filesystem::FileImpl file("", dispatcher, mutex, stats_store, *api_, std::chrono::milliseconds(40)); EXPECT_CALL(os_sys_calls, write_(_, _, _)) @@ -324,7 +325,7 @@ TEST_F(FileSystemImplTest, reopenThrows) { Sequence sq; EXPECT_CALL(os_sys_calls, open_(_, _, _)).InSequence(sq).WillOnce(Return(5)); - Filesystem::FileImpl file("", dispatcher, mutex, stats_store, api_, + Filesystem::FileImpl file("", dispatcher, mutex, stats_store, *api_, std::chrono::milliseconds(40)); EXPECT_CALL(os_sys_calls, close(5)).InSequence(sq); EXPECT_CALL(os_sys_calls, open_(_, _, _)).InSequence(sq).WillOnce(Return(-1)); @@ -361,7 +362,7 @@ TEST_F(FileSystemImplTest, bigDataChunkShouldBeFlushedWithoutTimer) { NiceMock os_sys_calls; TestThreadsafeSingletonInjector os_calls(&os_sys_calls); - Filesystem::FileImpl file("", dispatcher, mutex, stats_store, api_, + Filesystem::FileImpl file("", dispatcher, mutex, stats_store, *api_, std::chrono::milliseconds(40)); EXPECT_CALL(os_sys_calls, write_(_, _, _)) diff --git a/test/common/grpc/BUILD b/test/common/grpc/BUILD index f6377a1ae6ba3..c7c9d3e7d4fdd 100644 --- a/test/common/grpc/BUILD +++ b/test/common/grpc/BUILD @@ -71,6 +71,7 @@ envoy_cc_test( "//test/mocks/tracing:tracing_mocks", "//test/proto:helloworld_proto_cc", "//test/test_common:test_time_lib", + "//test/test_common:utility_lib", ] + envoy_select_google_grpc(["//source/common/grpc:google_async_client_lib"]), ) @@ -106,6 +107,7 @@ envoy_cc_test_library( "//test/mocks/server:server_mocks", "//test/proto:helloworld_proto_cc", "//test/test_common:test_time_lib", + "//test/test_common:utility_lib", ], ) diff --git a/test/common/grpc/async_client_manager_impl_test.cc b/test/common/grpc/async_client_manager_impl_test.cc index a16e0cb3e51a9..938c3895bdba0 100644 --- a/test/common/grpc/async_client_manager_impl_test.cc +++ b/test/common/grpc/async_client_manager_impl_test.cc @@ -18,15 +18,17 @@ namespace { class AsyncClientManagerImplTest : public testing::Test { public: + AsyncClientManagerImplTest() : api_(Api::createApiForTest()) {} + Upstream::MockClusterManager cm_; NiceMock tls_; Stats::MockStore scope_; DangerousDeprecatedTestTime test_time_; - Api::Impl api_; + Api::ApiPtr api_; }; TEST_F(AsyncClientManagerImplTest, EnvoyGrpcOk) { - AsyncClientManagerImpl async_client_manager(cm_, tls_, test_time_.timeSystem(), api_); + AsyncClientManagerImpl async_client_manager(cm_, tls_, test_time_.timeSystem(), *api_); envoy::api::v2::core::GrpcService grpc_service; grpc_service.mutable_envoy_grpc()->set_cluster_name("foo"); @@ -41,7 +43,7 @@ TEST_F(AsyncClientManagerImplTest, EnvoyGrpcOk) { } TEST_F(AsyncClientManagerImplTest, EnvoyGrpcUnknown) { - AsyncClientManagerImpl async_client_manager(cm_, tls_, test_time_.timeSystem(), api_); + AsyncClientManagerImpl async_client_manager(cm_, tls_, test_time_.timeSystem(), *api_); envoy::api::v2::core::GrpcService grpc_service; grpc_service.mutable_envoy_grpc()->set_cluster_name("foo"); @@ -51,7 +53,7 @@ TEST_F(AsyncClientManagerImplTest, EnvoyGrpcUnknown) { } TEST_F(AsyncClientManagerImplTest, EnvoyGrpcDynamicCluster) { - AsyncClientManagerImpl async_client_manager(cm_, tls_, test_time_.timeSystem(), api_); + AsyncClientManagerImpl async_client_manager(cm_, tls_, test_time_.timeSystem(), *api_); envoy::api::v2::core::GrpcService grpc_service; grpc_service.mutable_envoy_grpc()->set_cluster_name("foo"); @@ -67,7 +69,7 @@ TEST_F(AsyncClientManagerImplTest, EnvoyGrpcDynamicCluster) { TEST_F(AsyncClientManagerImplTest, GoogleGrpc) { EXPECT_CALL(scope_, createScope_("grpc.foo.")); - AsyncClientManagerImpl async_client_manager(cm_, tls_, test_time_.timeSystem(), api_); + AsyncClientManagerImpl async_client_manager(cm_, tls_, test_time_.timeSystem(), *api_); envoy::api::v2::core::GrpcService grpc_service; grpc_service.mutable_google_grpc()->set_stat_prefix("foo"); @@ -80,7 +82,7 @@ TEST_F(AsyncClientManagerImplTest, GoogleGrpc) { } TEST_F(AsyncClientManagerImplTest, EnvoyGrpcUnknownOk) { - AsyncClientManagerImpl async_client_manager(cm_, tls_, test_time_.timeSystem(), api_); + AsyncClientManagerImpl async_client_manager(cm_, tls_, test_time_.timeSystem(), *api_); envoy::api::v2::core::GrpcService grpc_service; grpc_service.mutable_envoy_grpc()->set_cluster_name("foo"); diff --git a/test/common/grpc/google_async_client_impl_test.cc b/test/common/grpc/google_async_client_impl_test.cc index b678d07566302..813f93c20d434 100644 --- a/test/common/grpc/google_async_client_impl_test.cc +++ b/test/common/grpc/google_async_client_impl_test.cc @@ -9,6 +9,7 @@ #include "test/mocks/tracing/mocks.h" #include "test/proto/helloworld.pb.h" #include "test/test_common/test_time.h" +#include "test/test_common/utility.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -46,20 +47,20 @@ class MockStubFactory : public GoogleStubFactory { class EnvoyGoogleAsyncClientImplTest : public testing::Test { public: EnvoyGoogleAsyncClientImplTest() - : dispatcher_(test_time_.timeSystem()), + : dispatcher_(test_time_.timeSystem()), api_(Api::createApiForTest()), method_descriptor_(helloworld::Greeter::descriptor()->FindMethodByName("SayHello")) { envoy::api::v2::core::GrpcService config; auto* google_grpc = config.mutable_google_grpc(); google_grpc->set_target_uri("fake_address"); google_grpc->set_stat_prefix("test_cluster"); - tls_ = std::make_unique(api_); + tls_ = std::make_unique(*api_); grpc_client_ = std::make_unique(dispatcher_, *tls_, stub_factory_, stats_store_, config); } DangerousDeprecatedTestTime test_time_; Event::DispatcherImpl dispatcher_; - Api::Impl api_; + Api::ApiPtr api_; std::unique_ptr tls_; MockStubFactory stub_factory_; const Protobuf::MethodDescriptor* method_descriptor_; diff --git a/test/common/grpc/grpc_client_integration_test_harness.h b/test/common/grpc/grpc_client_integration_test_harness.h index 9efe45e965fc8..d4b5e133fffcf 100644 --- a/test/common/grpc/grpc_client_integration_test_harness.h +++ b/test/common/grpc/grpc_client_integration_test_harness.h @@ -23,6 +23,7 @@ #include "test/proto/helloworld.pb.h" #include "test/test_common/environment.h" #include "test/test_common/test_time.h" +#include "test/test_common/utility.h" using testing::_; using testing::Invoke; @@ -208,7 +209,7 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { public: GrpcClientIntegrationTest() : method_descriptor_(helloworld::Greeter::descriptor()->FindMethodByName("SayHello")), - dispatcher_(test_time_.timeSystem()) {} + dispatcher_(test_time_.timeSystem()), api_(Api::createApiForTest()) {} virtual void initialize() { if (fake_upstream_ == nullptr) { @@ -294,7 +295,7 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { AsyncClientPtr createGoogleAsyncClientImpl() { #ifdef ENVOY_GOOGLE_GRPC - google_tls_ = std::make_unique(api_); + google_tls_ = std::make_unique(*api_); GoogleGenericStubFactory stub_factory; return std::make_unique(dispatcher_, *google_tls_, stub_factory, stats_scope_, createGoogleGrpcConfig()); @@ -402,7 +403,7 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { DangerousDeprecatedTestTime test_time_; Event::DispatcherImpl dispatcher_; DispatcherHelper dispatcher_helper_{dispatcher_}; - Api::Impl api_; + Api::ApiPtr api_; Stats::IsolatedStoreImpl* stats_store_ = new Stats::IsolatedStoreImpl(); Stats::ScopeSharedPtr stats_scope_{stats_store_}; TestMetadata service_wide_initial_metadata_; diff --git a/test/common/singleton/BUILD b/test/common/singleton/BUILD index a47107d07f80d..cb66e6dcad8fd 100644 --- a/test/common/singleton/BUILD +++ b/test/common/singleton/BUILD @@ -21,9 +21,9 @@ envoy_cc_test( name = "threadsafe_singleton_test", srcs = ["threadsafe_singleton_test.cc"], deps = [ - "//source/common/api:api_lib", "//source/common/common:thread_lib", "//source/common/singleton:threadsafe_singleton", "//test/test_common:threadsafe_singleton_injector_lib", + "//test/test_common:utility_lib", ], ) diff --git a/test/common/singleton/threadsafe_singleton_test.cc b/test/common/singleton/threadsafe_singleton_test.cc index 61a51e42502ef..138d3309fc498 100644 --- a/test/common/singleton/threadsafe_singleton_test.cc +++ b/test/common/singleton/threadsafe_singleton_test.cc @@ -1,11 +1,11 @@ #include -#include "common/api/api_impl.h" #include "common/common/lock_guard.h" #include "common/common/thread.h" #include "common/singleton/threadsafe_singleton.h" #include "test/test_common/threadsafe_singleton_injector.h" +#include "test/test_common/utility.h" #include "gtest/gtest.h" @@ -43,7 +43,7 @@ class EvilMathSingleton : public TestSingleton { class AddTen { public: AddTen() { - thread_ = api_.createThread([this]() -> void { threadRoutine(); }); + thread_ = Thread::threadFactoryForTest().createThread([this]() -> void { threadRoutine(); }); } ~AddTen() { thread_->join(); @@ -57,7 +57,6 @@ class AddTen { singleton.addOne(); } } - Api::Impl api_; Thread::ThreadPtr thread_; }; diff --git a/test/common/thread_local/thread_local_impl_test.cc b/test/common/thread_local/thread_local_impl_test.cc index 96b3f5ad92b52..2f5e2c15f8884 100644 --- a/test/common/thread_local/thread_local_impl_test.cc +++ b/test/common/thread_local/thread_local_impl_test.cc @@ -1,4 +1,4 @@ -#include "common/api/api_impl.h" +#include "common/common/thread.h" #include "common/event/dispatcher_impl.h" #include "common/thread_local/thread_local_impl.h" @@ -116,7 +116,6 @@ TEST_F(ThreadLocalInstanceImplTest, RunOnAllThreads) { TEST(ThreadLocalInstanceImplDispatcherTest, Dispatcher) { InstanceImpl tls; - Api::Impl api; DangerousDeprecatedTestTime test_time; Event::DispatcherImpl main_dispatcher(test_time.timeSystem()); Event::DispatcherImpl thread_dispatcher(test_time.timeSystem()); @@ -129,12 +128,13 @@ TEST(ThreadLocalInstanceImplDispatcherTest, Dispatcher) { // Verify we have the expected dispatcher for the main thread. EXPECT_EQ(&main_dispatcher, &tls.dispatcher()); - Thread::ThreadPtr thread = api.createThread([&thread_dispatcher, &tls]() { - // Ensure that the dispatcher update in tls posted during the above registerThread happens. - thread_dispatcher.run(Event::Dispatcher::RunType::NonBlock); - // Verify we have the expected dispatcher for the new thread thread. - EXPECT_EQ(&thread_dispatcher, &tls.dispatcher()); - }); + Thread::ThreadPtr thread = + Thread::threadFactoryForTest().createThread([&thread_dispatcher, &tls]() { + // Ensure that the dispatcher update in tls posted during the above registerThread happens. + thread_dispatcher.run(Event::Dispatcher::RunType::NonBlock); + // Verify we have the expected dispatcher for the new thread thread. + EXPECT_EQ(&thread_dispatcher, &tls.dispatcher()); + }); thread->join(); // Verify we still have the expected dispatcher for the main thread. diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index 55a3c809c0f0a..d96daeb19f008 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -165,12 +165,14 @@ envoy::config::bootstrap::v2::Bootstrap parseBootstrapFromV2Yaml(const std::stri class ClusterManagerImplTest : public testing::Test { public: - ClusterManagerImplTest() { factory_.dispatcher_.setTimeSystem(time_system_); } + ClusterManagerImplTest() : api_(Api::createApiForTest()) { + factory_.dispatcher_.setTimeSystem(time_system_); + } void create(const envoy::config::bootstrap::v2::Bootstrap& bootstrap) { cluster_manager_ = std::make_unique( bootstrap, factory_, factory_.stats_, factory_.tls_, factory_.runtime_, factory_.random_, - factory_.local_info_, log_manager_, factory_.dispatcher_, admin_, api_); + factory_.local_info_, log_manager_, factory_.dispatcher_, admin_, *api_); } void createWithLocalClusterUpdate(const bool enable_merge_window = true) { @@ -204,7 +206,7 @@ class ClusterManagerImplTest : public testing::Test { cluster_manager_ = std::make_unique( bootstrap, factory_, factory_.stats_, factory_.tls_, factory_.runtime_, factory_.random_, - factory_.local_info_, log_manager_, factory_.dispatcher_, admin_, api_, + factory_.local_info_, log_manager_, factory_.dispatcher_, admin_, *api_, local_cluster_update_); } @@ -239,7 +241,7 @@ class ClusterManagerImplTest : public testing::Test { return metadata; } - Api::Impl api_; + Api::ApiPtr api_; NiceMock factory_; std::unique_ptr cluster_manager_; AccessLog::MockAccessLogManager log_manager_; diff --git a/test/config_test/config_test.cc b/test/config_test/config_test.cc index 3694690559c84..1074ec8dacadb 100644 --- a/test/config_test/config_test.cc +++ b/test/config_test/config_test.cc @@ -4,7 +4,6 @@ #include #include -#include "common/api/api_impl.h" #include "common/common/fmt.h" #include "common/filesystem/filesystem_impl.h" #include "common/protobuf/utility.h" @@ -43,7 +42,7 @@ OptionsImpl asConfigYaml(const OptionsImpl& src) { class ConfigTest { public: - ConfigTest(const OptionsImpl& options) : options_(options) { + ConfigTest(const OptionsImpl& options) : api_(Api::createApiForTest()), options_(options) { ON_CALL(server_, options()).WillByDefault(ReturnRef(options_)); ON_CALL(server_, random()).WillByDefault(ReturnRef(random_)); ON_CALL(server_, sslContextManager()).WillByDefault(ReturnRef(ssl_context_manager_)); @@ -58,7 +57,7 @@ class ConfigTest { cluster_manager_factory_ = std::make_unique( server_.runtime(), server_.stats(), server_.threadLocal(), server_.random(), server_.dnsResolver(), ssl_context_manager_, server_.dispatcher(), server_.localInfo(), - server_.secretManager(), api_); + server_.secretManager(), *api_); ON_CALL(server_, clusterManager()).WillByDefault(Invoke([&]() -> Upstream::ClusterManager& { return *main_config.clusterManager(); @@ -91,7 +90,7 @@ class ConfigTest { server_.thread_local_.shutdownThread(); } - Api::Impl api_; + Api::ApiPtr api_; NiceMock server_; NiceMock ssl_context_manager_; OptionsImpl options_; diff --git a/test/exe/main_common_test.cc b/test/exe/main_common_test.cc index 623190a83144f..c79a35b66112d 100644 --- a/test/exe/main_common_test.cc +++ b/test/exe/main_common_test.cc @@ -1,6 +1,5 @@ #include -#include "common/api/api_impl.h" #include "common/common/lock_guard.h" #include "common/common/mutex_tracer_impl.h" #include "common/common/thread.h" @@ -158,7 +157,7 @@ class AdminRequestTest : public MainCommonTest { // Initiates Envoy running in its own thread. void startEnvoy() { - envoy_thread_ = api_.createThread([this]() { + envoy_thread_ = Thread::threadFactoryForTest().createThread([this]() { // Note: main_common_ is accessed in the testing thread, but // is race-free, as MainCommon::run() does not return until // triggered with an adminRequest POST to /quitquitquit, which @@ -193,7 +192,6 @@ class AdminRequestTest : public MainCommonTest { return envoy_return_; } - Api::Impl api_; std::unique_ptr envoy_thread_; std::unique_ptr main_common_; absl::Notification started_; diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index c6e047f66dd99..148b494dd2c9e 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -364,7 +364,7 @@ FakeUpstream::FakeUpstream(Network::TransportSocketFactoryPtr&& transport_socket FakeUpstream::FakeUpstream(Network::TransportSocketFactoryPtr&& transport_socket_factory, Network::SocketPtr&& listen_socket, FakeHttpConnection::Type type, Event::TestTimeSystem& time_system, bool enable_half_close) - : http_type_(type), socket_(std::move(listen_socket)), api_(new Api::Impl(milliseconds(10000))), + : http_type_(type), socket_(std::move(listen_socket)), api_(Api::createApiForTest()), time_system_(time_system), dispatcher_(api_->allocateDispatcher(time_system_)), handler_(new Server::ConnectionHandlerImpl(ENVOY_LOGGER(), *dispatcher_)), allow_unexpected_disconnects_(false), enable_half_close_(enable_half_close), listener_(*this), diff --git a/test/integration/integration.cc b/test/integration/integration.cc index d2bebc7cc39e2..49fda350ce85d 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -213,8 +213,8 @@ void IntegrationTcpClient::ConnectionCallbacks::onEvent(Network::ConnectionEvent BaseIntegrationTest::BaseIntegrationTest(Network::Address::IpVersion version, TestTimeSystemPtr time_system, const std::string& config) - : api_(new Api::Impl(std::chrono::milliseconds(10000))), - mock_buffer_factory_(new NiceMock), time_system_(std::move(time_system)), + : api_(Api::createApiForTest()), mock_buffer_factory_(new NiceMock), + time_system_(std::move(time_system)), dispatcher_(new Event::DispatcherImpl(*time_system_, Buffer::WatermarkFactoryPtr{mock_buffer_factory_})), version_(version), config_helper_(version, config), diff --git a/test/integration/server.cc b/test/integration/server.cc index 1e90091ad727d..3f2319536c6b5 100644 --- a/test/integration/server.cc +++ b/test/integration/server.cc @@ -5,6 +5,7 @@ #include "envoy/http/header_map.h" +#include "common/common/thread.h" #include "common/filesystem/filesystem_impl.h" #include "common/local_info/local_info_impl.h" #include "common/network/utility.h" @@ -154,7 +155,8 @@ void IntegrationTestServerImpl::createAndRunEnvoyServer( Stats::ThreadLocalStoreImpl stat_store(options.statsOptions(), stats_allocator); Server::InstanceImpl server(options, time_system, local_address, hooks, restarter, stat_store, - access_log_lock, component_factory, std::move(random_generator), tls); + access_log_lock, component_factory, std::move(random_generator), tls, + Thread::threadFactoryForTest()); // This is technically thread unsafe (assigning to a shared_ptr accessed // across threads), but because we synchronize below through serverReady(), the only // consumer on the main test thread in ~IntegrationTestServerImpl will not race. diff --git a/test/integration/utility.cc b/test/integration/utility.cc index ef4166830c893..30740c5719181 100644 --- a/test/integration/utility.cc +++ b/test/integration/utility.cc @@ -61,8 +61,8 @@ IntegrationUtil::makeSingleRequest(const Network::Address::InstanceConstSharedPt const std::string& body, Http::CodecClient::Type type, const std::string& host, const std::string& content_type) { - Api::Impl api(std::chrono::milliseconds(9000)); - Event::DispatcherPtr dispatcher(api.allocateDispatcher(evil_singleton_test_time_.timeSystem())); + Api::ApiPtr api(Api::createApiForTest()); + Event::DispatcherPtr dispatcher(api->allocateDispatcher(evil_singleton_test_time_.timeSystem())); std::shared_ptr cluster{new NiceMock()}; Upstream::HostDescriptionConstSharedPtr host_description{ Upstream::makeTestHostDescription(cluster, "tcp://127.0.0.1:80")}; @@ -109,7 +109,7 @@ IntegrationUtil::makeSingleRequest(uint32_t port, const std::string& method, con RawConnectionDriver::RawConnectionDriver(uint32_t port, Buffer::Instance& initial_data, ReadCallback data_callback, Network::Address::IpVersion version) { - api_ = std::make_unique(std::chrono::milliseconds(10000)); + api_ = Api::createApiForTest(); dispatcher_ = api_->allocateDispatcher(IntegrationUtil::evil_singleton_test_time_.timeSystem()); callbacks_ = std::make_unique(); client_ = dispatcher_->createClientConnection( diff --git a/test/server/BUILD b/test/server/BUILD index 88972215cbddb..5055ead4ec35a 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -101,6 +101,7 @@ envoy_cc_test( "//test/mocks:common_lib", "//test/mocks/server:server_mocks", "//test/mocks/stats:stats_mocks", + "//test/test_common:utility_lib", ], ) diff --git a/test/server/config_validation/BUILD b/test/server/config_validation/BUILD index 6b0eef4b8c410..8c5ae0b0d1e1f 100644 --- a/test/server/config_validation/BUILD +++ b/test/server/config_validation/BUILD @@ -86,6 +86,7 @@ envoy_cc_fuzz_test( srcs = ["config_fuzz_test.cc"], corpus = "//test/server:server_fuzz_test_corpus", deps = [ + "//source/common/common:thread_lib", "//source/server/config_validation:server_lib", "//source/server:proto_descriptors_lib", "//test/integration:integration_lib", diff --git a/test/server/config_validation/cluster_manager_test.cc b/test/server/config_validation/cluster_manager_test.cc index 8bfca0a806ebd..ee23518be51a7 100644 --- a/test/server/config_validation/cluster_manager_test.cc +++ b/test/server/config_validation/cluster_manager_test.cc @@ -23,7 +23,7 @@ namespace Envoy { namespace Upstream { TEST(ValidationClusterManagerTest, MockedMethods) { - Api::Impl api; + Api::ApiPtr api(Api::createApiForTest()); NiceMock runtime; Event::SimulatedTimeSystem time_system; Stats::IsolatedStoreImpl stats; @@ -38,7 +38,7 @@ TEST(ValidationClusterManagerTest, MockedMethods) { ValidationClusterManagerFactory factory(runtime, stats, tls, random, dns_resolver, ssl_context_manager, dispatcher, local_info, - secret_manager, api); + secret_manager, *api); AccessLog::MockAccessLogManager log_manager; const envoy::config::bootstrap::v2::Bootstrap bootstrap; diff --git a/test/server/config_validation/config_fuzz_test.cc b/test/server/config_validation/config_fuzz_test.cc index 51f7049a681e7..550e3a1b88c10 100644 --- a/test/server/config_validation/config_fuzz_test.cc +++ b/test/server/config_validation/config_fuzz_test.cc @@ -1,5 +1,6 @@ #include +#include "common/common/thread.h" #include "common/network/address_impl.h" #include "server/config_validation/server.h" @@ -29,7 +30,8 @@ DEFINE_PROTO_FUZZER(const envoy::config::bootstrap::v2::Bootstrap& input) { options.log_level_ = Fuzz::Runner::logLevel(); try { - validateConfig(options, Network::Address::InstanceConstSharedPtr(), component_factory); + validateConfig(options, Network::Address::InstanceConstSharedPtr(), component_factory, + Thread::threadFactoryForTest()); } catch (const EnvoyException& ex) { ENVOY_LOG_MISC(debug, "Controlled EnvoyException exit: {}", ex.what()); } diff --git a/test/server/config_validation/dispatcher_test.cc b/test/server/config_validation/dispatcher_test.cc index 8f9757bb7e72f..0ae3531e6ffd0 100644 --- a/test/server/config_validation/dispatcher_test.cc +++ b/test/server/config_validation/dispatcher_test.cc @@ -1,5 +1,6 @@ #include +#include "common/common/thread.h" #include "common/event/dispatcher_impl.h" #include "common/event/libevent.h" #include "common/network/address_impl.h" @@ -22,7 +23,8 @@ class ConfigValidation : public ::testing::TestWithParam(std::chrono::milliseconds(1000)); + validation_ = std::make_unique(std::chrono::milliseconds(1000), + Thread::threadFactoryForTest()); dispatcher_ = validation_->allocateDispatcher(test_time_.timeSystem()); } diff --git a/test/server/config_validation/server_test.cc b/test/server/config_validation/server_test.cc index e05f1a659023b..fa687ad263740 100644 --- a/test/server/config_validation/server_test.cc +++ b/test/server/config_validation/server_test.cc @@ -49,8 +49,8 @@ class ValidationServerTest_1 : public ValidationServerTest { }; TEST_P(ValidationServerTest, Validate) { - EXPECT_TRUE( - validateConfig(options_, Network::Address::InstanceConstSharedPtr(), component_factory_)); + EXPECT_TRUE(validateConfig(options_, Network::Address::InstanceConstSharedPtr(), + component_factory_, Thread::threadFactoryForTest())); } // TODO(rlazarus): We'd like use this setup to replace //test/config_test (that is, run it against @@ -64,7 +64,8 @@ INSTANTIATE_TEST_CASE_P(ValidConfigs, ValidationServerTest, // Just make sure that all configs can be ingested without a crash. Processing of config files // may not be successful, but there should be no crash. TEST_P(ValidationServerTest_1, RunWithoutCrash) { - validateConfig(options_, Network::Address::InstanceConstSharedPtr(), component_factory_); + validateConfig(options_, Network::Address::InstanceConstSharedPtr(), component_factory_, + Thread::threadFactoryForTest()); SUCCEED(); } diff --git a/test/server/configuration_impl_test.cc b/test/server/configuration_impl_test.cc index 0e14f89b37904..13304556fd7fe 100644 --- a/test/server/configuration_impl_test.cc +++ b/test/server/configuration_impl_test.cc @@ -53,12 +53,13 @@ TEST(FilterChainUtility, buildFilterChainFailWithBadFilters) { class ConfigurationImplTest : public testing::Test { protected: ConfigurationImplTest() - : cluster_manager_factory_(server_.runtime(), server_.stats(), server_.threadLocal(), + : api_(Api::createApiForTest()), + cluster_manager_factory_(server_.runtime(), server_.stats(), server_.threadLocal(), server_.random(), server_.dnsResolver(), server_.sslContextManager(), server_.dispatcher(), - server_.localInfo(), server_.secretManager(), api_) {} + server_.localInfo(), server_.secretManager(), *api_) {} - Api::Impl api_; + Api::ApiPtr api_; NiceMock server_; Upstream::ProdClusterManagerFactory cluster_manager_factory_; }; diff --git a/test/server/guarddog_impl_test.cc b/test/server/guarddog_impl_test.cc index c873be0880740..1dbf0850185dd 100644 --- a/test/server/guarddog_impl_test.cc +++ b/test/server/guarddog_impl_test.cc @@ -13,6 +13,7 @@ #include "test/mocks/server/mocks.h" #include "test/mocks/stats/mocks.h" #include "test/test_common/simulated_time_system.h" +#include "test/test_common/utility.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -25,8 +26,10 @@ namespace Server { class GuardDogTestBase : public testing::Test { protected: + GuardDogTestBase() : api_(Api::createApiForTest()) {} + Event::SimulatedTimeSystem time_system_; - Api::Impl api_; + Api::ApiPtr api_; }; /** @@ -47,7 +50,7 @@ class GuardDogDeathTest : public GuardDogTestBase { */ void SetupForDeath() { InSequence s; - guard_dog_ = std::make_unique(fakestats_, config_kill_, time_system_, api_); + guard_dog_ = std::make_unique(fakestats_, config_kill_, time_system_, *api_); unpet_dog_ = guard_dog_->createWatchDog(0); guard_dog_->forceCheckForTest(); time_system_.sleep(std::chrono::milliseconds(500)); @@ -59,7 +62,7 @@ class GuardDogDeathTest : public GuardDogTestBase { */ void SetupForMultiDeath() { InSequence s; - guard_dog_ = std::make_unique(fakestats_, config_multikill_, time_system_, api_); + guard_dog_ = std::make_unique(fakestats_, config_multikill_, time_system_, *api_); auto unpet_dog_ = guard_dog_->createWatchDog(0); guard_dog_->forceCheckForTest(); auto second_dog_ = guard_dog_->createWatchDog(1); @@ -116,7 +119,7 @@ TEST_F(GuardDogAlmostDeadTest, NearDeathTest) { // This ensures that if only one thread surpasses the multiple kill threshold // there is no death. The positive case is covered in MultiKillDeathTest. InSequence s; - GuardDogImpl gd(fakestats_, config_multikill_, time_system_, api_); + GuardDogImpl gd(fakestats_, config_multikill_, time_system_, *api_); auto unpet_dog = gd.createWatchDog(0); auto pet_dog = gd.createWatchDog(1); // This part "waits" 600 milliseconds while one dog is touched every 100, and @@ -142,7 +145,7 @@ class GuardDogMissTest : public GuardDogTestBase { TEST_F(GuardDogMissTest, MissTest) { // This test checks the actual collected statistics after doing some timer // advances that should and shouldn't increment the counters. - GuardDogImpl gd(stats_store_, config_miss_, time_system_, api_); + GuardDogImpl gd(stats_store_, config_miss_, time_system_, *api_); // We'd better start at 0: EXPECT_EQ(0UL, stats_store_.counter("server.watchdog_miss").value()); auto unpet_dog = gd.createWatchDog(0); @@ -161,7 +164,7 @@ TEST_F(GuardDogMissTest, MissTest) { TEST_F(GuardDogMissTest, MegaMissTest) { // This test checks the actual collected statistics after doing some timer // advances that should and shouldn't increment the counters. - GuardDogImpl gd(stats_store_, config_mega_, time_system_, api_); + GuardDogImpl gd(stats_store_, config_mega_, time_system_, *api_); auto unpet_dog = gd.createWatchDog(0); // We'd better start at 0: EXPECT_EQ(0UL, stats_store_.counter("server.watchdog_mega_miss").value()); @@ -181,7 +184,7 @@ TEST_F(GuardDogMissTest, MissCountTest) { // This tests a flake discovered in the MissTest where real timeout or // spurious condition_variable wakeup causes the counter to get incremented // more than it should be. - GuardDogImpl gd(stats_store_, config_miss_, time_system_, api_); + GuardDogImpl gd(stats_store_, config_miss_, time_system_, *api_); auto sometimes_pet_dog = gd.createWatchDog(0); // These steps are executed once without ever touching the watchdog. // Then the last step is to touch the watchdog and repeat the steps. @@ -221,27 +224,27 @@ TEST_F(GuardDogMissTest, MissCountTest) { TEST_F(GuardDogTestBase, StartStopTest) { NiceMock stats; NiceMock config(0, 0, 0, 0); - GuardDogImpl gd(stats, config, time_system_, api_); + GuardDogImpl gd(stats, config, time_system_, *api_); } TEST_F(GuardDogTestBase, LoopIntervalNoKillTest) { NiceMock stats; NiceMock config(40, 50, 0, 0); - GuardDogImpl gd(stats, config, time_system_, api_); + GuardDogImpl gd(stats, config, time_system_, *api_); EXPECT_EQ(gd.loopIntervalForTest(), 40); } TEST_F(GuardDogTestBase, LoopIntervalTest) { NiceMock stats; NiceMock config(100, 90, 1000, 500); - GuardDogImpl gd(stats, config, time_system_, api_); + GuardDogImpl gd(stats, config, time_system_, *api_); EXPECT_EQ(gd.loopIntervalForTest(), 90); } TEST_F(GuardDogTestBase, WatchDogThreadIdTest) { NiceMock stats; NiceMock config(100, 90, 1000, 500); - GuardDogImpl gd(stats, config, time_system_, api_); + GuardDogImpl gd(stats, config, time_system_, *api_); auto watched_dog = gd.createWatchDog(123); EXPECT_EQ(watched_dog->threadId(), 123); gd.stopWatching(watched_dog); diff --git a/test/server/server_fuzz_test.cc b/test/server/server_fuzz_test.cc index 7b6879d8d2128..973982f60436b 100644 --- a/test/server/server_fuzz_test.cc +++ b/test/server/server_fuzz_test.cc @@ -79,7 +79,7 @@ DEFINE_PROTO_FUZZER(const envoy::config::bootstrap::v2::Bootstrap& input) { 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_local_instance, Thread::threadFactoryForTest()); } catch (const EnvoyException& ex) { ENVOY_LOG_MISC(debug, "Controlled EnvoyException exit: {}", ex.what()); return; diff --git a/test/server/server_test.cc b/test/server/server_test.cc index 1161d20b7ef52..2ec7b7b02d46d 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -126,7 +126,8 @@ class ServerInstanceImplTest : public testing::TestWithParam>(), thread_local_); + std::make_unique>(), thread_local_, + Thread::threadFactoryForTest()); EXPECT_TRUE(server_->api().fileExists("/dev/null")); } @@ -142,7 +143,8 @@ class ServerInstanceImplTest : public testing::TestWithParam>(), thread_local_); + std::make_unique>(), thread_local_, + Thread::threadFactoryForTest()); EXPECT_TRUE(server_->api().fileExists("/dev/null")); } @@ -337,7 +339,8 @@ TEST_P(ServerInstanceImplTest, NoOptionsPassed) { 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_))); + std::make_unique>(), thread_local_, + Thread::threadFactoryForTest()))); } // Validate that when std::exception is unexpectedly thrown, we exit safely. diff --git a/test/server/worker_impl_test.cc b/test/server/worker_impl_test.cc index d91a790b98dbe..d5a8b4c3f4ea0 100644 --- a/test/server/worker_impl_test.cc +++ b/test/server/worker_impl_test.cc @@ -24,7 +24,7 @@ namespace Server { class WorkerImplTest : public testing::Test { public: - WorkerImplTest() { + WorkerImplTest() : api_(Api::createApiForTest()) { // In the real worker the watchdog has timers that prevent exit. Here we need to prevent event // loop exit since we use mock timers. no_exit_timer_->enableTimer(std::chrono::hours(1)); @@ -36,14 +36,14 @@ class WorkerImplTest : public testing::Test { Network::MockConnectionHandler* handler_ = new Network::MockConnectionHandler(); NiceMock guard_dog_; NiceMock overload_manager_; - Api::Impl api_; + Api::ApiPtr api_; DefaultTestHooks hooks_; WorkerImpl worker_{tls_, hooks_, Event::DispatcherPtr{dispatcher_}, Network::ConnectionHandlerPtr{handler_}, overload_manager_, - api_}; + *api_}; Event::TimerPtr no_exit_timer_ = dispatcher_->createTimer([]() -> void {}); }; diff --git a/test/test_common/BUILD b/test/test_common/BUILD index 84593f9108f30..46eed2f32ddc7 100644 --- a/test/test_common/BUILD +++ b/test/test_common/BUILD @@ -57,6 +57,7 @@ envoy_cc_test_library( "//source/common/common:mutex_tracer_lib", "//source/common/common:thread_lib", "//test/test_common:test_time_lib", + "//test/test_common:utility_lib", ], ) @@ -89,6 +90,7 @@ envoy_cc_test_library( "//include/envoy/buffer:buffer_interface", "//include/envoy/http:codec_interface", "//include/envoy/network:address_interface", + "//source/common/api:api_lib", "//source/common/common:empty_string", "//source/common/common:thread_lib", "//source/common/common:utility_lib", @@ -162,7 +164,10 @@ envoy_cc_test_library( envoy_cc_test( name = "simulated_time_system_test", srcs = ["simulated_time_system_test.cc"], - deps = [":simulated_time_system_lib"], + deps = [ + ":simulated_time_system_lib", + "//test/test_common:utility_lib", + ], ) envoy_cc_library( diff --git a/test/test_common/contention.cc b/test/test_common/contention.cc index 9744d76a8612f..8461ef2d47cc6 100644 --- a/test/test_common/contention.cc +++ b/test/test_common/contention.cc @@ -1,5 +1,7 @@ #include "test/test_common/contention.h" +#include "test/test_common/utility.h" + namespace Envoy { namespace Thread { namespace TestUtil { @@ -14,7 +16,7 @@ void ContentionGenerator::generateContention(MutexTracerImpl& tracer) { Envoy::Thread::ThreadPtr ContentionGenerator::launchThread(MutexTracerImpl& tracer, MutexBasicLockable* mu) { - return std::make_unique( + return threadFactoryForTest().createThread( [&tracer, mu]() -> void { holdUntilContention(tracer, mu); }); } diff --git a/test/test_common/simulated_time_system_test.cc b/test/test_common/simulated_time_system_test.cc index 6e9243709bdfe..3dcd5a6278681 100644 --- a/test/test_common/simulated_time_system_test.cc +++ b/test/test_common/simulated_time_system_test.cc @@ -2,6 +2,7 @@ #include "common/event/libevent.h" #include "test/test_common/simulated_time_system.h" +#include "test/test_common/utility.h" #include "event2/event.h" #include "gtest/gtest.h" @@ -60,7 +61,7 @@ TEST_F(SimulatedTimeSystemTest, WaitFor) { // Run an event loop in the background to activate timers. std::atomic done(false); - auto thread = std::make_unique([this, &done]() { + auto thread = Thread::threadFactoryForTest().createThread([this, &done]() { while (!done) { event_base_loop(event_system_.get(), 0); } diff --git a/test/test_common/utility.cc b/test/test_common/utility.cc index 495afb3988063..ce095ac9e8f9a 100644 --- a/test/test_common/utility.cc +++ b/test/test_common/utility.cc @@ -22,10 +22,12 @@ #include "envoy/buffer/buffer.h" #include "envoy/http/codec.h" +#include "common/api/api_impl.h" #include "common/common/empty_string.h" #include "common/common/fmt.h" #include "common/common/lock_guard.h" #include "common/common/stack_array.h" +#include "common/common/thread.h" #include "common/common/utility.h" #include "common/config/bootstrap_json.h" #include "common/json/json_loader.h" @@ -342,4 +344,21 @@ MockedTestAllocator::~MockedTestAllocator() {} } // namespace Stats +namespace Thread { + +ThreadFactory& threadFactoryForTest() { + static ThreadFactoryImpl* thread_factory = new ThreadFactoryImpl(); + return *thread_factory; +} + +} // namespace Thread + +namespace Api { + +ApiPtr createApiForTest() { + return std::make_unique(std::chrono::milliseconds(1000), Thread::threadFactoryForTest()); +} + +} // namespace Api + } // namespace Envoy diff --git a/test/test_common/utility.h b/test/test_common/utility.h index dc528663bc844..a8e4cfb0d5c21 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -7,11 +7,13 @@ #include #include +#include "envoy/api/api.h" #include "envoy/buffer/buffer.h" #include "envoy/config/bootstrap/v2/bootstrap.pb.h" #include "envoy/network/address.h" #include "envoy/stats/stats.h" #include "envoy/stats/store.h" +#include "envoy/thread/thread.h" #include "common/buffer/buffer_impl.h" #include "common/common/c_smart_ptr.h" @@ -451,6 +453,14 @@ class MockedTestAllocator : public RawStatDataAllocator { } // namespace Stats +namespace Thread { +ThreadFactory& threadFactoryForTest(); +} // namespace Thread + +namespace Api { +ApiPtr createApiForTest(); +} // namespace Api + MATCHER_P(HeaderMapEqualIgnoreOrder, rhs, "") { *result_listener << *rhs << " is not equal to " << *arg; return TestUtility::headerMapEqualIgnoreOrder(*arg, *rhs);