diff --git a/include/envoy/api/api.h b/include/envoy/api/api.h index 526b812d28b0..82ccc58cff06 100644 --- a/include/envoy/api/api.h +++ b/include/envoy/api/api.h @@ -48,10 +48,9 @@ class Api { virtual std::string fileReadToEnd(const std::string& path) PURE; /** - * Create a thread. - * @param thread_routine supplies the function to invoke in the thread. + * @return a reference to the ThreadFactory */ - virtual Thread::ThreadPtr createThread(std::function thread_routine) PURE; + virtual Thread::ThreadFactory& threadFactory() PURE; }; typedef std::unique_ptr ApiPtr; diff --git a/include/envoy/common/platform.h b/include/envoy/common/platform.h index bfc8eef2e12e..e98b9d136562 100644 --- a/include/envoy/common/platform.h +++ b/include/envoy/common/platform.h @@ -1,14 +1,14 @@ #pragma once // NOLINT(namespace-envoy) -#if !defined(_MSC_VER) -#define PACKED_STRUCT(definition, ...) definition, ##__VA_ARGS__ __attribute__((packed)) - -#else +#ifdef _MSC_VER #include #define PACKED_STRUCT(definition, ...) \ __pragma(pack(push, 1)) definition, ##__VA_ARGS__; \ __pragma(pack(pop)) +#else +#define PACKED_STRUCT(definition, ...) definition, ##__VA_ARGS__ __attribute__((packed)) + #endif diff --git a/include/envoy/event/BUILD b/include/envoy/event/BUILD index 8b133c7737e1..33e466455bb2 100644 --- a/include/envoy/event/BUILD +++ b/include/envoy/event/BUILD @@ -48,7 +48,6 @@ envoy_cc_library( hdrs = ["timer.h"], deps = [ "//include/envoy/common:time_interface", - "//source/common/common:thread_lib", "//source/common/event:libevent_lib", ], ) diff --git a/include/envoy/event/timer.h b/include/envoy/event/timer.h index 60bbc2e223a9..7ebc8bb14787 100644 --- a/include/envoy/event/timer.h +++ b/include/envoy/event/timer.h @@ -7,7 +7,6 @@ #include "envoy/common/pure.h" #include "envoy/common/time.h" -#include "common/common/thread.h" #include "common/event/libevent.h" namespace Envoy { diff --git a/include/envoy/server/BUILD b/include/envoy/server/BUILD index 2bb6674b7fc9..bbf95b4a9614 100644 --- a/include/envoy/server/BUILD +++ b/include/envoy/server/BUILD @@ -64,6 +64,7 @@ envoy_cc_library( deps = [ "//include/envoy/server:watchdog_interface", "//include/envoy/stats:stats_interface", + "//include/envoy/thread:thread_interface", ], ) @@ -134,6 +135,7 @@ envoy_cc_library( deps = [ "//include/envoy/event:dispatcher_interface", "//include/envoy/network:address_interface", + "//include/envoy/thread:thread_interface", ], ) diff --git a/include/envoy/server/guarddog.h b/include/envoy/server/guarddog.h index 4d7077b00c2b..ce5a37c4b2bc 100644 --- a/include/envoy/server/guarddog.h +++ b/include/envoy/server/guarddog.h @@ -26,9 +26,9 @@ class GuardDog { * to avoid triggering the GuardDog. If no longer needed use the * stopWatching() method to remove it from the list of watched objects. * - * @param thread_id A numeric thread ID, like from Thread::currentThreadId() + * @param thread_id a Thread::ThreadIdPtr containing the system thread id */ - virtual WatchDogSharedPtr createWatchDog(int32_t thread_id) PURE; + virtual WatchDogSharedPtr createWatchDog(Thread::ThreadIdPtr&& thread_id) PURE; /** * Tell the GuardDog to forget about this WatchDog. diff --git a/include/envoy/server/watchdog.h b/include/envoy/server/watchdog.h index 7def94792d04..a571326f6ad4 100644 --- a/include/envoy/server/watchdog.h +++ b/include/envoy/server/watchdog.h @@ -4,6 +4,7 @@ #include "envoy/common/pure.h" #include "envoy/event/dispatcher.h" +#include "envoy/thread/thread.h" namespace Envoy { namespace Server { @@ -35,7 +36,7 @@ class WatchDog { * This can be used if this is later used on a thread where there is no dispatcher. */ virtual void touch() PURE; - virtual int32_t threadId() const PURE; + virtual const Thread::ThreadId& threadId() const PURE; virtual MonotonicTime lastTouchTime() const PURE; }; diff --git a/include/envoy/thread/thread.h b/include/envoy/thread/thread.h index 92f6063fafdb..e9078afa476b 100644 --- a/include/envoy/thread/thread.h +++ b/include/envoy/thread/thread.h @@ -10,6 +10,16 @@ namespace Envoy { namespace Thread { +class ThreadId { +public: + virtual ~ThreadId() {} + + virtual std::string debugString() const PURE; + virtual bool isCurrentThreadId() const PURE; +}; + +typedef std::unique_ptr ThreadIdPtr; + class Thread { public: virtual ~Thread() {} @@ -34,6 +44,11 @@ class ThreadFactory { * @param thread_routine supplies the function to invoke in the thread. */ virtual ThreadPtr createThread(std::function thread_routine) PURE; + + /** + * Return the current system thread ID + */ + virtual ThreadIdPtr currentThreadId() PURE; }; /** diff --git a/source/common/api/api_impl.cc b/source/common/api/api_impl.cc index 0fec894a3827..2d2eeeef1b28 100644 --- a/source/common/api/api_impl.cc +++ b/source/common/api/api_impl.cc @@ -16,7 +16,7 @@ Impl::Impl(std::chrono::milliseconds file_flush_interval_msec, file_system_(file_flush_interval_msec, thread_factory, stats_store) {} Event::DispatcherPtr Impl::allocateDispatcher(Event::TimeSystem& time_system) { - return Event::DispatcherPtr{new Event::DispatcherImpl(time_system)}; + return std::make_unique(time_system, *this); } Filesystem::FileSharedPtr Impl::createFile(const std::string& path, Event::Dispatcher& dispatcher, @@ -28,9 +28,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 thread_factory_.createThread(thread_routine); -} +Thread::ThreadFactory& Impl::threadFactory() { return thread_factory_; } } // namespace Api } // namespace Envoy diff --git a/source/common/api/api_impl.h b/source/common/api/api_impl.h index 69cdaf79fef4..4546ea336c98 100644 --- a/source/common/api/api_impl.h +++ b/source/common/api/api_impl.h @@ -27,7 +27,7 @@ class Impl : public Api::Api { Thread::BasicLockable& lock) override; bool fileExists(const std::string& path) override; std::string fileReadToEnd(const std::string& path) override; - Thread::ThreadPtr createThread(std::function thread_routine) override; + Thread::ThreadFactory& threadFactory() override; Filesystem::Instance& fileSystem() { return file_system_; } private: diff --git a/source/common/common/BUILD b/source/common/common/BUILD index 8287d0189bf7..169862b25582 100644 --- a/source/common/common/BUILD +++ b/source/common/common/BUILD @@ -4,6 +4,9 @@ load( "//bazel:envoy_build_system.bzl", "envoy_basic_cc_library", "envoy_cc_library", + "envoy_cc_platform_dep", + "envoy_cc_posix_library", + "envoy_cc_win32_library", "envoy_include_prefix", "envoy_package", "envoy_select_boringssl", @@ -180,12 +183,29 @@ envoy_cc_library( envoy_cc_library( name = "thread_lib", - srcs = ["thread.cc"], hdrs = ["thread.h"], external_deps = ["abseil_synchronization"], + deps = envoy_cc_platform_dep("thread_impl_lib"), +) + +envoy_cc_posix_library( + name = "thread_impl_lib", + srcs = ["posix/thread_impl.cc"], + hdrs = ["posix/thread_impl.h"], + strip_include_prefix = "posix", + deps = [ + ":assert_lib", + "//include/envoy/thread:thread_interface", + ], +) + +envoy_cc_win32_library( + name = "thread_impl_lib", + srcs = ["win32/thread_impl.cc"], + hdrs = ["win32/thread_impl.h"], + strip_include_prefix = "win32", deps = [ ":assert_lib", - ":macros", "//include/envoy/thread:thread_interface", ], ) diff --git a/source/common/common/posix/thread_impl.cc b/source/common/common/posix/thread_impl.cc new file mode 100644 index 000000000000..6421e32f2212 --- /dev/null +++ b/source/common/common/posix/thread_impl.cc @@ -0,0 +1,59 @@ +#include "common/common/assert.h" +#include "common/common/thread_impl.h" + +#if defined(__linux__) +#include +#endif + +namespace Envoy { +namespace Thread { + +namespace { + +int64_t getCurrentThreadId() { +#ifdef __linux__ + return static_cast(syscall(SYS_gettid)); +#elif defined(__APPLE__) + uint64_t tid; + pthread_threadid_np(NULL, &tid); + return tid; +#else +#error "Enable and test pthread id retrieval code for you arch in pthread/thread_impl.cc" +#endif +} + +} // namespace + +ThreadIdImplPosix::ThreadIdImplPosix(int64_t id) : id_(id) {} + +std::string ThreadIdImplPosix::debugString() const { return std::to_string(id_); } + +bool ThreadIdImplPosix::isCurrentThreadId() const { return id_ == getCurrentThreadId(); } + +ThreadImplPosix::ThreadImplPosix(std::function thread_routine) + : thread_routine_(thread_routine) { + RELEASE_ASSERT(Logger::Registry::initialized(), ""); + const int rc = pthread_create(&thread_handle_, nullptr, + [](void* arg) -> void* { + static_cast(arg)->thread_routine_(); + return nullptr; + }, + this); + RELEASE_ASSERT(rc == 0, ""); +} + +void ThreadImplPosix::join() { + const int rc = pthread_join(thread_handle_, nullptr); + RELEASE_ASSERT(rc == 0, ""); +} + +ThreadPtr ThreadFactoryImplPosix::createThread(std::function thread_routine) { + return std::make_unique(thread_routine); +} + +ThreadIdPtr ThreadFactoryImplPosix::currentThreadId() { + return std::make_unique(getCurrentThreadId()); +} + +} // namespace Thread +} // namespace Envoy diff --git a/source/common/common/posix/thread_impl.h b/source/common/common/posix/thread_impl.h new file mode 100755 index 000000000000..aecc59e05b09 --- /dev/null +++ b/source/common/common/posix/thread_impl.h @@ -0,0 +1,51 @@ +#pragma once + +#include + +#include + +#include "envoy/thread/thread.h" + +namespace Envoy { +namespace Thread { + +class ThreadIdImplPosix : public ThreadId { +public: + ThreadIdImplPosix(int64_t id); + + // Thread::ThreadId + std::string debugString() const override; + bool isCurrentThreadId() const override; + +private: + int64_t id_; +}; + +/** + * Wrapper for a pthread thread. We don't use std::thread because it eats exceptions and leads to + * unusable stack traces. + */ +class ThreadImplPosix : public Thread { +public: + ThreadImplPosix(std::function thread_routine); + + // Thread::Thread + void join() override; + +private: + std::function thread_routine_; + pthread_t thread_handle_; +}; + +/** + * Implementation of ThreadFactory + */ +class ThreadFactoryImplPosix : public ThreadFactory { +public: + // Thread::ThreadFactory + ThreadPtr createThread(std::function thread_routine) override; + ThreadIdPtr currentThreadId() override; +}; + +} // namespace Thread +} // namespace Envoy diff --git a/source/common/common/stack_array.h b/source/common/common/stack_array.h index a7e93f01a3d3..21e2c7aa97b5 100644 --- a/source/common/common/stack_array.h +++ b/source/common/common/stack_array.h @@ -1,10 +1,10 @@ #pragma once -#if !defined(WIN32) -#include +#ifdef WIN32 +#include #else -#include +#include #endif #include diff --git a/source/common/common/thread.cc b/source/common/common/thread.cc deleted file mode 100644 index 9f3ef5e9f660..000000000000 --- a/source/common/common/thread.cc +++ /dev/null @@ -1,61 +0,0 @@ -#include "common/common/thread.h" - -#ifdef __linux__ -#include -#elif defined(__APPLE__) -#include -#endif - -#include - -#include "common/common/assert.h" -#include "common/common/macros.h" - -namespace Envoy { -namespace Thread { - -/** - * 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() { -#ifdef __linux__ - return syscall(SYS_gettid); -#elif defined(__APPLE__) - uint64_t tid; - pthread_threadid_np(NULL, &tid); - return static_cast(tid); -#else -#error "Enable and test pthread id retrieval code for you arch in thread.cc" -#endif -} - -} // namespace Thread -} // namespace Envoy diff --git a/source/common/common/thread.h b/source/common/common/thread.h index 191500ab3e41..071699fc49a4 100644 --- a/source/common/common/thread.h +++ b/source/common/common/thread.h @@ -10,23 +10,6 @@ namespace Envoy { namespace Thread { -typedef int32_t ThreadId; - -/** - * Get current thread id. - */ -ThreadId currentThreadId(); - -/** - * Implementation of ThreadFactory - */ -class ThreadFactoryImpl : public ThreadFactory { -public: - ThreadFactoryImpl() {} - - ThreadPtr createThread(std::function thread_routine) override; -}; - /** * Implementation of BasicLockable */ diff --git a/source/common/common/win32/thread_impl.cc b/source/common/common/win32/thread_impl.cc new file mode 100644 index 000000000000..28bd8b189d21 --- /dev/null +++ b/source/common/common/win32/thread_impl.cc @@ -0,0 +1,44 @@ +#include + +#include "common/common/assert.h" +#include "common/common/thread_impl.h" + +namespace Envoy { +namespace Thread { + +ThreadIdImplWin32::ThreadIdImplWin32(DWORD id) : id_(id) {} + +std::string ThreadIdImplWin32::debugString() const { return std::to_string(id_); } + +bool ThreadIdImplWin32::isCurrentThreadId() const { return id_ == ::GetCurrentThreadId(); } + +ThreadImplWin32::ThreadImplWin32(std::function thread_routine) + : thread_routine_(thread_routine) { + RELEASE_ASSERT(Logger::Registry::initialized(), ""); + thread_handle_ = reinterpret_cast( + ::_beginthreadex(nullptr, 0, + [](void* arg) -> unsigned int { + static_cast(arg)->thread_routine_(); + return 0; + }, + this, 0, nullptr)); + RELEASE_ASSERT(thread_handle_ != 0, ""); +} + +ThreadImplWin32::~ThreadImplWin32() { ::CloseHandle(thread_handle_); } + +void ThreadImplWin32::join() { + const DWORD rc = ::WaitForSingleObject(thread_handle_, INFINITE); + RELEASE_ASSERT(rc == WAIT_OBJECT_0, ""); +} + +ThreadPtr ThreadFactoryImplWin32::createThread(std::function thread_routine) { + return std::make_unique(thread_routine); +} + +ThreadIdPtr ThreadFactoryImplWin32::currentThreadId() { + return std::make_unique(::GetCurrentThreadId()); +} + +} // namespace Thread +} // namespace Envoy diff --git a/source/common/common/win32/thread_impl.h b/source/common/common/win32/thread_impl.h new file mode 100644 index 000000000000..cd3821900f85 --- /dev/null +++ b/source/common/common/win32/thread_impl.h @@ -0,0 +1,56 @@ +#pragma once + +#include + +// defines some macros that interfere with our code, so undef them +#undef DELETE +#undef GetMessage + +#include + +#include "envoy/thread/thread.h" + +namespace Envoy { +namespace Thread { + +class ThreadIdImplWin32 : public ThreadId { +public: + ThreadIdImplWin32(DWORD id); + + // Thread::ThreadId + std::string debugString() const override; + bool isCurrentThreadId() const override; + +private: + DWORD id_; +}; + +/** + * Wrapper for a win32 thread. We don't use std::thread because it eats exceptions and leads to + * unusable stack traces. + */ +class ThreadImplWin32 : public Thread { +public: + ThreadImplWin32(std::function thread_routine); + ~ThreadImplWin32(); + + // Thread::Thread + void join() override; + +private: + std::function thread_routine_; + HANDLE thread_handle_; +}; + +/** + * Implementation of ThreadFactory + */ +class ThreadFactoryImplWin32 : public ThreadFactory { +public: + // Thread::ThreadFactory + ThreadPtr createThread(std::function thread_routine) override; + ThreadIdPtr currentThreadId() override; +}; + +} // namespace Thread +} // namespace Envoy diff --git a/source/common/event/BUILD b/source/common/event/BUILD index a980b72e8b29..00afdf22613e 100644 --- a/source/common/event/BUILD +++ b/source/common/event/BUILD @@ -63,6 +63,7 @@ envoy_cc_library( ], deps = [ ":libevent_lib", + "//include/envoy/api:api_interface", "//include/envoy/event:deferred_deletable", "//include/envoy/event:dispatcher_interface", "//include/envoy/event:file_event_interface", diff --git a/source/common/event/dispatched_thread.cc b/source/common/event/dispatched_thread.cc index 04285fa4dede..c215ff56cbca 100644 --- a/source/common/event/dispatched_thread.cc +++ b/source/common/event/dispatched_thread.cc @@ -14,7 +14,8 @@ namespace Envoy { namespace Event { void DispatchedThreadImpl::start(Server::GuardDog& guard_dog) { - thread_ = api_.createThread([this, &guard_dog]() -> void { threadRoutine(guard_dog); }); + thread_ = + api_.threadFactory().createThread([this, &guard_dog]() -> void { threadRoutine(guard_dog); }); } void DispatchedThreadImpl::exit() { @@ -26,7 +27,7 @@ void DispatchedThreadImpl::exit() { void DispatchedThreadImpl::threadRoutine(Server::GuardDog& guard_dog) { ENVOY_LOG(debug, "dispatched thread entering dispatch loop"); - auto watchdog = guard_dog.createWatchDog(Thread::currentThreadId()); + auto watchdog = guard_dog.createWatchDog(api_.threadFactory().currentThreadId()); watchdog->startWatchdog(*dispatcher_); dispatcher_->run(Dispatcher::RunType::Block); ENVOY_LOG(debug, "dispatched thread exited dispatch loop"); diff --git a/source/common/event/dispatched_thread.h b/source/common/event/dispatched_thread.h index ed6aca9d59cd..1f3aaf86c41d 100644 --- a/source/common/event/dispatched_thread.h +++ b/source/common/event/dispatched_thread.h @@ -40,7 +40,7 @@ namespace Event { class DispatchedThreadImpl : Logger::Loggable { public: DispatchedThreadImpl(Api::Api& api, TimeSystem& time_system) - : api_(api), dispatcher_(new DispatcherImpl(time_system)) {} + : api_(api), dispatcher_(new DispatcherImpl(time_system, api_)) {} /** * Start the thread. diff --git a/source/common/event/dispatcher_impl.cc b/source/common/event/dispatcher_impl.cc index bb4f6af5330f..be1134964e91 100644 --- a/source/common/event/dispatcher_impl.cc +++ b/source/common/event/dispatcher_impl.cc @@ -6,6 +6,7 @@ #include #include +#include "envoy/api/api.h" #include "envoy/network/listen_socket.h" #include "envoy/network/listener.h" @@ -24,15 +25,17 @@ namespace Envoy { namespace Event { -DispatcherImpl::DispatcherImpl(TimeSystem& time_system) - : DispatcherImpl(time_system, Buffer::WatermarkFactoryPtr{new Buffer::WatermarkBufferFactory}) { +DispatcherImpl::DispatcherImpl(TimeSystem& time_system, Api::Api& api) + : DispatcherImpl(time_system, Buffer::WatermarkFactoryPtr{new Buffer::WatermarkBufferFactory}, + api) { // The dispatcher won't work as expected if libevent hasn't been configured to use threads. RELEASE_ASSERT(Libevent::Global::initialized(), ""); } -DispatcherImpl::DispatcherImpl(TimeSystem& time_system, Buffer::WatermarkFactoryPtr&& factory) - : time_system_(time_system), buffer_factory_(std::move(factory)), base_(event_base_new()), - scheduler_(time_system_.createScheduler(base_)), +DispatcherImpl::DispatcherImpl(TimeSystem& time_system, Buffer::WatermarkFactoryPtr&& factory, + Api::Api& api) + : api_(api), time_system_(time_system), buffer_factory_(std::move(factory)), + base_(event_base_new()), scheduler_(time_system_.createScheduler(base_)), deferred_delete_timer_(createTimer([this]() -> void { clearDeferredDeleteList(); })), post_timer_(createTimer([this]() -> void { runPostCallbacks(); })), current_to_delete_(&to_delete_1_) { @@ -151,7 +154,7 @@ void DispatcherImpl::post(std::function callback) { } void DispatcherImpl::run(RunType type) { - run_tid_ = Thread::currentThreadId(); + run_tid_ = api_.threadFactory().currentThreadId(); // Flush all post callbacks before we run the event loop. We do this because there are post // callbacks that have to get run before the initial event loop starts running. libevent does diff --git a/source/common/event/dispatcher_impl.h b/source/common/event/dispatcher_impl.h index 08217d7d2aab..f8edabdc7218 100644 --- a/source/common/event/dispatcher_impl.h +++ b/source/common/event/dispatcher_impl.h @@ -5,6 +5,7 @@ #include #include +#include "envoy/api/api.h" #include "envoy/common/time.h" #include "envoy/event/deferred_deletable.h" #include "envoy/event/dispatcher.h" @@ -22,8 +23,8 @@ namespace Event { */ class DispatcherImpl : Logger::Loggable, public Dispatcher { public: - explicit DispatcherImpl(TimeSystem& time_system); - DispatcherImpl(TimeSystem& time_system, Buffer::WatermarkFactoryPtr&& factory); + explicit DispatcherImpl(TimeSystem& time_system, Api::Api& api); + DispatcherImpl(TimeSystem& time_system, Buffer::WatermarkFactoryPtr&& factory, Api::Api& api); ~DispatcherImpl(); /** @@ -62,12 +63,13 @@ class DispatcherImpl : Logger::Loggable, public Dispatcher { void runPostCallbacks(); // Validate that an operation is thread safe, i.e. it's invoked on the same thread that the - // dispatcher run loop is executing on. We allow run_tid_ == 0 for tests where we don't invoke - // run(). - bool isThreadSafe() const { return run_tid_ == 0 || run_tid_ == Thread::currentThreadId(); } + // dispatcher run loop is executing on. We allow run_tid_ == nullptr for tests where we don't + // invoke run(). + bool isThreadSafe() const { return run_tid_ == nullptr || run_tid_->isCurrentThreadId(); } + Api::Api& api_; TimeSystem& time_system_; - Thread::ThreadId run_tid_{}; + Thread::ThreadIdPtr run_tid_; Buffer::WatermarkFactoryPtr buffer_factory_; Libevent::BasePtr base_; SchedulerPtr scheduler_; diff --git a/source/common/grpc/google_async_client_impl.cc b/source/common/grpc/google_async_client_impl.cc index b8d39b84a1ba..2ea310b49ba3 100644 --- a/source/common/grpc/google_async_client_impl.cc +++ b/source/common/grpc/google_async_client_impl.cc @@ -14,7 +14,7 @@ namespace Envoy { namespace Grpc { GoogleAsyncClientThreadLocal::GoogleAsyncClientThreadLocal(Api::Api& api) - : completion_thread_(api.createThread([this] { completionThread(); })) {} + : completion_thread_(api.threadFactory().createThread([this] { completionThread(); })) {} GoogleAsyncClientThreadLocal::~GoogleAsyncClientThreadLocal() { // Force streams to shutdown and invoke TryCancel() to start the drain of diff --git a/source/common/grpc/google_async_client_impl.h b/source/common/grpc/google_async_client_impl.h index 61f97d632fdc..71a22e2efde6 100644 --- a/source/common/grpc/google_async_client_impl.h +++ b/source/common/grpc/google_async_client_impl.h @@ -10,6 +10,7 @@ #include "envoy/tracing/http_tracer.h" #include "common/common/linked_object.h" +#include "common/common/thread.h" #include "common/common/thread_annotations.h" #include "common/tracing/http_tracer_impl.h" diff --git a/source/common/singleton/manager_impl.cc b/source/common/singleton/manager_impl.cc index 1ad2b5569270..9023b2b744ca 100644 --- a/source/common/singleton/manager_impl.cc +++ b/source/common/singleton/manager_impl.cc @@ -9,7 +9,7 @@ namespace Envoy { namespace Singleton { InstanceSharedPtr ManagerImpl::get(const std::string& name, SingletonFactoryCb cb) { - ASSERT(run_tid_ == Thread::currentThreadId()); + ASSERT(run_tid_->isCurrentThreadId()); if (nullptr == Registry::FactoryRegistry::getFactory(name)) { PANIC(fmt::format("invalid singleton name '{}'. Make sure it is registered.", name)); } diff --git a/source/common/singleton/manager_impl.h b/source/common/singleton/manager_impl.h index 58ee64b77bec..9625087683d2 100644 --- a/source/common/singleton/manager_impl.h +++ b/source/common/singleton/manager_impl.h @@ -3,8 +3,7 @@ #include #include "envoy/singleton/manager.h" - -#include "common/common/thread.h" +#include "envoy/thread/thread.h" namespace Envoy { namespace Singleton { @@ -16,14 +15,14 @@ namespace Singleton { */ class ManagerImpl : public Manager { public: - ManagerImpl() : run_tid_(Thread::currentThreadId()) {} + ManagerImpl(Thread::ThreadIdPtr&& thread_id) : run_tid_(std::move(thread_id)) {} // Singleton::Manager InstanceSharedPtr get(const std::string& name, SingletonFactoryCb cb) override; private: std::unordered_map> singletons_; - Thread::ThreadId run_tid_{}; + Thread::ThreadIdPtr run_tid_; }; } // namespace Singleton diff --git a/source/exe/BUILD b/source/exe/BUILD index 52064272fa0a..cfe3b1714354 100644 --- a/source/exe/BUILD +++ b/source/exe/BUILD @@ -4,6 +4,9 @@ load( "//bazel:envoy_build_system.bzl", "envoy_cc_binary", "envoy_cc_library", + "envoy_cc_platform_dep", + "envoy_cc_posix_library", + "envoy_cc_win32_library", "envoy_package", ) load( @@ -54,7 +57,7 @@ envoy_cc_library( ], deps = [ ":envoy_main_common_lib", - ], + ] + envoy_cc_platform_dep("platform_impl_lib"), ) envoy_cc_library( @@ -76,7 +79,24 @@ envoy_cc_library( ":sigaction_lib", ":terminate_handler_lib", ], - }), + }) + envoy_cc_platform_dep("platform_impl_lib"), +) + +envoy_cc_posix_library( + name = "platform_impl_lib", + hdrs = ["posix/platform_impl.h"], + strip_include_prefix = "posix", + deps = ["//source/common/common:thread_lib"], +) + +envoy_cc_win32_library( + name = "platform_impl_lib", + hdrs = ["win32/platform_impl.h"], + strip_include_prefix = "win32", + deps = [ + "//source/common/common:assert_lib", + "//source/common/common:thread_lib", + ], ) envoy_cc_library( diff --git a/source/exe/main_common.cc b/source/exe/main_common.cc index 6a7c5b611c2d..4291aca8d943 100644 --- a/source/exe/main_common.cc +++ b/source/exe/main_common.cc @@ -128,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(), thread_factory_) {} + std::make_unique(), platform_impl_.threadFactory()) {} std::string MainCommon::hotRestartVersion(uint64_t max_num_stats, uint64_t max_stat_name_len, bool hot_restart_enabled) { diff --git a/source/exe/main_common.h b/source/exe/main_common.h index 9002f70275ab..d67acce58985 100644 --- a/source/exe/main_common.h +++ b/source/exe/main_common.h @@ -8,6 +8,8 @@ #include "common/stats/thread_local_store.h" #include "common/thread_local/thread_local_impl.h" +#include "exe/platform_impl.h" + #include "server/options_impl.h" #include "server/server.h" #include "server/test_hooks.h" @@ -106,11 +108,11 @@ class MainCommon { Envoy::TerminateHandler log_on_terminate; #endif + PlatformImpl platform_impl_; Envoy::OptionsImpl options_; Event::RealTimeSystem real_time_system_; DefaultTestHooks default_test_hooks_; ProdComponentFactory prod_component_factory_; - Thread::ThreadFactoryImpl thread_factory_; MainCommonBase base_; }; diff --git a/source/exe/posix/platform_impl.h b/source/exe/posix/platform_impl.h new file mode 100644 index 000000000000..f43f765172a1 --- /dev/null +++ b/source/exe/posix/platform_impl.h @@ -0,0 +1,16 @@ +#pragma once + +#include "common/common/macros.h" +#include "common/common/thread_impl.h" + +namespace Envoy { + +class PlatformImpl { +public: + Thread::ThreadFactory& threadFactory() { return thread_factory_; } + +private: + Thread::ThreadFactoryImplPosix thread_factory_; +}; + +} // namespace Envoy \ No newline at end of file diff --git a/source/exe/win32/platform_impl.h b/source/exe/win32/platform_impl.h new file mode 100644 index 000000000000..b69b2f5b1be1 --- /dev/null +++ b/source/exe/win32/platform_impl.h @@ -0,0 +1,29 @@ +#pragma once + +#include "common/common/assert.h" +#include "common/common/thread_impl.h" + +// clang-format off +#include +// clang-format on + +namespace Envoy { + +class PlatformImpl { +public: + PlatformImpl() { + const WORD wVersionRequested = MAKEWORD(2, 2); + WSADATA wsaData; + const int rc = ::WSAStartup(wVersionRequested, &wsaData); + RELEASE_ASSERT(rc == 0, "WSAStartup failed with error"); + } + + ~PlatformImpl() { ::WSACleanup(); } + + Thread::ThreadFactory& threadFactory() { return thread_factory_; } + +private: + Thread::ThreadFactoryImplWin32 thread_factory_; +}; + +} // namespace Envoy \ No newline at end of file diff --git a/source/server/config_validation/api.cc b/source/server/config_validation/api.cc index 89973e735fd0..14b52e6e8e65 100644 --- a/source/server/config_validation/api.cc +++ b/source/server/config_validation/api.cc @@ -10,7 +10,7 @@ ValidationImpl::ValidationImpl(std::chrono::milliseconds file_flush_interval_mse : Impl(file_flush_interval_msec, thread_factory, stats_store) {} Event::DispatcherPtr ValidationImpl::allocateDispatcher(Event::TimeSystem& time_system) { - return Event::DispatcherPtr{new Event::ValidationDispatcher(time_system)}; + return Event::DispatcherPtr{new Event::ValidationDispatcher(time_system, *this)}; } } // namespace Api diff --git a/source/server/config_validation/async_client.cc b/source/server/config_validation/async_client.cc index a39adf4881f3..9d04323d5fe3 100644 --- a/source/server/config_validation/async_client.cc +++ b/source/server/config_validation/async_client.cc @@ -3,8 +3,8 @@ namespace Envoy { namespace Http { -ValidationAsyncClient::ValidationAsyncClient(Event::TimeSystem& time_system) - : dispatcher_(time_system) {} +ValidationAsyncClient::ValidationAsyncClient(Event::TimeSystem& time_system, Api::Api& api) + : dispatcher_(time_system, api) {} AsyncClient::Request* ValidationAsyncClient::send(MessagePtr&&, Callbacks&, const RequestOptions&) { return nullptr; diff --git a/source/server/config_validation/async_client.h b/source/server/config_validation/async_client.h index 9f1dd010ee5d..29b533c013d3 100644 --- a/source/server/config_validation/async_client.h +++ b/source/server/config_validation/async_client.h @@ -19,7 +19,7 @@ namespace Http { */ class ValidationAsyncClient : public AsyncClient { public: - ValidationAsyncClient(Event::TimeSystem& time_system); + ValidationAsyncClient(Event::TimeSystem& time_system, Api::Api& api); // Http::AsyncClient AsyncClient::Request* send(MessagePtr&& request, Callbacks& callbacks, diff --git a/source/server/config_validation/cluster_manager.cc b/source/server/config_validation/cluster_manager.cc index dbf558e1e69e..e2c1ddc934b6 100644 --- a/source/server/config_validation/cluster_manager.cc +++ b/source/server/config_validation/cluster_manager.cc @@ -42,7 +42,7 @@ ValidationClusterManager::ValidationClusterManager( Server::Admin& admin, Api::Api& api, Http::Context& http_context) : ClusterManagerImpl(bootstrap, factory, stats, tls, runtime, random, local_info, log_manager, main_thread_dispatcher, admin, api, http_context), - async_client_(main_thread_dispatcher.timeSystem()) {} + async_client_(main_thread_dispatcher.timeSystem(), api) {} Http::ConnectionPool::Instance* ValidationClusterManager::httpConnPoolForCluster(const std::string&, ResourcePriority, diff --git a/source/server/config_validation/dispatcher.h b/source/server/config_validation/dispatcher.h index 8e51d4b02afc..a30addebe8a5 100644 --- a/source/server/config_validation/dispatcher.h +++ b/source/server/config_validation/dispatcher.h @@ -16,7 +16,7 @@ namespace Event { */ class ValidationDispatcher : public DispatcherImpl { public: - ValidationDispatcher(TimeSystem& time_system) : DispatcherImpl(time_system) {} + ValidationDispatcher(TimeSystem& time_system, Api::Api& api) : DispatcherImpl(time_system, api) {} Network::ClientConnectionPtr createClientConnection(Network::Address::InstanceConstSharedPtr, diff --git a/source/server/config_validation/server.cc b/source/server/config_validation/server.cc index 37cd94dde6bc..3de88d46be41 100644 --- a/source/server/config_validation/server.cc +++ b/source/server/config_validation/server.cc @@ -43,7 +43,7 @@ ValidationInstance::ValidationInstance(Options& options, Event::TimeSystem& time : options_(options), time_system_(time_system), stats_store_(store), api_(new Api::ValidationImpl(options.fileFlushIntervalMsec(), thread_factory, store)), dispatcher_(api_->allocateDispatcher(time_system)), - singleton_manager_(new Singleton::ManagerImpl()), + singleton_manager_(new Singleton::ManagerImpl(api_->threadFactory().currentThreadId())), access_log_manager_(*api_, *dispatcher_, access_log_lock), mutex_tracer_(nullptr) { try { initialize(options, local_address, component_factory); diff --git a/source/server/guarddog_impl.cc b/source/server/guarddog_impl.cc index cf392393fc57..ecae1349b9a4 100644 --- a/source/server/guarddog_impl.cc +++ b/source/server/guarddog_impl.cc @@ -66,14 +66,14 @@ void GuardDogImpl::threadRoutine() { } if (killEnabled() && delta > kill_timeout_) { PANIC(fmt::format("GuardDog: one thread ({}) stuck for more than watchdog_kill_timeout", - watched_dog.dog_->threadId())); + watched_dog.dog_->threadId().debugString())); } if (multikillEnabled() && delta > multi_kill_timeout_) { if (seen_one_multi_timeout) { PANIC(fmt::format( "GuardDog: multiple threads ({},...) stuck for more than watchdog_multikill_timeout", - watched_dog.dog_->threadId())); + watched_dog.dog_->threadId().debugString())); } else { seen_one_multi_timeout = true; } @@ -82,14 +82,14 @@ void GuardDogImpl::threadRoutine() { } while (waitOrDetectStop()); } -WatchDogSharedPtr GuardDogImpl::createWatchDog(int32_t thread_id) { +WatchDogSharedPtr GuardDogImpl::createWatchDog(Thread::ThreadIdPtr&& thread_id) { // Timer started by WatchDog will try to fire at 1/2 of the interval of the // minimum timeout specified. loop_interval_ is const so all shared state // accessed out of the locked section below is const (time_system_ has no // state). auto wd_interval = loop_interval_ / 2; WatchDogSharedPtr new_watchdog = - std::make_shared(thread_id, time_system_, wd_interval); + std::make_shared(std::move(thread_id), time_system_, wd_interval); WatchedDog watched_dog; watched_dog.dog_ = new_watchdog; { @@ -137,7 +137,7 @@ bool GuardDogImpl::waitOrDetectStop() { void GuardDogImpl::start(Api::Api& api) { run_thread_ = true; - thread_ = api.createThread([this]() -> void { threadRoutine(); }); + thread_ = api.threadFactory().createThread([this]() -> void { threadRoutine(); }); } void GuardDogImpl::stop() { diff --git a/source/server/guarddog_impl.h b/source/server/guarddog_impl.h index bcc3f58572a5..27f50eade905 100644 --- a/source/server/guarddog_impl.h +++ b/source/server/guarddog_impl.h @@ -10,10 +10,10 @@ #include "envoy/server/watchdog.h" #include "envoy/stats/scope.h" #include "envoy/stats/stats.h" -#include "envoy/thread/thread.h" #include "common/common/lock_guard.h" #include "common/common/logger.h" +#include "common/common/thread.h" #include "common/event/libevent.h" #include "absl/types/optional.h" @@ -54,7 +54,7 @@ class GuardDogImpl : public GuardDog { } // Server::GuardDog - WatchDogSharedPtr createWatchDog(int32_t thread_id) override; + WatchDogSharedPtr createWatchDog(Thread::ThreadIdPtr&& thread_id) override; void stopWatching(WatchDogSharedPtr wd) override; private: diff --git a/source/server/server.cc b/source/server/server.cc index 8c282651e8db..e8353cdb0770 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -58,7 +58,7 @@ InstanceImpl::InstanceImpl(Options& options, Event::TimeSystem& time_system, api_(new Api::Impl(options.fileFlushIntervalMsec(), thread_factory, store)), secret_manager_(std::make_unique()), dispatcher_(api_->allocateDispatcher(time_system)), - singleton_manager_(new Singleton::ManagerImpl()), + singleton_manager_(new Singleton::ManagerImpl(api_->threadFactory().currentThreadId())), handler_(new ConnectionHandlerImpl(ENVOY_LOGGER(), *dispatcher_)), random_generator_(std::move(random_generator)), listener_component_factory_(*this), worker_factory_(thread_local_, *api_, hooks, time_system), @@ -461,7 +461,7 @@ void InstanceImpl::run() { // Run the main dispatch loop waiting to exit. ENVOY_LOG(info, "starting main dispatch loop"); - auto watchdog = guard_dog_->createWatchDog(Thread::currentThreadId()); + auto watchdog = guard_dog_->createWatchDog(api_->threadFactory().currentThreadId()); watchdog->startWatchdog(*dispatcher_); dispatcher_->run(Event::Dispatcher::RunType::Block); ENVOY_LOG(info, "main dispatch loop exited"); diff --git a/source/server/watchdog_impl.h b/source/server/watchdog_impl.h index dd0bd7fe580f..b4a4db1340af 100644 --- a/source/server/watchdog_impl.h +++ b/source/server/watchdog_impl.h @@ -17,15 +17,15 @@ namespace Server { class WatchDogImpl : public WatchDog { public: /** - * @param thread_id A system thread ID (such as from Thread::currentThreadId()) * @param interval WatchDog timer interval (used after startWatchdog()) */ - WatchDogImpl(int32_t thread_id, TimeSource& tsource, std::chrono::milliseconds interval) - : thread_id_(thread_id), time_source_(tsource), + WatchDogImpl(Thread::ThreadIdPtr&& thread_id, TimeSource& tsource, + std::chrono::milliseconds interval) + : thread_id_(std::move(thread_id)), time_source_(tsource), latest_touch_time_since_epoch_(tsource.monotonicTime().time_since_epoch()), timer_interval_(interval) {} - int32_t threadId() const override { return thread_id_; } + const Thread::ThreadId& threadId() const override { return *thread_id_; } MonotonicTime lastTouchTime() const override { return MonotonicTime(latest_touch_time_since_epoch_.load()); } @@ -37,7 +37,7 @@ class WatchDogImpl : public WatchDog { } private: - const int32_t thread_id_; + Thread::ThreadIdPtr thread_id_; TimeSource& time_source_; std::atomic latest_touch_time_since_epoch_; Event::TimerPtr timer_; diff --git a/source/server/worker_impl.cc b/source/server/worker_impl.cc index edbecc8c967c..a7ec17545405 100644 --- a/source/server/worker_impl.cc +++ b/source/server/worker_impl.cc @@ -71,7 +71,8 @@ void WorkerImpl::removeListener(Network::ListenerConfig& listener, void WorkerImpl::start(GuardDog& guard_dog) { ASSERT(!thread_); - thread_ = api_.createThread([this, &guard_dog]() -> void { threadRoutine(guard_dog); }); + thread_ = + api_.threadFactory().createThread([this, &guard_dog]() -> void { threadRoutine(guard_dog); }); } void WorkerImpl::stop() { @@ -96,7 +97,7 @@ void WorkerImpl::stopListeners() { void WorkerImpl::threadRoutine(GuardDog& guard_dog) { ENVOY_LOG(debug, "worker entering dispatch loop"); - auto watchdog = guard_dog.createWatchDog(Thread::currentThreadId()); + auto watchdog = guard_dog.createWatchDog(api_.threadFactory().currentThreadId()); watchdog->startWatchdog(*dispatcher_); dispatcher_->run(Event::Dispatcher::RunType::Block); ENVOY_LOG(debug, "worker exited dispatch loop"); diff --git a/test/common/config/filesystem_subscription_test_harness.h b/test/common/config/filesystem_subscription_test_harness.h index 4193a4393fc6..f53f3533376f 100644 --- a/test/common/config/filesystem_subscription_test_harness.h +++ b/test/common/config/filesystem_subscription_test_harness.h @@ -30,7 +30,8 @@ typedef FilesystemSubscriptionImpl class FilesystemSubscriptionTestHarness : public SubscriptionTestHarness { public: FilesystemSubscriptionTestHarness() - : path_(TestEnvironment::temporaryPath("eds.json")), dispatcher_(test_time_.timeSystem()), + : path_(TestEnvironment::temporaryPath("eds.json")), + api_(Api::createApiForTest(stats_store_)), dispatcher_(test_time_.timeSystem(), *api_), subscription_(dispatcher_, path_, stats_) {} ~FilesystemSubscriptionTestHarness() { EXPECT_EQ(0, ::unlink(path_.c_str())); } @@ -97,6 +98,8 @@ class FilesystemSubscriptionTestHarness : public SubscriptionTestHarness { const std::string path_; std::string version_; + Stats::IsolatedStoreImpl stats_store_; + Api::ApiPtr api_; DangerousDeprecatedTestTime test_time_; Event::DispatcherImpl dispatcher_; NiceMock> callbacks_; diff --git a/test/common/event/BUILD b/test/common/event/BUILD index d29918714b6a..2ecbb8a21354 100644 --- a/test/common/event/BUILD +++ b/test/common/event/BUILD @@ -29,6 +29,7 @@ envoy_cc_test( "//include/envoy/event:file_event_interface", "//source/common/event:dispatcher_includes", "//source/common/event:dispatcher_lib", + "//source/common/stats:isolated_store_lib", "//test/mocks:common_lib", "//test/test_common:environment_lib", "//test/test_common:test_time_lib", diff --git a/test/common/event/dispatcher_impl_test.cc b/test/common/event/dispatcher_impl_test.cc index a61ce53064c0..1d2d6b54a533 100644 --- a/test/common/event/dispatcher_impl_test.cc +++ b/test/common/event/dispatcher_impl_test.cc @@ -30,8 +30,10 @@ class TestDeferredDeletable : public DeferredDeletable { TEST(DeferredDeleteTest, DeferredDelete) { InSequence s; + Stats::IsolatedStoreImpl stats_store; + Api::ApiPtr api = Api::createApiForTest(stats_store); DangerousDeprecatedTestTime test_time; - DispatcherImpl dispatcher(test_time.timeSystem()); + DispatcherImpl dispatcher(test_time.timeSystem(), *api); ReadyWatcher watcher1; dispatcher.deferredDelete( @@ -63,9 +65,9 @@ class DispatcherImplTest : public ::testing::Test { protected: DispatcherImplTest() : api_(Api::createApiForTest(stat_store_)), - dispatcher_(std::make_unique(test_time_.timeSystem())), + dispatcher_(std::make_unique(test_time_.timeSystem(), *api_)), work_finished_(false) { - dispatcher_thread_ = api_->createThread([this]() { + dispatcher_thread_ = api_->threadFactory().createThread([this]() { // Must create a keepalive timer to keep the dispatcher from exiting. std::chrono::milliseconds time_interval(500); keepalive_timer_ = dispatcher_->createTimer( diff --git a/test/common/event/file_event_impl_test.cc b/test/common/event/file_event_impl_test.cc index 48542baf42cb..b9297a730991 100644 --- a/test/common/event/file_event_impl_test.cc +++ b/test/common/event/file_event_impl_test.cc @@ -3,6 +3,7 @@ #include "envoy/event/file_event.h" #include "common/event/dispatcher_impl.h" +#include "common/stats/isolated_store_impl.h" #include "test/mocks/common.h" #include "test/test_common/environment.h" @@ -16,7 +17,8 @@ namespace Event { class FileEventImplTest : public testing::Test { public: - FileEventImplTest() : dispatcher_(test_time_.timeSystem()) {} + FileEventImplTest() + : api_(Api::createApiForTest(stats_store_)), dispatcher_(test_time_.timeSystem(), *api_) {} void SetUp() override { int rc = socketpair(AF_UNIX, SOCK_DGRAM, 0, fds_); @@ -33,6 +35,8 @@ class FileEventImplTest : public testing::Test { protected: int fds_[2]; + Stats::IsolatedStoreImpl stats_store_; + Api::ApiPtr api_; DangerousDeprecatedTestTime test_time_; DispatcherImpl dispatcher_; }; @@ -53,7 +57,9 @@ TEST_P(FileEventImplActivateTest, Activate) { ASSERT_NE(-1, fd); DangerousDeprecatedTestTime test_time; - DispatcherImpl dispatcher(test_time.timeSystem()); + Stats::IsolatedStoreImpl stats_store; + Api::ApiPtr api = Api::createApiForTest(stats_store); + DispatcherImpl dispatcher(test_time.timeSystem(), *api); ReadyWatcher read_event; EXPECT_CALL(read_event, ready()).Times(1); ReadyWatcher write_event; diff --git a/test/common/filesystem/BUILD b/test/common/filesystem/BUILD index c797d6c72f92..9538338b6614 100644 --- a/test/common/filesystem/BUILD +++ b/test/common/filesystem/BUILD @@ -46,6 +46,7 @@ envoy_cc_test( "//source/common/event:dispatcher_includes", "//source/common/event:dispatcher_lib", "//source/common/filesystem:watcher_lib", + "//source/common/stats:isolated_store_lib", "//test/test_common:environment_lib", "//test/test_common:test_time_lib", ], diff --git a/test/common/filesystem/directory_test.cc b/test/common/filesystem/directory_test.cc index eb36ff7b8e77..5ef056e63603 100644 --- a/test/common/filesystem/directory_test.cc +++ b/test/common/filesystem/directory_test.cc @@ -198,23 +198,23 @@ TEST_F(DirectoryTest, DirectoryWithEmptyDirectory) { TEST(DirectoryIteratorImpl, NonExistingDir) { const std::string dir_path("some/non/existing/dir"); -#if !defined(WIN32) +#ifdef WIN32 EXPECT_THROW_WITH_MESSAGE( DirectoryIteratorImpl dir_iterator(dir_path), EnvoyException, - fmt::format("unable to open directory {}: No such file or directory", dir_path)); + fmt::format("unable to open directory {}: {}", dir_path, ERROR_PATH_NOT_FOUND)); #else EXPECT_THROW_WITH_MESSAGE( DirectoryIteratorImpl dir_iterator(dir_path), EnvoyException, - fmt::format("unable to open directory {}: {}", dir_path, ERROR_PATH_NOT_FOUND)); + fmt::format("unable to open directory {}: No such file or directory", dir_path)); #endif } // Test that we correctly handle trailing path separators TEST(Directory, DirectoryHasTrailingPathSeparator) { -#if !defined(WIN32) - const std::string dir_path(TestEnvironment::temporaryPath("envoy_test") + "/"); -#else +#ifdef WIN32 const std::string dir_path(TestEnvironment::temporaryPath("envoy_test") + "\\"); +#else + const std::string dir_path(TestEnvironment::temporaryPath("envoy_test") + "/"); #endif TestUtility::createDirectory(dir_path); diff --git a/test/common/filesystem/watcher_impl_test.cc b/test/common/filesystem/watcher_impl_test.cc index 430c33337381..0f90304f001f 100644 --- a/test/common/filesystem/watcher_impl_test.cc +++ b/test/common/filesystem/watcher_impl_test.cc @@ -4,6 +4,7 @@ #include "common/common/assert.h" #include "common/event/dispatcher_impl.h" #include "common/filesystem/watcher_impl.h" +#include "common/stats/isolated_store_impl.h" #include "test/test_common/environment.h" #include "test/test_common/test_time.h" @@ -17,8 +18,11 @@ namespace Filesystem { class WatcherImplTest : public testing::Test { protected: - WatcherImplTest() : dispatcher_(test_time_.timeSystem()) {} + WatcherImplTest() + : api_(Api::createApiForTest(stats_store_)), dispatcher_(test_time_.timeSystem(), *api_) {} + Stats::IsolatedStoreImpl stats_store_; + Api::ApiPtr api_; DangerousDeprecatedTestTime test_time_; Event::DispatcherImpl dispatcher_; }; diff --git a/test/common/grpc/google_async_client_impl_test.cc b/test/common/grpc/google_async_client_impl_test.cc index 4b9ea5fa299b..5b42d4229c64 100644 --- a/test/common/grpc/google_async_client_impl_test.cc +++ b/test/common/grpc/google_async_client_impl_test.cc @@ -47,8 +47,8 @@ class MockStubFactory : public GoogleStubFactory { class EnvoyGoogleAsyncClientImplTest : public testing::Test { public: EnvoyGoogleAsyncClientImplTest() - : dispatcher_(test_time_.timeSystem()), stats_store_(new Stats::IsolatedStoreImpl), - api_(Api::createApiForTest(*stats_store_)), scope_(stats_store_), + : stats_store_(new Stats::IsolatedStoreImpl), api_(Api::createApiForTest(*stats_store_)), + dispatcher_(test_time_.timeSystem(), *api_), scope_(stats_store_), method_descriptor_(helloworld::Greeter::descriptor()->FindMethodByName("SayHello")) { envoy::api::v2::core::GrpcService config; auto* google_grpc = config.mutable_google_grpc(); @@ -60,9 +60,9 @@ class EnvoyGoogleAsyncClientImplTest : public testing::Test { } DangerousDeprecatedTestTime test_time_; - Event::DispatcherImpl dispatcher_; Stats::IsolatedStoreImpl* stats_store_; // Ownership transerred to scope_. Api::ApiPtr api_; + Event::DispatcherImpl dispatcher_; Stats::ScopeSharedPtr scope_; std::unique_ptr tls_; MockStubFactory stub_factory_; diff --git a/test/common/grpc/grpc_client_integration_test_harness.h b/test/common/grpc/grpc_client_integration_test_harness.h index d953842c365b..04628f17b18b 100644 --- a/test/common/grpc/grpc_client_integration_test_harness.h +++ b/test/common/grpc/grpc_client_integration_test_harness.h @@ -216,7 +216,7 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { public: GrpcClientIntegrationTest() : method_descriptor_(helloworld::Greeter::descriptor()->FindMethodByName("SayHello")), - dispatcher_(test_time_.timeSystem()), api_(Api::createApiForTest(*stats_store_)) {} + api_(Api::createApiForTest(*stats_store_)), dispatcher_(test_time_.timeSystem(), *api_) {} virtual void initialize() { if (fake_upstream_ == nullptr) { @@ -408,10 +408,10 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { FakeHttpConnectionPtr fake_connection_; std::vector fake_streams_; const Protobuf::MethodDescriptor* method_descriptor_; - Event::DispatcherImpl dispatcher_; - DispatcherHelper dispatcher_helper_{dispatcher_}; Stats::IsolatedStoreImpl* stats_store_ = new Stats::IsolatedStoreImpl(); Api::ApiPtr api_; + Event::DispatcherImpl dispatcher_; + DispatcherHelper dispatcher_helper_{dispatcher_}; Stats::ScopeSharedPtr stats_scope_{stats_store_}; TestMetadata service_wide_initial_metadata_; #ifdef ENVOY_GOOGLE_GRPC diff --git a/test/common/http/codec_client_test.cc b/test/common/http/codec_client_test.cc index 38359e29d509..fa6a882cf47c 100644 --- a/test/common/http/codec_client_test.cc +++ b/test/common/http/codec_client_test.cc @@ -261,8 +261,8 @@ TEST_F(CodecClientTest, WatermarkPassthrough) { // Test the codec getting input from a real TCP connection. class CodecNetworkTest : public testing::TestWithParam { public: - CodecNetworkTest() { - dispatcher_ = std::make_unique(test_time_.timeSystem()); + CodecNetworkTest() : api_(Api::createApiForTest(stats_store_)) { + dispatcher_ = std::make_unique(test_time_.timeSystem(), *api_); upstream_listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true, false); Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket_.localAddress(), source_address_, Network::Test::createRawBufferSocket(), nullptr); @@ -328,13 +328,14 @@ class CodecNetworkTest : public testing::TestWithParam client_; diff --git a/test/common/http/http1/conn_pool_test.cc b/test/common/http/http1/conn_pool_test.cc index dab0aaf2023d..ddcd230d5001 100644 --- a/test/common/http/http1/conn_pool_test.cc +++ b/test/common/http/http1/conn_pool_test.cc @@ -47,7 +47,8 @@ class ConnPoolImplForTest : public ConnPoolImpl { NiceMock* upstream_ready_timer) : ConnPoolImpl(dispatcher, Upstream::makeTestHost(cluster, "tcp://127.0.0.1:9000"), Upstream::ResourcePriority::Default, nullptr), - mock_dispatcher_(dispatcher), mock_upstream_ready_timer_(upstream_ready_timer) {} + api_(Api::createApiForTest(stats_store_)), mock_dispatcher_(dispatcher), + mock_upstream_ready_timer_(upstream_ready_timer) {} ~ConnPoolImplForTest() { EXPECT_EQ(0U, ready_clients_.size()); @@ -81,7 +82,7 @@ class ConnPoolImplForTest : public ConnPoolImpl { test_client.connect_timer_ = new NiceMock(&mock_dispatcher_); std::shared_ptr cluster{new NiceMock()}; test_client.client_dispatcher_ = - std::make_unique(test_time_.timeSystem()); + std::make_unique(test_time_.timeSystem(), *api_); Network::ClientConnectionPtr connection{test_client.connection_}; test_client.codec_client_ = new CodecClientForTest( std::move(connection), test_client.codec_, @@ -112,6 +113,8 @@ class ConnPoolImplForTest : public ConnPoolImpl { EXPECT_FALSE(upstream_ready_enabled_); } + Stats::IsolatedStoreImpl stats_store_; + Api::ApiPtr api_; DangerousDeprecatedTestTime test_time_; Event::MockDispatcher& mock_dispatcher_; NiceMock* mock_upstream_ready_timer_; diff --git a/test/common/http/http2/conn_pool_test.cc b/test/common/http/http2/conn_pool_test.cc index f838334fdd16..9e67df328a16 100644 --- a/test/common/http/http2/conn_pool_test.cc +++ b/test/common/http/http2/conn_pool_test.cc @@ -65,7 +65,8 @@ class Http2ConnPoolImplTest : public testing::Test { }; Http2ConnPoolImplTest() - : pool_(dispatcher_, host_, Upstream::ResourcePriority::Default, nullptr) {} + : api_(Api::createApiForTest(stats_store_)), + pool_(dispatcher_, host_, Upstream::ResourcePriority::Default, nullptr) {} ~Http2ConnPoolImplTest() { // Make sure all gauges are 0. @@ -83,7 +84,7 @@ class Http2ConnPoolImplTest : public testing::Test { test_client.codec_ = new NiceMock(); test_client.connect_timer_ = new NiceMock(&dispatcher_); test_client.client_dispatcher_ = - std::make_unique(test_time_.timeSystem()); + std::make_unique(test_time_.timeSystem(), *api_); EXPECT_CALL(dispatcher_, createClientConnection_(_, _, _, _)) .WillOnce(Return(test_client.connection_)); auto cluster = std::make_shared>(); @@ -117,6 +118,8 @@ class Http2ConnPoolImplTest : public testing::Test { MOCK_METHOD0(onClientDestroy, void()); + Stats::IsolatedStoreImpl stats_store_; + Api::ApiPtr api_; DangerousDeprecatedTestTime test_time_; NiceMock dispatcher_; std::shared_ptr cluster_{new NiceMock()}; diff --git a/test/common/network/connection_impl_test.cc b/test/common/network/connection_impl_test.cc index fe6ea3a67206..6fe481ec53d9 100644 --- a/test/common/network/connection_impl_test.cc +++ b/test/common/network/connection_impl_test.cc @@ -76,8 +76,10 @@ INSTANTIATE_TEST_CASE_P(IpVersions, ConnectionImplDeathTest, TestUtility::ipTestParamsToString); TEST_P(ConnectionImplDeathTest, BadFd) { + Stats::IsolatedStoreImpl stats_store; + Api::ApiPtr api = Api::createApiForTest(stats_store); Event::SimulatedTimeSystem time_system; - Event::DispatcherImpl dispatcher(time_system); + Event::DispatcherImpl dispatcher(time_system, *api); EXPECT_DEATH_LOG_TO_STDERR( ConnectionImpl(dispatcher, std::make_unique(-1, nullptr, nullptr), Network::Test::createRawBufferSocket(), false), @@ -86,9 +88,11 @@ TEST_P(ConnectionImplDeathTest, BadFd) { class ConnectionImplTest : public testing::TestWithParam { public: + ConnectionImplTest() : api_(Api::createApiForTest(stats_store_)) {} + void setUpBasicConnection() { if (dispatcher_.get() == nullptr) { - dispatcher_ = std::make_unique(time_system_); + dispatcher_ = std::make_unique(time_system_, *api_); } listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true, false); @@ -150,8 +154,8 @@ class ConnectionImplTest : public testing::TestWithParam { ASSERT(dispatcher_.get() == nullptr); MockBufferFactory* factory = new StrictMock; - dispatcher_ = - std::make_unique(time_system_, Buffer::WatermarkFactoryPtr{factory}); + dispatcher_ = std::make_unique( + time_system_, Buffer::WatermarkFactoryPtr{factory}, *api_); // The first call to create a client session will get a MockBuffer. // Other calls for server sessions will by default get a normal OwnedImpl. EXPECT_CALL(*factory, create_(_, _)) @@ -200,6 +204,7 @@ class ConnectionImplTest : public testing::TestWithParam { Event::SimulatedTimeSystem time_system_; Event::DispatcherPtr dispatcher_; Stats::IsolatedStoreImpl stats_store_; + Api::ApiPtr api_; Network::TcpListenSocket socket_{Network::Test::getAnyAddress(GetParam()), nullptr, true}; Network::MockListenerCallbacks listener_callbacks_; Network::MockConnectionHandler connection_handler_; @@ -261,7 +266,7 @@ TEST_P(ConnectionImplTest, CloseDuringConnectCallback) { } TEST_P(ConnectionImplTest, ImmediateConnectError) { - dispatcher_ = std::make_unique(time_system_); + dispatcher_ = std::make_unique(time_system_, *api_); // Using a broadcast/multicast address as the connection destinations address causes an // immediate error return from connect(). @@ -849,7 +854,7 @@ TEST_P(ConnectionImplTest, BindFailureTest) { source_address_ = Network::Address::InstanceConstSharedPtr{ new Network::Address::Ipv6Instance(address_string, 0)}; } - dispatcher_ = std::make_unique(time_system_); + dispatcher_ = std::make_unique(time_system_, *api_); listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true, false); client_connection_ = dispatcher_->createClientConnection( @@ -1492,7 +1497,7 @@ class ReadBufferLimitTest : public ConnectionImplTest { public: void readBufferLimitTest(uint32_t read_buffer_limit, uint32_t expected_chunk_size) { const uint32_t buffer_size = 256 * 1024; - dispatcher_ = std::make_unique(time_system_); + dispatcher_ = std::make_unique(time_system_, *api_); listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true, false); client_connection_ = dispatcher_->createClientConnection( @@ -1563,7 +1568,11 @@ TEST_P(ReadBufferLimitTest, SomeLimit) { class TcpClientConnectionImplTest : public testing::TestWithParam { protected: - TcpClientConnectionImplTest() : dispatcher_(time_system_) {} + TcpClientConnectionImplTest() + : api_(Api::createApiForTest(stats_store_)), dispatcher_(time_system_, *api_) {} + + Stats::IsolatedStoreImpl stats_store_; + Api::ApiPtr api_; Event::SimulatedTimeSystem time_system_; Event::DispatcherImpl dispatcher_; }; @@ -1604,7 +1613,11 @@ TEST_P(TcpClientConnectionImplTest, BadConnectConnRefused) { class PipeClientConnectionImplTest : public testing::Test { protected: - PipeClientConnectionImplTest() : dispatcher_(time_system_) {} + PipeClientConnectionImplTest() + : api_(Api::createApiForTest(stats_store_)), dispatcher_(time_system_, *api_) {} + + Stats::IsolatedStoreImpl stats_store_; + Api::ApiPtr api_; Event::SimulatedTimeSystem time_system_; Event::DispatcherImpl dispatcher_; const std::string path_{TestEnvironment::unixDomainSocketPath("foo")}; diff --git a/test/common/network/dns_impl_test.cc b/test/common/network/dns_impl_test.cc index bbb1ccc9445d..59ce87eea6ba 100644 --- a/test/common/network/dns_impl_test.cc +++ b/test/common/network/dns_impl_test.cc @@ -300,6 +300,7 @@ class TestDnsServer : public ListenerCallbacks { class DnsResolverImplPeer { public: DnsResolverImplPeer(DnsResolverImpl* resolver) : resolver_(resolver) {} + ares_channel channel() const { return resolver_->channel_; } const std::unordered_map& events() { return resolver_->events_; } // Reset the channel state for a DnsResolverImpl such that it will only use @@ -324,7 +325,11 @@ class DnsResolverImplPeer { class DnsImplConstructor : public testing::Test { protected: - DnsImplConstructor() : dispatcher_(test_time_.timeSystem()) {} + DnsImplConstructor() + : api_(Api::createApiForTest(stats_store_)), dispatcher_(test_time_.timeSystem(), *api_) {} + + Stats::IsolatedStoreImpl stats_store_; + Api::ApiPtr api_; DangerousDeprecatedTestTime test_time_; Event::DispatcherImpl dispatcher_; }; @@ -402,7 +407,8 @@ TEST_F(DnsImplConstructor, BadCustomResolvers) { class DnsImplTest : public testing::TestWithParam { public: - DnsImplTest() : dispatcher_(test_time_.timeSystem()) {} + DnsImplTest() + : api_(Api::createApiForTest(stats_store_)), dispatcher_(test_time_.timeSystem(), *api_) {} void SetUp() override { resolver_ = dispatcher_.createDnsResolver({}); @@ -434,6 +440,7 @@ class DnsImplTest : public testing::TestWithParam { Network::TcpListenSocketPtr socket_; Stats::IsolatedStoreImpl stats_store_; std::unique_ptr listener_; + Api::ApiPtr api_; DangerousDeprecatedTestTime test_time_; Event::DispatcherImpl dispatcher_; DnsResolverSharedPtr resolver_; diff --git a/test/common/network/listener_impl_test.cc b/test/common/network/listener_impl_test.cc index 7a76a73d1bb3..0ee3c8ea61d5 100644 --- a/test/common/network/listener_impl_test.cc +++ b/test/common/network/listener_impl_test.cc @@ -23,8 +23,9 @@ static void errorCallbackTest(Address::IpVersion version) { // Force the error callback to fire by closing the socket under the listener. We run this entire // test in the forked process to avoid confusion when the fork happens. Stats::IsolatedStoreImpl stats_store; + Api::ApiPtr api = Api::createApiForTest(stats_store); DangerousDeprecatedTestTime test_time; - Event::DispatcherImpl dispatcher(test_time.timeSystem()); + Event::DispatcherImpl dispatcher(test_time.timeSystem(), *api); Network::TcpListenSocket socket(Network::Test::getCanonicalLoopbackAddress(version), nullptr, true); @@ -78,10 +79,12 @@ class ListenerImplTest : public testing::TestWithParam { : version_(GetParam()), alt_address_(Network::Test::findOrCheckFreePort( Network::Test::getCanonicalLoopbackAddress(version_), Address::SocketType::Stream)), - dispatcher_(test_time_.timeSystem()) {} + api_(Api::createApiForTest(stats_store_)), dispatcher_(test_time_.timeSystem(), *api_) {} const Address::IpVersion version_; const Address::InstanceConstSharedPtr alt_address_; + Stats::IsolatedStoreImpl stats_store_; + Api::ApiPtr api_; DangerousDeprecatedTestTime test_time_; Event::DispatcherImpl dispatcher_; }; diff --git a/test/common/runtime/runtime_impl_test.cc b/test/common/runtime/runtime_impl_test.cc index 0cb7c410cb42..5b03399638fa 100644 --- a/test/common/runtime/runtime_impl_test.cc +++ b/test/common/runtime/runtime_impl_test.cc @@ -258,7 +258,7 @@ TEST(LoaderImplTest, All) { } TEST(DiskLayer, IllegalPath) { -#if defined(WIN32) +#ifdef WIN32 // no illegal paths on Windows at the moment return; #endif diff --git a/test/common/singleton/manager_impl_test.cc b/test/common/singleton/manager_impl_test.cc index 1b84b4338904..238ff8479cbd 100644 --- a/test/common/singleton/manager_impl_test.cc +++ b/test/common/singleton/manager_impl_test.cc @@ -11,7 +11,8 @@ namespace Singleton { // Must be a dedicated function so that TID is within the death test. static void deathTestWorker() { - ManagerImpl manager; + ManagerImpl manager(Thread::threadFactoryForTest().currentThreadId()); + manager.get("foo", [] { return nullptr; }); } @@ -33,7 +34,7 @@ class TestSingleton : public Instance { }; TEST(SingletonManagerImplTest, Basic) { - ManagerImpl manager; + ManagerImpl manager(Thread::threadFactoryForTest().currentThreadId()); std::shared_ptr singleton = std::make_shared(); EXPECT_EQ(singleton, manager.get("test_singleton", [singleton] { return singleton; })); diff --git a/test/common/ssl/ssl_socket_test.cc b/test/common/ssl/ssl_socket_test.cc index cc818b288dd4..058569ee0267 100644 --- a/test/common/ssl/ssl_socket_test.cc +++ b/test/common/ssl/ssl_socket_test.cc @@ -74,7 +74,8 @@ void testUtil(const std::string& client_ctx_yaml, const std::string& server_ctx_ std::move(server_cfg), manager, server_stats_store, std::vector{}); DangerousDeprecatedTestTime test_time; - Event::DispatcherImpl dispatcher(test_time.timeSystem()); + Api::ApiPtr api = Api::createApiForTest(server_stats_store); + Event::DispatcherImpl dispatcher(test_time.timeSystem(), *api); Network::TcpListenSocket socket(Network::Test::getCanonicalLoopbackAddress(version), nullptr, true); Network::MockListenerCallbacks callbacks; @@ -196,7 +197,8 @@ const std::string testUtilV2( server_stats_store, server_names); DangerousDeprecatedTestTime test_time; - Event::DispatcherImpl dispatcher(test_time.timeSystem()); + Api::ApiPtr api = Api::createApiForTest(server_stats_store); + Event::DispatcherImpl dispatcher(test_time.timeSystem(), *api); Network::TcpListenSocket socket(Network::Test::getCanonicalLoopbackAddress(version), nullptr, true); NiceMock callbacks; @@ -365,8 +367,12 @@ void configureServerAndExpiredClientCertificate(envoy::api::v2::Listener& listen class SslSocketTest : public SslCertsTest, public testing::WithParamInterface { protected: - SslSocketTest() : dispatcher_(std::make_unique(test_time_.timeSystem())) {} + SslSocketTest() + : api_(Api::createApiForTest(stats_store_)), + dispatcher_(std::make_unique(test_time_.timeSystem(), *api_)) {} + Stats::IsolatedStoreImpl stats_store_; + Api::ApiPtr api_; DangerousDeprecatedTestTime test_time_; std::unique_ptr dispatcher_; }; @@ -1895,7 +1901,8 @@ void testTicketSessionResumption(const std::string& server_ctx_yaml1, NiceMock callbacks; Network::MockConnectionHandler connection_handler; DangerousDeprecatedTestTime test_time; - Event::DispatcherImpl dispatcher(test_time.timeSystem()); + Api::ApiPtr api = Api::createApiForTest(server_stats_store); + Event::DispatcherImpl dispatcher(test_time.timeSystem(), *api); Network::ListenerPtr listener1 = dispatcher.createListener(socket1, callbacks, true, false); Network::ListenerPtr listener2 = dispatcher.createListener(socket2, callbacks, true, false); @@ -2408,7 +2415,8 @@ void testClientSessionResumption(const std::string& server_ctx_yaml, true); NiceMock callbacks; Network::MockConnectionHandler connection_handler; - Event::DispatcherImpl dispatcher(time_system); + Api::ApiPtr api = Api::createApiForTest(server_stats_store); + Event::DispatcherImpl dispatcher(time_system, *api); Network::ListenerPtr listener = dispatcher.createListener(socket, callbacks, true, false); Network::ConnectionPtr server_connection; @@ -3330,8 +3338,8 @@ class SslReadBufferLimitTest : public SslSocketTest { void singleWriteTest(uint32_t read_buffer_limit, uint32_t bytes_to_write) { MockWatermarkBuffer* client_write_buffer = nullptr; MockBufferFactory* factory = new StrictMock; - dispatcher_ = std::make_unique(test_time_.timeSystem(), - Buffer::WatermarkFactoryPtr{factory}); + dispatcher_ = std::make_unique( + test_time_.timeSystem(), Buffer::WatermarkFactoryPtr{factory}, *api_); // By default, expect 4 buffers to be created - the client and server read and write buffers. EXPECT_CALL(*factory, create_(_, _)) diff --git a/test/common/stats/BUILD b/test/common/stats/BUILD index 76d74d1a05cf..18a81ff4431c 100644 --- a/test/common/stats/BUILD +++ b/test/common/stats/BUILD @@ -132,5 +132,6 @@ envoy_cc_test_binary( "//source/common/stats:thread_local_store_lib", "//source/common/thread_local:thread_local_lib", "//test/test_common:simulated_time_system_lib", + "//test/test_common:utility_lib", ], ) diff --git a/test/common/stats/thread_local_store_speed_test.cc b/test/common/stats/thread_local_store_speed_test.cc index 86ab8f30fa58..520f78fe3a3a 100644 --- a/test/common/stats/thread_local_store_speed_test.cc +++ b/test/common/stats/thread_local_store_speed_test.cc @@ -12,6 +12,7 @@ #include "test/common/stats/stat_test_utility.h" #include "test/test_common/simulated_time_system.h" +#include "test/test_common/utility.h" #include "testing/base/public/benchmark.h" @@ -19,7 +20,7 @@ namespace Envoy { class ThreadLocalStorePerf { public: - ThreadLocalStorePerf() : store_(options_, heap_alloc_) { + ThreadLocalStorePerf() : store_(options_, heap_alloc_), api_(Api::createApiForTest(store_)) { store_.setTagProducer(std::make_unique(stats_config_)); } @@ -36,18 +37,19 @@ class ThreadLocalStorePerf { } void initThreading() { - dispatcher_ = std::make_unique(time_system_); + dispatcher_ = std::make_unique(time_system_, *api_); tls_ = std::make_unique(); store_.initializeThreading(*dispatcher_, *tls_); } private: Stats::StatsOptionsImpl options_; - Event::SimulatedTimeSystem time_system_; Stats::HeapStatDataAllocator heap_alloc_; + Stats::ThreadLocalStoreImpl store_; + Api::ApiPtr api_; + Event::SimulatedTimeSystem time_system_; std::unique_ptr dispatcher_; std::unique_ptr tls_; - Stats::ThreadLocalStoreImpl store_; envoy::config::metrics::v2::StatsConfig stats_config_; }; diff --git a/test/common/thread_local/thread_local_impl_test.cc b/test/common/thread_local/thread_local_impl_test.cc index 0f81e38a13b1..0f67f18c5022 100644 --- a/test/common/thread_local/thread_local_impl_test.cc +++ b/test/common/thread_local/thread_local_impl_test.cc @@ -118,8 +118,10 @@ TEST(ThreadLocalInstanceImplDispatcherTest, Dispatcher) { InstanceImpl tls; DangerousDeprecatedTestTime test_time; - Event::DispatcherImpl main_dispatcher(test_time.timeSystem()); - Event::DispatcherImpl thread_dispatcher(test_time.timeSystem()); + Stats::IsolatedStoreImpl stats_store; + Api::ApiPtr api = Api::createApiForTest(stats_store); + Event::DispatcherImpl main_dispatcher(test_time.timeSystem(), *api); + Event::DispatcherImpl thread_dispatcher(test_time.timeSystem(), *api); tls.registerThread(main_dispatcher, true); tls.registerThread(thread_dispatcher, false); diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 1715af82aa39..ea2dfa3be19f 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -50,7 +50,7 @@ class ProxyProtocolTest : public testing::TestWithParam { public: ProxyProtocolTest() - : dispatcher_(test_time_.timeSystem()), + : api_(Api::createApiForTest(stats_store_)), dispatcher_(test_time_.timeSystem(), *api_), socket_(Network::Test::getCanonicalLoopbackAddress(GetParam()), nullptr, true), connection_handler_(new Server::ConnectionHandlerImpl(ENVOY_LOGGER(), dispatcher_)), name_("proxy"), filter_chain_(Network::Test::createEmptyFilterChainWithRawBufferSockets()) { @@ -149,10 +149,11 @@ class ProxyProtocolTest : public testing::TestWithParam { public: WildcardProxyProtocolTest() - : dispatcher_(test_time_.timeSystem()), + : api_(Api::createApiForTest(stats_store_)), dispatcher_(test_time_.timeSystem(), *api_), socket_(Network::Test::getAnyAddress(GetParam()), nullptr, true), local_dst_address_(Network::Utility::getAddressWithPort( *Network::Test::getCanonicalLoopbackAddress(GetParam()), @@ -955,11 +956,12 @@ class WildcardProxyProtocolTest : public testing::TestWithParamcreateResourceMonitor(config, context); EXPECT_NE(monitor, nullptr); diff --git a/test/extensions/resource_monitors/injected_resource/injected_resource_monitor_test.cc b/test/extensions/resource_monitors/injected_resource/injected_resource_monitor_test.cc index 10036b37ee51..5096c19246b3 100644 --- a/test/extensions/resource_monitors/injected_resource/injected_resource_monitor_test.cc +++ b/test/extensions/resource_monitors/injected_resource/injected_resource_monitor_test.cc @@ -1,4 +1,5 @@ #include "common/event/dispatcher_impl.h" +#include "common/stats/isolated_store_impl.h" #include "server/resource_monitor_config_impl.h" @@ -45,7 +46,7 @@ class MockedCallbacks : public Server::ResourceMonitor::Callbacks { class InjectedResourceMonitorTest : public testing::Test { protected: InjectedResourceMonitorTest() - : dispatcher_(test_time_.timeSystem()), + : api_(Api::createApiForTest(stats_store_)), dispatcher_(test_time_.timeSystem(), *api_), resource_filename_(TestEnvironment::temporaryPath("injected_resource")), file_updater_(resource_filename_), monitor_(createMonitor()) {} @@ -64,6 +65,8 @@ class InjectedResourceMonitorTest : public testing::Test { return std::make_unique(config, context); } + Stats::IsolatedStoreImpl stats_store_; + Api::ApiPtr api_; DangerousDeprecatedTestTime test_time_; Event::DispatcherImpl dispatcher_; const std::string resource_filename_; diff --git a/test/extensions/transport_sockets/alts/alts_integration_test.cc b/test/extensions/transport_sockets/alts/alts_integration_test.cc index 14f08ba382e9..34340907a612 100644 --- a/test/extensions/transport_sockets/alts/alts_integration_test.cc +++ b/test/extensions/transport_sockets/alts/alts_integration_test.cc @@ -55,7 +55,7 @@ class AltsIntegrationTestBase : public HttpIntegrationTest, } void SetUp() override { - fake_handshaker_server_thread_ = api_->createThread([this]() { + fake_handshaker_server_thread_ = api_->threadFactory().createThread([this]() { std::unique_ptr service = grpc::gcp::CreateFakeHandshakerService(); std::string server_address = Network::Test::getLoopbackAddressUrlString(version_) + ":0"; diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index ed9217b57e34..e43ca2c55bdf 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -376,7 +376,7 @@ FakeUpstream::FakeUpstream(Network::TransportSocketFactoryPtr&& transport_socket handler_(new Server::ConnectionHandlerImpl(ENVOY_LOGGER(), *dispatcher_)), allow_unexpected_disconnects_(false), enable_half_close_(enable_half_close), listener_(*this), filter_chain_(Network::Test::createEmptyFilterChain(std::move(transport_socket_factory))) { - thread_ = api_->createThread([this]() -> void { threadRoutine(); }); + thread_ = api_->threadFactory().createThread([this]() -> void { threadRoutine(); }); server_initialized_.waitReady(); } diff --git a/test/integration/integration.cc b/test/integration/integration.cc index 1b2a96d03a1d..94bea9dcd7b8 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -222,8 +222,8 @@ BaseIntegrationTest::BaseIntegrationTest(Network::Address::IpVersion version, TestTimeSystemPtr time_system, const std::string& config) : api_(Api::createApiForTest(stats_store_)), mock_buffer_factory_(new NiceMock), time_system_(std::move(time_system)), - dispatcher_(new Event::DispatcherImpl(*time_system_, - Buffer::WatermarkFactoryPtr{mock_buffer_factory_})), + dispatcher_(new Event::DispatcherImpl( + *time_system_, Buffer::WatermarkFactoryPtr{mock_buffer_factory_}, *api_)), version_(version), config_helper_(version, config), default_log_level_(TestEnvironment::getOptions().logLevel()) { // This is a hack, but there are situations where we disconnect fake upstream connections and diff --git a/test/integration/server.cc b/test/integration/server.cc index 3f2319536c6b..a1fb92d02067 100644 --- a/test/integration/server.cc +++ b/test/integration/server.cc @@ -61,7 +61,7 @@ void IntegrationTestServer::start(const Network::Address::IpVersion version, bool deterministic) { ENVOY_LOG(info, "starting integration test server"); ASSERT(!thread_); - thread_ = api_.createThread( + thread_ = api_.threadFactory().createThread( [version, deterministic, this]() -> void { threadRoutine(version, deterministic); }); // If any steps need to be done prior to workers starting, do them now. E.g., xDS pre-init. diff --git a/test/mocks/api/mocks.h b/test/mocks/api/mocks.h index e0fa75215e02..26eb42752cb3 100644 --- a/test/mocks/api/mocks.h +++ b/test/mocks/api/mocks.h @@ -35,7 +35,7 @@ class MockApi : public Api { Thread::BasicLockable& lock)); MOCK_METHOD1(fileExists, bool(const std::string& path)); MOCK_METHOD1(fileReadToEnd, std::string(const std::string& path)); - MOCK_METHOD1(createThread, Thread::ThreadPtr(std::function thread_routine)); + MOCK_METHOD0(threadFactory, Thread::ThreadFactory&()); std::shared_ptr file_{new Filesystem::MockFile()}; }; diff --git a/test/mocks/server/mocks.cc b/test/mocks/server/mocks.cc index 89e44cd3fbd5..dc194d3f5810 100644 --- a/test/mocks/server/mocks.cc +++ b/test/mocks/server/mocks.cc @@ -121,7 +121,8 @@ MockWorker::~MockWorker() {} MockInstance::MockInstance() : secret_manager_(new Secret::SecretManagerImpl()), cluster_manager_(timeSystem()), - ssl_context_manager_(timeSystem()), singleton_manager_(new Singleton::ManagerImpl()) { + ssl_context_manager_(timeSystem()), singleton_manager_(new Singleton::ManagerImpl( + Thread::threadFactoryForTest().currentThreadId())) { ON_CALL(*this, threadLocal()).WillByDefault(ReturnRef(thread_local_)); ON_CALL(*this, stats()).WillByDefault(ReturnRef(stats_store_)); ON_CALL(*this, httpContext()).WillByDefault(ReturnRef(http_context_)); @@ -160,7 +161,9 @@ MockMain::MockMain(int wd_miss, int wd_megamiss, int wd_kill, int wd_multikill) MockMain::~MockMain() {} -MockFactoryContext::MockFactoryContext() : singleton_manager_(new Singleton::ManagerImpl()) { +MockFactoryContext::MockFactoryContext() + : singleton_manager_( + new Singleton::ManagerImpl(Thread::threadFactoryForTest().currentThreadId())) { ON_CALL(*this, accessLogManager()).WillByDefault(ReturnRef(access_log_manager_)); ON_CALL(*this, clusterManager()).WillByDefault(ReturnRef(cluster_manager_)); ON_CALL(*this, dispatcher()).WillByDefault(ReturnRef(dispatcher_)); diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index 4b66867edda1..8c9f9c445445 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -159,7 +159,7 @@ class MockWatchDog : public WatchDog { // Server::WatchDog MOCK_METHOD1(startWatchdog, void(Event::Dispatcher& dispatcher)); MOCK_METHOD0(touch, void()); - MOCK_CONST_METHOD0(threadId, int32_t()); + MOCK_CONST_METHOD0(threadId, const Thread::ThreadId&()); MOCK_CONST_METHOD0(lastTouchTime, MonotonicTime()); }; @@ -169,7 +169,7 @@ class MockGuardDog : public GuardDog { ~MockGuardDog(); // Server::GuardDog - MOCK_METHOD1(createWatchDog, WatchDogSharedPtr(int32_t thread_id)); + MOCK_METHOD1(createWatchDog, WatchDogSharedPtr(Thread::ThreadIdPtr&&)); MOCK_METHOD1(stopWatching, void(WatchDogSharedPtr wd)); std::shared_ptr watch_dog_; @@ -417,7 +417,6 @@ class MockFactoryContext : public FactoryContext { MOCK_CONST_METHOD0(listenerMetadata, const envoy::api::v2::core::Metadata&()); MOCK_METHOD0(timeSource, TimeSource&()); Event::SimulatedTimeSystem& timeSystem() { return time_system_; } - Http::Context& httpContext() override { return http_context_; } testing::NiceMock access_log_manager_; diff --git a/test/server/config_validation/async_client_test.cc b/test/server/config_validation/async_client_test.cc index baa1e201797c..7dfc5e3a6e6b 100644 --- a/test/server/config_validation/async_client_test.cc +++ b/test/server/config_validation/async_client_test.cc @@ -17,7 +17,9 @@ TEST(ValidationAsyncClientTest, MockedMethods) { MockAsyncClientStreamCallbacks stream_callbacks; DangerousDeprecatedTestTime test_time; - ValidationAsyncClient client(test_time.timeSystem()); + Stats::IsolatedStoreImpl stats_store; + Api::ApiPtr api = Api::createApiForTest(stats_store); + ValidationAsyncClient client(test_time.timeSystem(), *api); EXPECT_EQ(nullptr, client.send(std::move(message), callbacks, AsyncClient::RequestOptions())); EXPECT_EQ(nullptr, client.start(stream_callbacks, AsyncClient::StreamOptions())); } diff --git a/test/server/guarddog_impl_test.cc b/test/server/guarddog_impl_test.cc index e074ddf4cea9..4b27653cc780 100644 --- a/test/server/guarddog_impl_test.cc +++ b/test/server/guarddog_impl_test.cc @@ -52,7 +52,7 @@ class GuardDogDeathTest : public GuardDogTestBase { void SetupForDeath() { InSequence s; guard_dog_ = std::make_unique(fakestats_, config_kill_, time_system_, *api_); - unpet_dog_ = guard_dog_->createWatchDog(0); + unpet_dog_ = guard_dog_->createWatchDog(api_->threadFactory().currentThreadId()); guard_dog_->forceCheckForTest(); time_system_.sleep(std::chrono::milliseconds(500)); } @@ -64,9 +64,9 @@ class GuardDogDeathTest : public GuardDogTestBase { void SetupForMultiDeath() { InSequence s; guard_dog_ = std::make_unique(fakestats_, config_multikill_, time_system_, *api_); - auto unpet_dog_ = guard_dog_->createWatchDog(0); + auto unpet_dog_ = guard_dog_->createWatchDog(api_->threadFactory().currentThreadId()); guard_dog_->forceCheckForTest(); - auto second_dog_ = guard_dog_->createWatchDog(1); + auto second_dog_ = guard_dog_->createWatchDog(api_->threadFactory().currentThreadId()); guard_dog_->forceCheckForTest(); time_system_.sleep(std::chrono::milliseconds(501)); } @@ -121,8 +121,8 @@ TEST_F(GuardDogAlmostDeadTest, NearDeathTest) { // there is no death. The positive case is covered in MultiKillDeathTest. InSequence s; GuardDogImpl gd(fakestats_, config_multikill_, time_system_, *api_); - auto unpet_dog = gd.createWatchDog(0); - auto pet_dog = gd.createWatchDog(1); + auto unpet_dog = gd.createWatchDog(api_->threadFactory().currentThreadId()); + auto pet_dog = gd.createWatchDog(api_->threadFactory().currentThreadId()); // This part "waits" 600 milliseconds while one dog is touched every 100, and // the other is not. 600ms is over the threshold of 500ms for multi-kill but // only one is nonresponsive, so there should be no kill (single kill @@ -149,7 +149,7 @@ TEST_F(GuardDogMissTest, MissTest) { 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); + auto unpet_dog = gd.createWatchDog(api_->threadFactory().currentThreadId()); // At 300ms we shouldn't have hit the timeout yet: time_system_.sleep(std::chrono::milliseconds(300)); gd.forceCheckForTest(); @@ -166,7 +166,7 @@ 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_); - auto unpet_dog = gd.createWatchDog(0); + auto unpet_dog = gd.createWatchDog(api_->threadFactory().currentThreadId()); // We'd better start at 0: EXPECT_EQ(0UL, stats_store_.counter("server.watchdog_mega_miss").value()); // This shouldn't be enough to increment the stat: @@ -186,7 +186,7 @@ TEST_F(GuardDogMissTest, MissCountTest) { // spurious condition_variable wakeup causes the counter to get incremented // more than it should be. GuardDogImpl gd(stats_store_, config_miss_, time_system_, *api_); - auto sometimes_pet_dog = gd.createWatchDog(0); + auto sometimes_pet_dog = gd.createWatchDog(api_->threadFactory().currentThreadId()); // These steps are executed once without ever touching the watchdog. // Then the last step is to touch the watchdog and repeat the steps. // This verifies that the behavior is reset back to baseline after a touch. @@ -246,8 +246,9 @@ TEST_F(GuardDogTestBase, WatchDogThreadIdTest) { NiceMock stats; NiceMock config(100, 90, 1000, 500); GuardDogImpl gd(stats, config, time_system_, *api_); - auto watched_dog = gd.createWatchDog(123); - EXPECT_EQ(watched_dog->threadId(), 123); + auto watched_dog = gd.createWatchDog(api_->threadFactory().currentThreadId()); + EXPECT_EQ(watched_dog->threadId().debugString(), + api_->threadFactory().currentThreadId()->debugString()); gd.stopWatching(watched_dog); } diff --git a/test/server/worker_impl_test.cc b/test/server/worker_impl_test.cc index c4348252a24c..398e59e2e571 100644 --- a/test/server/worker_impl_test.cc +++ b/test/server/worker_impl_test.cc @@ -33,11 +33,11 @@ class WorkerImplTest : public testing::Test { Stats::IsolatedStoreImpl stats_store_; NiceMock tls_; DangerousDeprecatedTestTime test_time; - Event::DispatcherImpl* dispatcher_ = new Event::DispatcherImpl(test_time.timeSystem()); Network::MockConnectionHandler* handler_ = new Network::MockConnectionHandler(); NiceMock guard_dog_; NiceMock overload_manager_; Api::ApiPtr api_; + Event::DispatcherImpl* dispatcher_ = new Event::DispatcherImpl(test_time.timeSystem(), *api_); DefaultTestHooks hooks_; WorkerImpl worker_{tls_, hooks_, diff --git a/test/test_common/BUILD b/test/test_common/BUILD index 3a743fdd9d06..0005186639cb 100644 --- a/test/test_common/BUILD +++ b/test/test_common/BUILD @@ -167,6 +167,7 @@ envoy_cc_test_library( hdrs = ["test_time_system.h"], deps = [ "//include/envoy/event:timer_interface", + "//source/common/common:thread_lib", ], ) diff --git a/test/test_common/environment.cc b/test/test_common/environment.cc index 7bde7bdcd584..43f1d53e4787 100644 --- a/test/test_common/environment.cc +++ b/test/test_common/environment.cc @@ -33,15 +33,15 @@ namespace Envoy { namespace { std::string makeTempDir(char* name_template) { -#if !defined(WIN32) - char* dirname = ::mkdtemp(name_template); +#ifdef WIN32 + char* dirname = ::_mktemp(name_template); RELEASE_ASSERT(dirname != nullptr, fmt::format("failed to create tempdir: {} {}", dirname, strerror(errno))); + std::experimental::filesystem::create_directories(dirname); #else - char* dirname = ::_mktemp(name_template); + char* dirname = ::mkdtemp(name_template); RELEASE_ASSERT(dirname != nullptr, fmt::format("failed to create tempdir: {} {}", dirname, strerror(errno))); - std::experimental::filesystem::create_directories(dirname); #endif return std::string(dirname); } @@ -305,10 +305,7 @@ std::string TestEnvironment::writeStringToFileForTest(const std::string& filenam } void TestEnvironment::setEnvVar(const std::string& name, const std::string& value, int overwrite) { -#if !defined(WIN32) - const int rc = ::setenv(name.c_str(), value.c_str(), overwrite); - ASSERT_EQ(rc, 0); -#else +#ifdef WIN32 if (!overwrite) { size_t requiredSize; const int rc = ::getenv_s(&requiredSize, NULL, 0, name.c_str()); @@ -319,6 +316,9 @@ void TestEnvironment::setEnvVar(const std::string& name, const std::string& valu } const int rc = ::_putenv_s(name.c_str(), value.c_str()); ASSERT_EQ(0, rc); +#else + const int rc = ::setenv(name.c_str(), value.c_str(), overwrite); + ASSERT_EQ(rc, 0); #endif } diff --git a/test/test_common/test_time_system.h b/test/test_common/test_time_system.h index 6f63ca3698bb..60a829124917 100644 --- a/test/test_common/test_time_system.h +++ b/test/test_common/test_time_system.h @@ -2,6 +2,8 @@ #include "envoy/event/timer.h" +#include "common/common/thread.h" + namespace Envoy { namespace Event { diff --git a/test/test_common/utility.cc b/test/test_common/utility.cc index 7dfc28c9ff00..5c7a94266304 100644 --- a/test/test_common/utility.cc +++ b/test/test_common/utility.cc @@ -1,6 +1,6 @@ #include "utility.h" -#if defined(WIN32) +#ifdef WIN32 #include // uses macros to #define a ton of symbols, two of which (DELETE and GetMessage) // interfere with our code. DELETE shows up in the base.pb.h header generated from @@ -27,7 +27,7 @@ #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/thread_impl.h" #include "common/common/utility.h" #include "common/config/bootstrap_json.h" #include "common/json/json_loader.h" @@ -187,30 +187,27 @@ std::vector TestUtility::split(const std::string& source, const std } void TestUtility::renameFile(const std::string& old_name, const std::string& new_name) { -#if !defined(WIN32) - const int rc = ::rename(old_name.c_str(), new_name.c_str()); - ASSERT_EQ(0, rc); -#else +#ifdef WIN32 // use MoveFileEx, since ::rename will not overwrite an existing file. See // https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/rename-wrename?view=vs-2017 const BOOL rc = ::MoveFileEx(old_name.c_str(), new_name.c_str(), MOVEFILE_REPLACE_EXISTING); ASSERT_NE(0, rc); +#else + const int rc = ::rename(old_name.c_str(), new_name.c_str()); + ASSERT_EQ(0, rc); #endif }; void TestUtility::createDirectory(const std::string& name) { -#if !defined(WIN32) - ::mkdir(name.c_str(), S_IRWXU); -#else +#ifdef WIN32 ::_mkdir(name.c_str()); +#else + ::mkdir(name.c_str(), S_IRWXU); #endif } void TestUtility::createSymlink(const std::string& target, const std::string& link) { -#if !defined(WIN32) - const int rc = ::symlink(target.c_str(), link.c_str()); - ASSERT_EQ(rc, 0); -#else +#ifdef WIN32 const DWORD attributes = ::GetFileAttributes(target.c_str()); ASSERT_NE(attributes, INVALID_FILE_ATTRIBUTES); int flags = SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE; @@ -220,6 +217,9 @@ void TestUtility::createSymlink(const std::string& target, const std::string& li const BOOLEAN rc = ::CreateSymbolicLink(link.c_str(), target.c_str(), flags); ASSERT_NE(rc, 0); +#else + const int rc = ::symlink(target.c_str(), link.c_str()); + ASSERT_EQ(rc, 0); #endif } @@ -358,8 +358,13 @@ MockedTestAllocator::~MockedTestAllocator() {} namespace Thread { +// TODO(sesmith177) Tests should get the ThreadFactory from the same location as the main code ThreadFactory& threadFactoryForTest() { - static ThreadFactoryImpl* thread_factory = new ThreadFactoryImpl(); +#ifdef WIN32 + static ThreadFactoryImplWin32* thread_factory = new ThreadFactoryImplWin32(); +#else + static ThreadFactoryImplPosix* thread_factory = new ThreadFactoryImplPosix(); +#endif return *thread_factory; }