From d77d349b983273ab58848d469c9c3f94b69e20c3 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Sat, 12 Dec 2020 20:21:27 -0600 Subject: [PATCH 01/47] macros and helper function Signed-off-by: chaoqin-li1123 --- include/envoy/common/exception.h | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/include/envoy/common/exception.h b/include/envoy/common/exception.h index d5b9255ead1e2..580a3c8253275 100644 --- a/include/envoy/common/exception.h +++ b/include/envoy/common/exception.h @@ -2,13 +2,26 @@ #include #include +#include namespace Envoy { /** * Base class for all envoy exceptions. */ +#define envoy_try \ +assert(isMainThread()); try + class EnvoyException : public std::runtime_error { public: EnvoyException(const std::string& message) : std::runtime_error(message) {} }; + + +static bool isMainThread() { + static std::thread::id main_thread_id = std::this_thread::get_id(); + return main_thread_id == std::this_thread::get_id(); +} + + + } // namespace Envoy From 3e9a5be9ab1ab147ed0aa62be95f9bacd130ebb4 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Sat, 12 Dec 2020 23:42:04 -0600 Subject: [PATCH 02/47] expose main thread verification interface Signed-off-by: chaoqin-li1123 --- include/envoy/common/exception.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/envoy/common/exception.h b/include/envoy/common/exception.h index 580a3c8253275..d2306b3dfaf4e 100644 --- a/include/envoy/common/exception.h +++ b/include/envoy/common/exception.h @@ -16,8 +16,7 @@ class EnvoyException : public std::runtime_error { EnvoyException(const std::string& message) : std::runtime_error(message) {} }; - -static bool isMainThread() { +bool isMainThread() { static std::thread::id main_thread_id = std::this_thread::get_id(); return main_thread_id == std::this_thread::get_id(); } From 51f55493b625fe08e9ad785f658af84a4104204e Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Sat, 12 Dec 2020 23:50:40 -0600 Subject: [PATCH 03/47] fix format Signed-off-by: chaoqin-li1123 --- include/envoy/common/exception.h | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/include/envoy/common/exception.h b/include/envoy/common/exception.h index d2306b3dfaf4e..fa66e27e9dea7 100644 --- a/include/envoy/common/exception.h +++ b/include/envoy/common/exception.h @@ -8,8 +8,9 @@ namespace Envoy { /** * Base class for all envoy exceptions. */ -#define envoy_try \ -assert(isMainThread()); try +#define envoy_try \ + assert(isMainThread()); \ + try class EnvoyException : public std::runtime_error { public: @@ -17,10 +18,8 @@ class EnvoyException : public std::runtime_error { }; bool isMainThread() { - static std::thread::id main_thread_id = std::this_thread::get_id(); - return main_thread_id == std::this_thread::get_id(); + static std::thread::id main_thread_id = std::this_thread::get_id(); + return main_thread_id == std::this_thread::get_id(); } - - } // namespace Envoy From eac8710eb878d9382dd4af2d12d0b2512dd1b13f Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Mon, 14 Dec 2020 17:16:06 -0600 Subject: [PATCH 04/47] use singleton Signed-off-by: chaoqin-li1123 --- include/envoy/common/BUILD | 3 +++ include/envoy/common/exception.h | 14 +++++++++++++- source/server/server.cc | 4 ++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/include/envoy/common/BUILD b/include/envoy/common/BUILD index d120b7c0cc541..3df0a8718dd94 100644 --- a/include/envoy/common/BUILD +++ b/include/envoy/common/BUILD @@ -19,6 +19,9 @@ envoy_basic_cc_library( ], external_deps = ["abseil_optional"], include_prefix = "envoy/common", + deps = [ + "//source/common/singleton:threadsafe_singleton", + ], ) envoy_cc_library( diff --git a/include/envoy/common/exception.h b/include/envoy/common/exception.h index fa66e27e9dea7..9d272504f00a0 100644 --- a/include/envoy/common/exception.h +++ b/include/envoy/common/exception.h @@ -9,7 +9,7 @@ namespace Envoy { * Base class for all envoy exceptions. */ #define envoy_try \ - assert(isMainThread()); \ + assert(MainThreadSingleton::get().isMainThread()); \ try class EnvoyException : public std::runtime_error { @@ -17,9 +17,21 @@ class EnvoyException : public std::runtime_error { EnvoyException(const std::string& message) : std::runtime_error(message) {} }; +struct MainThread { + MainThread() : main_thread_id_{std::this_thread::get_id()} {} + isMainThread() { return main_thread_id_ == std::this_thread::get_id(); } + +private: + std::thread::id main_thread_id_; +}; + +using MainThreadSingleton = InjectableSingleton; + +/* bool isMainThread() { static std::thread::id main_thread_id = std::this_thread::get_id(); return main_thread_id == std::this_thread::get_id(); } +*/ } // namespace Envoy diff --git a/source/server/server.cc b/source/server/server.cc index ca18d62e43757..4eed0a0554ab2 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -324,6 +324,8 @@ void InstanceImpl::initialize(const Options& options, ENVOY_LOG(info, " {}: {}", ext.first, absl::StrJoin(ext.second->registeredNames(), ", ")); } + MainThreadSingleton::initialize(new MainThread()); + // Handle configuration that needs to take place prior to the main configuration load. InstanceUtil::loadBootstrapConfig(bootstrap_, options, messageValidationContext().staticValidationVisitor(), *api_); @@ -551,6 +553,8 @@ void InstanceImpl::initialize(const Options& options, // instantiated (which in turn relies on runtime...). Runtime::LoaderSingleton::get().initialize(clusterManager()); + MainThreadSingleton + clusterManager().setPrimaryClustersInitializedCb( [this]() { onClusterManagerPrimaryInitializationComplete(); }); From a40cf1f24dcc03d612bf23f39335ff7788ea64f0 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Mon, 14 Dec 2020 18:36:32 -0600 Subject: [PATCH 05/47] assert Signed-off-by: chaoqin-li1123 --- include/envoy/common/exception.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/envoy/common/exception.h b/include/envoy/common/exception.h index 9d272504f00a0..f10ede5c67ad7 100644 --- a/include/envoy/common/exception.h +++ b/include/envoy/common/exception.h @@ -9,7 +9,7 @@ namespace Envoy { * Base class for all envoy exceptions. */ #define envoy_try \ - assert(MainThreadSingleton::get().isMainThread()); \ + ASSERT(MainThreadSingleton::get().isMainThread()); \ try class EnvoyException : public std::runtime_error { From f7abf765160ee374da071691dfc911020942cdc9 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Mon, 14 Dec 2020 19:09:50 -0600 Subject: [PATCH 06/47] fix dependency Signed-off-by: chaoqin-li1123 --- include/envoy/common/BUILD | 3 --- include/envoy/common/exception.h | 4 +++- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/include/envoy/common/BUILD b/include/envoy/common/BUILD index 3df0a8718dd94..d120b7c0cc541 100644 --- a/include/envoy/common/BUILD +++ b/include/envoy/common/BUILD @@ -19,9 +19,6 @@ envoy_basic_cc_library( ], external_deps = ["abseil_optional"], include_prefix = "envoy/common", - deps = [ - "//source/common/singleton:threadsafe_singleton", - ], ) envoy_cc_library( diff --git a/include/envoy/common/exception.h b/include/envoy/common/exception.h index f10ede5c67ad7..1c7bd985e715f 100644 --- a/include/envoy/common/exception.h +++ b/include/envoy/common/exception.h @@ -12,6 +12,8 @@ namespace Envoy { ASSERT(MainThreadSingleton::get().isMainThread()); \ try +#define MainThreadSingleton InjectableSingleton + class EnvoyException : public std::runtime_error { public: EnvoyException(const std::string& message) : std::runtime_error(message) {} @@ -25,7 +27,7 @@ struct MainThread { std::thread::id main_thread_id_; }; -using MainThreadSingleton = InjectableSingleton; + /* bool isMainThread() { From 2b9db0da03ee0940cfaae04a875136fc2ac06e0f Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Mon, 14 Dec 2020 19:12:43 -0600 Subject: [PATCH 07/47] fix dependency Signed-off-by: chaoqin-li1123 --- include/envoy/common/exception.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/envoy/common/exception.h b/include/envoy/common/exception.h index 1c7bd985e715f..098368d0f220b 100644 --- a/include/envoy/common/exception.h +++ b/include/envoy/common/exception.h @@ -21,7 +21,7 @@ class EnvoyException : public std::runtime_error { struct MainThread { MainThread() : main_thread_id_{std::this_thread::get_id()} {} - isMainThread() { return main_thread_id_ == std::this_thread::get_id(); } + bool isMainThread() { return main_thread_id_ == std::this_thread::get_id(); } private: std::thread::id main_thread_id_; From 702dec0197f1cbf26c55f3fd44f63e76606e0eff Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Mon, 14 Dec 2020 20:37:34 -0600 Subject: [PATCH 08/47] fix format Signed-off-by: chaoqin-li1123 --- source/server/server.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/source/server/server.cc b/source/server/server.cc index 4eed0a0554ab2..6ddee6a147a11 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -553,8 +553,6 @@ void InstanceImpl::initialize(const Options& options, // instantiated (which in turn relies on runtime...). Runtime::LoaderSingleton::get().initialize(clusterManager()); - MainThreadSingleton - clusterManager().setPrimaryClustersInitializedCb( [this]() { onClusterManagerPrimaryInitializationComplete(); }); From 360087a6621a17ffb97ce6f212d1c76bb59e45ac Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Mon, 14 Dec 2020 23:22:15 -0600 Subject: [PATCH 09/47] use in server.cc Signed-off-by: chaoqin-li1123 --- include/envoy/common/exception.h | 2 +- source/server/server.cc | 6 +++--- source/server/server.h | 1 - 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/include/envoy/common/exception.h b/include/envoy/common/exception.h index 098368d0f220b..8813265097214 100644 --- a/include/envoy/common/exception.h +++ b/include/envoy/common/exception.h @@ -24,7 +24,7 @@ struct MainThread { bool isMainThread() { return main_thread_id_ == std::this_thread::get_id(); } private: - std::thread::id main_thread_id_; + const std::thread::id main_thread_id_; }; diff --git a/source/server/server.cc b/source/server/server.cc index 6ddee6a147a11..472ecf5138f3f 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -86,8 +86,7 @@ InstanceImpl::InstanceImpl( mutex_tracer_(options.mutexTracingEnabled() ? &Envoy::MutexTracerImpl::getOrCreateTracer() : nullptr), grpc_context_(store.symbolTable()), http_context_(store.symbolTable()), - router_context_(store.symbolTable()), process_context_(std::move(process_context)), - main_thread_id_(std::this_thread::get_id()), server_contexts_(*this) { + router_context_(store.symbolTable()), process_context_(std::move(process_context)), server_contexts_(*this) { try { if (!options.logPath().empty()) { try { @@ -809,7 +808,8 @@ InstanceImpl::registerCallback(Stage stage, StageCallbackWithCompletion callback } void InstanceImpl::notifyCallbacksForStage(Stage stage, Event::PostCb completion_cb) { - ASSERT(std::this_thread::get_id() == main_thread_id_); + // ASSERT(std::this_thread::get_id() == main_thread_id_); + ASSERT(MainThreadSingleton::get().isMainThread()); const auto it = stage_callbacks_.find(stage); if (it != stage_callbacks_.end()) { for (const StageCallback& callback : it->second) { diff --git a/source/server/server.h b/source/server/server.h index 15d950e5683d3..fd03a933806a6 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -364,7 +364,6 @@ class InstanceImpl final : Logger::Loggable, Router::ContextImpl router_context_; std::unique_ptr process_context_; std::unique_ptr heap_shrinker_; - const std::thread::id main_thread_id_; // initialization_time is a histogram for tracking the initialization time across hot restarts // whenever we have support for histogram merge across hot restarts. Stats::TimespanPtr initialization_timer_; From 48abc004598be3a38f375036bc13e432001e4d1f Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Tue, 15 Dec 2020 00:09:28 -0600 Subject: [PATCH 10/47] clean up Signed-off-by: chaoqin-li1123 --- include/envoy/common/exception.h | 9 --------- source/server/server.cc | 4 ++-- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/include/envoy/common/exception.h b/include/envoy/common/exception.h index 8813265097214..85e9176600338 100644 --- a/include/envoy/common/exception.h +++ b/include/envoy/common/exception.h @@ -27,13 +27,4 @@ struct MainThread { const std::thread::id main_thread_id_; }; - - -/* -bool isMainThread() { - static std::thread::id main_thread_id = std::this_thread::get_id(); - return main_thread_id == std::this_thread::get_id(); -} -*/ - } // namespace Envoy diff --git a/source/server/server.cc b/source/server/server.cc index 472ecf5138f3f..a80c41c8dcad0 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -86,7 +86,8 @@ InstanceImpl::InstanceImpl( mutex_tracer_(options.mutexTracingEnabled() ? &Envoy::MutexTracerImpl::getOrCreateTracer() : nullptr), grpc_context_(store.symbolTable()), http_context_(store.symbolTable()), - router_context_(store.symbolTable()), process_context_(std::move(process_context)), server_contexts_(*this) { + router_context_(store.symbolTable()), process_context_(std::move(process_context)), + server_contexts_(*this) { try { if (!options.logPath().empty()) { try { @@ -808,7 +809,6 @@ InstanceImpl::registerCallback(Stage stage, StageCallbackWithCompletion callback } void InstanceImpl::notifyCallbacksForStage(Stage stage, Event::PostCb completion_cb) { - // ASSERT(std::this_thread::get_id() == main_thread_id_); ASSERT(MainThreadSingleton::get().isMainThread()); const auto it = stage_callbacks_.find(stage); if (it != stage_callbacks_.end()) { From 230ad2120f29fe1556c9de425b456af4c275171d Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Tue, 15 Dec 2020 21:39:30 -0600 Subject: [PATCH 11/47] move main thread utility Signed-off-by: chaoqin-li1123 --- include/envoy/common/exception.h | 11 ----------- include/envoy/thread_local/BUILD | 1 + include/envoy/thread_local/thread_local.h | 14 ++++++++++++++ source/server/server.h | 4 ++++ 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/include/envoy/common/exception.h b/include/envoy/common/exception.h index 85e9176600338..f41e80e4a9f19 100644 --- a/include/envoy/common/exception.h +++ b/include/envoy/common/exception.h @@ -8,23 +8,12 @@ namespace Envoy { /** * Base class for all envoy exceptions. */ -#define envoy_try \ - ASSERT(MainThreadSingleton::get().isMainThread()); \ - try - -#define MainThreadSingleton InjectableSingleton class EnvoyException : public std::runtime_error { public: EnvoyException(const std::string& message) : std::runtime_error(message) {} }; -struct MainThread { - MainThread() : main_thread_id_{std::this_thread::get_id()} {} - bool isMainThread() { return main_thread_id_ == std::this_thread::get_id(); } -private: - const std::thread::id main_thread_id_; -}; } // namespace Envoy diff --git a/include/envoy/thread_local/BUILD b/include/envoy/thread_local/BUILD index 9a5e612354943..ab8e5f74a470d 100644 --- a/include/envoy/thread_local/BUILD +++ b/include/envoy/thread_local/BUILD @@ -15,6 +15,7 @@ envoy_cc_library( ":thread_local_object", "//include/envoy/event:dispatcher_interface", "//source/common/common:assert_lib", + "//source/common/singleton:threadsafe_singleton", ], ) diff --git a/include/envoy/thread_local/thread_local.h b/include/envoy/thread_local/thread_local.h index 8ff2ca99c8c00..eec48b99673a8 100644 --- a/include/envoy/thread_local/thread_local.h +++ b/include/envoy/thread_local/thread_local.h @@ -228,5 +228,19 @@ class Instance : public SlotAllocator { virtual Event::Dispatcher& dispatcher() PURE; }; +struct MainThread { + MainThread() : main_thread_id_{std::this_thread::get_id()} {} + bool isMainThread() { return main_thread_id_ == std::this_thread::get_id(); } + +private: + const std::thread::id main_thread_id_; +}; + +using MainThreadSingleton = InjectableSingleton + +#define envoy_try \ + ASSERT(MainThreadSingleton::get().isMainThread()); \ + try + } // namespace ThreadLocal } // namespace Envoy diff --git a/source/server/server.h b/source/server/server.h index fd03a933806a6..df92e9654cdde 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -414,5 +414,9 @@ class MetricSnapshotImpl : public Stats::MetricSnapshot { SystemTime snapshot_time_; }; + + + + } // namespace Server } // namespace Envoy From 402662dd1d5fea4b98edbdc04400c9281a44aa78 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Tue, 15 Dec 2020 22:04:50 -0600 Subject: [PATCH 12/47] move main thread utility Signed-off-by: chaoqin-li1123 --- include/envoy/common/exception.h | 2 -- include/envoy/thread_local/thread_local.h | 2 +- source/common/thread_local/thread_local_impl.cc | 16 ++++++++-------- source/common/thread_local/thread_local_impl.h | 3 +-- source/server/server.h | 4 ---- 5 files changed, 10 insertions(+), 17 deletions(-) diff --git a/include/envoy/common/exception.h b/include/envoy/common/exception.h index f41e80e4a9f19..4c2f2b2025c3b 100644 --- a/include/envoy/common/exception.h +++ b/include/envoy/common/exception.h @@ -14,6 +14,4 @@ class EnvoyException : public std::runtime_error { EnvoyException(const std::string& message) : std::runtime_error(message) {} }; - - } // namespace Envoy diff --git a/include/envoy/thread_local/thread_local.h b/include/envoy/thread_local/thread_local.h index eec48b99673a8..0d6585d025097 100644 --- a/include/envoy/thread_local/thread_local.h +++ b/include/envoy/thread_local/thread_local.h @@ -236,7 +236,7 @@ struct MainThread { const std::thread::id main_thread_id_; }; -using MainThreadSingleton = InjectableSingleton +using MainThreadSingleton = InjectableSingleton; #define envoy_try \ ASSERT(MainThreadSingleton::get().isMainThread()); \ diff --git a/source/common/thread_local/thread_local_impl.cc b/source/common/thread_local/thread_local_impl.cc index 0815236a31956..a5956d2125079 100644 --- a/source/common/thread_local/thread_local_impl.cc +++ b/source/common/thread_local/thread_local_impl.cc @@ -16,13 +16,13 @@ namespace ThreadLocal { thread_local InstanceImpl::ThreadLocalData InstanceImpl::thread_local_data_; InstanceImpl::~InstanceImpl() { - ASSERT(std::this_thread::get_id() == main_thread_id_); + ASSERT(MainThreadSingleton::get().isMainThread()); ASSERT(shutdown_); thread_local_data_.data_.clear(); } SlotPtr InstanceImpl::allocateSlot() { - ASSERT(std::this_thread::get_id() == main_thread_id_); + ASSERT(MainThreadSingleton::get().isMainThread()); ASSERT(!shutdown_); if (free_slot_indexes_.empty()) { @@ -91,7 +91,7 @@ void InstanceImpl::SlotImpl::runOnAllThreads(const UpdateCb& cb) { } void InstanceImpl::SlotImpl::set(InitializeCb cb) { - ASSERT(std::this_thread::get_id() == parent_.main_thread_id_); + ASSERT(MainThreadSingleton::get().isMainThread()); ASSERT(!parent_.shutdown_); for (Event::Dispatcher& dispatcher : parent_.registered_threads_) { @@ -105,7 +105,7 @@ void InstanceImpl::SlotImpl::set(InitializeCb cb) { } void InstanceImpl::registerThread(Event::Dispatcher& dispatcher, bool main_thread) { - ASSERT(std::this_thread::get_id() == main_thread_id_); + ASSERT(MainThreadSingleton::get().isMainThread()); ASSERT(!shutdown_); if (main_thread) { @@ -119,7 +119,7 @@ void InstanceImpl::registerThread(Event::Dispatcher& dispatcher, bool main_threa } void InstanceImpl::removeSlot(uint32_t slot) { - ASSERT(std::this_thread::get_id() == main_thread_id_); + ASSERT(MainThreadSingleton::get().isMainThread()); // When shutting down, we do not post slot removals to other threads. This is because the other // threads have already shut down and the dispatcher is no longer alive. There is also no reason @@ -146,7 +146,7 @@ void InstanceImpl::removeSlot(uint32_t slot) { } void InstanceImpl::runOnAllThreads(Event::PostCb cb) { - ASSERT(std::this_thread::get_id() == main_thread_id_); + ASSERT(MainThreadSingleton::get().isMainThread()); ASSERT(!shutdown_); for (Event::Dispatcher& dispatcher : registered_threads_) { @@ -158,7 +158,7 @@ void InstanceImpl::runOnAllThreads(Event::PostCb cb) { } void InstanceImpl::runOnAllThreads(Event::PostCb cb, Event::PostCb all_threads_complete_cb) { - ASSERT(std::this_thread::get_id() == main_thread_id_); + ASSERT(MainThreadSingleton::get().isMainThread()); ASSERT(!shutdown_); // Handle main thread first so that when the last worker thread wins, we could just call the // all_threads_complete_cb method. Parallelism of main thread execution is being traded off @@ -185,7 +185,7 @@ void InstanceImpl::setThreadLocal(uint32_t index, ThreadLocalObjectSharedPtr obj } void InstanceImpl::shutdownGlobalThreading() { - ASSERT(std::this_thread::get_id() == main_thread_id_); + ASSERT(MainThreadSingleton::get().isMainThread()); ASSERT(!shutdown_); shutdown_ = true; } diff --git a/source/common/thread_local/thread_local_impl.h b/source/common/thread_local/thread_local_impl.h index 7abed04991666..24aecfafdf5d5 100644 --- a/source/common/thread_local/thread_local_impl.h +++ b/source/common/thread_local/thread_local_impl.h @@ -19,7 +19,7 @@ namespace ThreadLocal { */ class InstanceImpl : Logger::Loggable, public NonCopyable, public Instance { public: - InstanceImpl() : main_thread_id_(std::this_thread::get_id()) {} + InstanceImpl() {} ~InstanceImpl() override; // ThreadLocal::Instance @@ -81,7 +81,6 @@ class InstanceImpl : Logger::Loggable, public NonCopyable, pub // A list of index of freed slots. std::list free_slot_indexes_; std::list> registered_threads_; - std::thread::id main_thread_id_; Event::Dispatcher* main_thread_dispatcher_{}; std::atomic shutdown_{}; diff --git a/source/server/server.h b/source/server/server.h index df92e9654cdde..fd03a933806a6 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -414,9 +414,5 @@ class MetricSnapshotImpl : public Stats::MetricSnapshot { SystemTime snapshot_time_; }; - - - - } // namespace Server } // namespace Envoy From 4624a04acb8c94d232f7f313944b6447c8336589 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Tue, 15 Dec 2020 22:09:42 -0600 Subject: [PATCH 13/47] move main thread utility Signed-off-by: chaoqin-li1123 --- source/common/thread_local/thread_local_impl.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/thread_local/thread_local_impl.cc b/source/common/thread_local/thread_local_impl.cc index a5956d2125079..1eb6e7494d300 100644 --- a/source/common/thread_local/thread_local_impl.cc +++ b/source/common/thread_local/thread_local_impl.cc @@ -6,7 +6,7 @@ #include #include "envoy/event/dispatcher.h" - +#include "common/singleton/threadsafe_singleton.h" #include "common/common/assert.h" #include "common/common/stl_helpers.h" From 079f10db39fddbc98a33d26375c1bad2f728fb1e Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Tue, 15 Dec 2020 22:14:34 -0600 Subject: [PATCH 14/47] move main thread utility Signed-off-by: chaoqin-li1123 --- include/envoy/thread_local/thread_local.h | 1 + source/common/thread_local/thread_local_impl.cc | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/include/envoy/thread_local/thread_local.h b/include/envoy/thread_local/thread_local.h index 0d6585d025097..3dde9006259d0 100644 --- a/include/envoy/thread_local/thread_local.h +++ b/include/envoy/thread_local/thread_local.h @@ -10,6 +10,7 @@ #include "envoy/thread_local/thread_local_object.h" #include "common/common/assert.h" +#include "common/singleton/threadsafe_singleton.h" namespace Envoy { namespace ThreadLocal { diff --git a/source/common/thread_local/thread_local_impl.cc b/source/common/thread_local/thread_local_impl.cc index 1eb6e7494d300..a5956d2125079 100644 --- a/source/common/thread_local/thread_local_impl.cc +++ b/source/common/thread_local/thread_local_impl.cc @@ -6,7 +6,7 @@ #include #include "envoy/event/dispatcher.h" -#include "common/singleton/threadsafe_singleton.h" + #include "common/common/assert.h" #include "common/common/stl_helpers.h" From bacbc44572a7fc30e24b1c241028433ea0c2e2de Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Tue, 15 Dec 2020 22:42:26 -0600 Subject: [PATCH 15/47] move main thread utility Signed-off-by: chaoqin-li1123 --- source/server/server.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/server/server.cc b/source/server/server.cc index a80c41c8dcad0..347fad3c1624d 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -809,7 +809,7 @@ InstanceImpl::registerCallback(Stage stage, StageCallbackWithCompletion callback } void InstanceImpl::notifyCallbacksForStage(Stage stage, Event::PostCb completion_cb) { - ASSERT(MainThreadSingleton::get().isMainThread()); + ASSERT(ThreadLocal::MainThreadSingleton::get().isMainThread()); const auto it = stage_callbacks_.find(stage); if (it != stage_callbacks_.end()) { for (const StageCallback& callback : it->second) { From 3bc0be00fe7469801e1b223e5d4973d630a91310 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Tue, 15 Dec 2020 23:05:06 -0600 Subject: [PATCH 16/47] move main thread utility Signed-off-by: chaoqin-li1123 --- source/server/server.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/server/server.cc b/source/server/server.cc index 347fad3c1624d..6a262d928aae9 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -324,7 +324,7 @@ void InstanceImpl::initialize(const Options& options, ENVOY_LOG(info, " {}: {}", ext.first, absl::StrJoin(ext.second->registeredNames(), ", ")); } - MainThreadSingleton::initialize(new MainThread()); + ThreadLocal::MainThreadSingleton::initialize(new ThreadLocal::MainThread()); // Handle configuration that needs to take place prior to the main configuration load. InstanceUtil::loadBootstrapConfig(bootstrap_, options, From dc708f753e659bf9fc53d5f256533e361cb59bdd Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Wed, 16 Dec 2020 09:54:13 -0600 Subject: [PATCH 17/47] change interface Signed-off-by: chaoqin-li1123 --- include/envoy/thread_local/thread_local.h | 17 +++++++++++------ source/common/thread_local/thread_local_impl.cc | 16 ++++++++-------- source/server/server.cc | 4 ++-- 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/include/envoy/thread_local/thread_local.h b/include/envoy/thread_local/thread_local.h index 3dde9006259d0..6efcbfd0f3634 100644 --- a/include/envoy/thread_local/thread_local.h +++ b/include/envoy/thread_local/thread_local.h @@ -230,17 +230,22 @@ class Instance : public SlotAllocator { }; struct MainThread { - MainThread() : main_thread_id_{std::this_thread::get_id()} {} - bool isMainThread() { return main_thread_id_ == std::this_thread::get_id(); } - + using MainThreadSingleton = InjectableSingleton; + MainThread() { + main_thread_id_ = std::this_thread::get_id(); + } + static void init() { + MainThreadSingleton::initialize(new MainThread()); + } + static bool isMainThread() const { return main_thread_id_ == std::this_thread::get_id(); } private: - const std::thread::id main_thread_id_; + static const std::thread::id main_thread_id_; }; -using MainThreadSingleton = InjectableSingleton; + #define envoy_try \ - ASSERT(MainThreadSingleton::get().isMainThread()); \ + ASSERT(ThreadLocal::MainThread::isMainThread()); \ try } // namespace ThreadLocal diff --git a/source/common/thread_local/thread_local_impl.cc b/source/common/thread_local/thread_local_impl.cc index a5956d2125079..dbda69c871079 100644 --- a/source/common/thread_local/thread_local_impl.cc +++ b/source/common/thread_local/thread_local_impl.cc @@ -16,13 +16,13 @@ namespace ThreadLocal { thread_local InstanceImpl::ThreadLocalData InstanceImpl::thread_local_data_; InstanceImpl::~InstanceImpl() { - ASSERT(MainThreadSingleton::get().isMainThread()); + ASSERT(ThreadLocal::MainThread::isMainThread()); ASSERT(shutdown_); thread_local_data_.data_.clear(); } SlotPtr InstanceImpl::allocateSlot() { - ASSERT(MainThreadSingleton::get().isMainThread()); + ASSERT(ThreadLocal::MainThread::isMainThread()); ASSERT(!shutdown_); if (free_slot_indexes_.empty()) { @@ -91,7 +91,7 @@ void InstanceImpl::SlotImpl::runOnAllThreads(const UpdateCb& cb) { } void InstanceImpl::SlotImpl::set(InitializeCb cb) { - ASSERT(MainThreadSingleton::get().isMainThread()); + ASSERT(ThreadLocal::MainThread::isMainThread()); ASSERT(!parent_.shutdown_); for (Event::Dispatcher& dispatcher : parent_.registered_threads_) { @@ -105,7 +105,7 @@ void InstanceImpl::SlotImpl::set(InitializeCb cb) { } void InstanceImpl::registerThread(Event::Dispatcher& dispatcher, bool main_thread) { - ASSERT(MainThreadSingleton::get().isMainThread()); + ASSERT(ThreadLocal::MainThread::isMainThread()); ASSERT(!shutdown_); if (main_thread) { @@ -119,7 +119,7 @@ void InstanceImpl::registerThread(Event::Dispatcher& dispatcher, bool main_threa } void InstanceImpl::removeSlot(uint32_t slot) { - ASSERT(MainThreadSingleton::get().isMainThread()); + ASSERT(ThreadLocal::MainThread::isMainThread()); // When shutting down, we do not post slot removals to other threads. This is because the other // threads have already shut down and the dispatcher is no longer alive. There is also no reason @@ -146,7 +146,7 @@ void InstanceImpl::removeSlot(uint32_t slot) { } void InstanceImpl::runOnAllThreads(Event::PostCb cb) { - ASSERT(MainThreadSingleton::get().isMainThread()); + ASSERT(ThreadLocal::MainThread::isMainThread()); ASSERT(!shutdown_); for (Event::Dispatcher& dispatcher : registered_threads_) { @@ -158,7 +158,7 @@ void InstanceImpl::runOnAllThreads(Event::PostCb cb) { } void InstanceImpl::runOnAllThreads(Event::PostCb cb, Event::PostCb all_threads_complete_cb) { - ASSERT(MainThreadSingleton::get().isMainThread()); + ASSERT(ThreadLocal::MainThread::isMainThread()); ASSERT(!shutdown_); // Handle main thread first so that when the last worker thread wins, we could just call the // all_threads_complete_cb method. Parallelism of main thread execution is being traded off @@ -185,7 +185,7 @@ void InstanceImpl::setThreadLocal(uint32_t index, ThreadLocalObjectSharedPtr obj } void InstanceImpl::shutdownGlobalThreading() { - ASSERT(MainThreadSingleton::get().isMainThread()); + ASSERT(ThreadLocal::MainThread::isMainThread()); ASSERT(!shutdown_); shutdown_ = true; } diff --git a/source/server/server.cc b/source/server/server.cc index 6a262d928aae9..b430f815e4e8b 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -324,7 +324,7 @@ void InstanceImpl::initialize(const Options& options, ENVOY_LOG(info, " {}: {}", ext.first, absl::StrJoin(ext.second->registeredNames(), ", ")); } - ThreadLocal::MainThreadSingleton::initialize(new ThreadLocal::MainThread()); + ThreadLocal::MainThread::init(); // Handle configuration that needs to take place prior to the main configuration load. InstanceUtil::loadBootstrapConfig(bootstrap_, options, @@ -809,7 +809,7 @@ InstanceImpl::registerCallback(Stage stage, StageCallbackWithCompletion callback } void InstanceImpl::notifyCallbacksForStage(Stage stage, Event::PostCb completion_cb) { - ASSERT(ThreadLocal::MainThreadSingleton::get().isMainThread()); + ASSERT(ThreadLocal::MainThread::isMainThread()); const auto it = stage_callbacks_.find(stage); if (it != stage_callbacks_.end()) { for (const StageCallback& callback : it->second) { From 545b8918114bbce746793c7fd7b5b34df715dba6 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Wed, 16 Dec 2020 09:59:08 -0600 Subject: [PATCH 18/47] change interface Signed-off-by: chaoqin-li1123 --- include/envoy/thread_local/thread_local.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/envoy/thread_local/thread_local.h b/include/envoy/thread_local/thread_local.h index 6efcbfd0f3634..26d19279048c0 100644 --- a/include/envoy/thread_local/thread_local.h +++ b/include/envoy/thread_local/thread_local.h @@ -239,7 +239,7 @@ struct MainThread { } static bool isMainThread() const { return main_thread_id_ == std::this_thread::get_id(); } private: - static const std::thread::id main_thread_id_; + static std::thread::id main_thread_id_; }; From 41f8cccecd93ed7367f9120899dacfbb856509f7 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Wed, 16 Dec 2020 10:03:20 -0600 Subject: [PATCH 19/47] change interface Signed-off-by: chaoqin-li1123 --- include/envoy/thread_local/thread_local.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/include/envoy/thread_local/thread_local.h b/include/envoy/thread_local/thread_local.h index 26d19279048c0..1ff5f51b06660 100644 --- a/include/envoy/thread_local/thread_local.h +++ b/include/envoy/thread_local/thread_local.h @@ -231,13 +231,11 @@ class Instance : public SlotAllocator { struct MainThread { using MainThreadSingleton = InjectableSingleton; - MainThread() { - main_thread_id_ = std::this_thread::get_id(); - } static void init() { + main_thread_id_ = std::this_thread::get_id(); MainThreadSingleton::initialize(new MainThread()); } - static bool isMainThread() const { return main_thread_id_ == std::this_thread::get_id(); } + static bool isMainThread() { return main_thread_id_ == std::this_thread::get_id(); } private: static std::thread::id main_thread_id_; }; From fc353f80769e382696cdb0092da729c9f343b938 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Wed, 16 Dec 2020 10:28:00 -0600 Subject: [PATCH 20/47] format Signed-off-by: chaoqin-li1123 --- include/envoy/thread_local/thread_local.h | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/include/envoy/thread_local/thread_local.h b/include/envoy/thread_local/thread_local.h index 1ff5f51b06660..c844fb01dde6c 100644 --- a/include/envoy/thread_local/thread_local.h +++ b/include/envoy/thread_local/thread_local.h @@ -232,18 +232,15 @@ class Instance : public SlotAllocator { struct MainThread { using MainThreadSingleton = InjectableSingleton; static void init() { - main_thread_id_ = std::this_thread::get_id(); + main_thread_id_ = std::this_thread::get_id(); MainThreadSingleton::initialize(new MainThread()); } - static bool isMainThread() { return main_thread_id_ == std::this_thread::get_id(); } -private: + static bool isMainThread() { return main_thread_id_ == std::this_thread::get_id(); } static std::thread::id main_thread_id_; }; - - #define envoy_try \ - ASSERT(ThreadLocal::MainThread::isMainThread()); \ + ASSERT(ThreadLocal::MainThread::isMainThread()); \ try } // namespace ThreadLocal From 67e7b944d0919a3619100ad52972749513931f02 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Wed, 16 Dec 2020 10:51:50 -0600 Subject: [PATCH 21/47] interface Signed-off-by: chaoqin-li1123 --- include/envoy/thread_local/thread_local.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/include/envoy/thread_local/thread_local.h b/include/envoy/thread_local/thread_local.h index c844fb01dde6c..0c891984496e3 100644 --- a/include/envoy/thread_local/thread_local.h +++ b/include/envoy/thread_local/thread_local.h @@ -231,12 +231,16 @@ class Instance : public SlotAllocator { struct MainThread { using MainThreadSingleton = InjectableSingleton; + MainThread(): main_thread_id_{std::this_thread::get_id()} {} + bool inMainThread() { + return main_thread_id_ == std::this_thread::get_id(); + } static void init() { - main_thread_id_ = std::this_thread::get_id(); MainThreadSingleton::initialize(new MainThread()); } - static bool isMainThread() { return main_thread_id_ == std::this_thread::get_id(); } - static std::thread::id main_thread_id_; + static bool isMainThread() { return MainThreadSingleton::get().inMainThread(); } +private: + std::thread::id main_thread_id_; }; #define envoy_try \ From d92cd8f26c81b7329d68c5688c4ee9e873625d24 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Wed, 16 Dec 2020 11:20:18 -0600 Subject: [PATCH 22/47] format Signed-off-by: chaoqin-li1123 --- include/envoy/thread_local/thread_local.h | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/include/envoy/thread_local/thread_local.h b/include/envoy/thread_local/thread_local.h index 0c891984496e3..8226bf59687d2 100644 --- a/include/envoy/thread_local/thread_local.h +++ b/include/envoy/thread_local/thread_local.h @@ -231,14 +231,11 @@ class Instance : public SlotAllocator { struct MainThread { using MainThreadSingleton = InjectableSingleton; - MainThread(): main_thread_id_{std::this_thread::get_id()} {} - bool inMainThread() { - return main_thread_id_ == std::this_thread::get_id(); - } - static void init() { - MainThreadSingleton::initialize(new MainThread()); - } + MainThread() : main_thread_id_{std::this_thread::get_id()} {} + bool inMainThread() { return main_thread_id_ == std::this_thread::get_id(); } + static void init() { MainThreadSingleton::initialize(new MainThread()); } static bool isMainThread() { return MainThreadSingleton::get().inMainThread(); } + private: std::thread::id main_thread_id_; }; From be7c2adea06742c4e474ee1bdefe2003f5b7e6b5 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Wed, 16 Dec 2020 13:30:52 -0600 Subject: [PATCH 23/47] fix test Signed-off-by: chaoqin-li1123 --- test/common/thread_local/thread_local_impl_test.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/test/common/thread_local/thread_local_impl_test.cc b/test/common/thread_local/thread_local_impl_test.cc index 59bdd6d0080b7..6b918750ca0b7 100644 --- a/test/common/thread_local/thread_local_impl_test.cc +++ b/test/common/thread_local/thread_local_impl_test.cc @@ -25,6 +25,7 @@ class TestThreadLocalObject : public ThreadLocalObject { class ThreadLocalInstanceImplTest : public testing::Test { public: ThreadLocalInstanceImplTest() { + ThreadLocal::MainThread::init(); tls_.registerThread(main_dispatcher_, true); EXPECT_EQ(&main_dispatcher_, &tls_.dispatcher()); EXPECT_CALL(thread_dispatcher_, post(_)); From f4d44ec63919af2ff9d370f018fce81be7e97677 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Wed, 16 Dec 2020 21:54:55 -0600 Subject: [PATCH 24/47] fix test Signed-off-by: chaoqin-li1123 --- test/common/thread_local/thread_local_impl_test.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/test/common/thread_local/thread_local_impl_test.cc b/test/common/thread_local/thread_local_impl_test.cc index 6b918750ca0b7..59bdd6d0080b7 100644 --- a/test/common/thread_local/thread_local_impl_test.cc +++ b/test/common/thread_local/thread_local_impl_test.cc @@ -25,7 +25,6 @@ class TestThreadLocalObject : public ThreadLocalObject { class ThreadLocalInstanceImplTest : public testing::Test { public: ThreadLocalInstanceImplTest() { - ThreadLocal::MainThread::init(); tls_.registerThread(main_dispatcher_, true); EXPECT_EQ(&main_dispatcher_, &tls_.dispatcher()); EXPECT_CALL(thread_dispatcher_, post(_)); From c28f6342a0a696aabdf9e56b9f884610b5b3a4de Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Wed, 16 Dec 2020 22:21:25 -0600 Subject: [PATCH 25/47] fix test Signed-off-by: chaoqin-li1123 --- include/envoy/thread_local/thread_local.h | 1 + source/server/server.cc | 1 + test/common/thread_local/thread_local_impl_test.cc | 4 ++++ 3 files changed, 6 insertions(+) diff --git a/include/envoy/thread_local/thread_local.h b/include/envoy/thread_local/thread_local.h index 8226bf59687d2..8bd39fb044e15 100644 --- a/include/envoy/thread_local/thread_local.h +++ b/include/envoy/thread_local/thread_local.h @@ -234,6 +234,7 @@ struct MainThread { MainThread() : main_thread_id_{std::this_thread::get_id()} {} bool inMainThread() { return main_thread_id_ == std::this_thread::get_id(); } static void init() { MainThreadSingleton::initialize(new MainThread()); } + static void clear() { MainThreadSingleton::clear(); } static bool isMainThread() { return MainThreadSingleton::get().inMainThread(); } private: diff --git a/source/server/server.cc b/source/server/server.cc index b430f815e4e8b..6bbc8aeab9ecd 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -137,6 +137,7 @@ InstanceImpl::~InstanceImpl() { ENVOY_LOG(debug, "destroying listener manager"); listener_manager_.reset(); ENVOY_LOG(debug, "destroyed listener manager"); + ThreadLocal::MainThread::clear(); } Upstream::ClusterManager& InstanceImpl::clusterManager() { return *config_.clusterManager(); } diff --git a/test/common/thread_local/thread_local_impl_test.cc b/test/common/thread_local/thread_local_impl_test.cc index 59bdd6d0080b7..1e4cbbade0371 100644 --- a/test/common/thread_local/thread_local_impl_test.cc +++ b/test/common/thread_local/thread_local_impl_test.cc @@ -25,11 +25,15 @@ class TestThreadLocalObject : public ThreadLocalObject { class ThreadLocalInstanceImplTest : public testing::Test { public: ThreadLocalInstanceImplTest() { + ThreadLocal::MainThread::init(); tls_.registerThread(main_dispatcher_, true); EXPECT_EQ(&main_dispatcher_, &tls_.dispatcher()); EXPECT_CALL(thread_dispatcher_, post(_)); tls_.registerThread(thread_dispatcher_, false); } + ~ThreadLocalInstanceImplTest() { + ThreadLocal::MainThread::clear(); + } MOCK_METHOD(ThreadLocalObjectSharedPtr, createThreadLocal, (Event::Dispatcher & dispatcher)); From c62f434bbb59b09d15ab313f20b14d51e62e9092 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Wed, 16 Dec 2020 23:00:37 -0600 Subject: [PATCH 26/47] fix test Signed-off-by: chaoqin-li1123 --- source/server/server.cc | 3 ++- test/common/thread_local/thread_local_impl_test.cc | 4 +--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/source/server/server.cc b/source/server/server.cc index 6bbc8aeab9ecd..cc462e7e1bcbb 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -88,6 +88,7 @@ InstanceImpl::InstanceImpl( grpc_context_(store.symbolTable()), http_context_(store.symbolTable()), router_context_(store.symbolTable()), process_context_(std::move(process_context)), server_contexts_(*this) { + ThreadLocal::MainThread::init(); try { if (!options.logPath().empty()) { try { @@ -325,7 +326,7 @@ void InstanceImpl::initialize(const Options& options, ENVOY_LOG(info, " {}: {}", ext.first, absl::StrJoin(ext.second->registeredNames(), ", ")); } - ThreadLocal::MainThread::init(); + // Handle configuration that needs to take place prior to the main configuration load. InstanceUtil::loadBootstrapConfig(bootstrap_, options, diff --git a/test/common/thread_local/thread_local_impl_test.cc b/test/common/thread_local/thread_local_impl_test.cc index 1e4cbbade0371..c37dd221e0501 100644 --- a/test/common/thread_local/thread_local_impl_test.cc +++ b/test/common/thread_local/thread_local_impl_test.cc @@ -25,15 +25,13 @@ class TestThreadLocalObject : public ThreadLocalObject { class ThreadLocalInstanceImplTest : public testing::Test { public: ThreadLocalInstanceImplTest() { + ThreadLocal::MainThread::clear(); ThreadLocal::MainThread::init(); tls_.registerThread(main_dispatcher_, true); EXPECT_EQ(&main_dispatcher_, &tls_.dispatcher()); EXPECT_CALL(thread_dispatcher_, post(_)); tls_.registerThread(thread_dispatcher_, false); } - ~ThreadLocalInstanceImplTest() { - ThreadLocal::MainThread::clear(); - } MOCK_METHOD(ThreadLocalObjectSharedPtr, createThreadLocal, (Event::Dispatcher & dispatcher)); From 7ad497186af45a712d32754718b3b47a789573da Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Wed, 16 Dec 2020 23:09:17 -0600 Subject: [PATCH 27/47] fix test Signed-off-by: chaoqin-li1123 --- source/common/thread_local/thread_local_impl.cc | 1 + source/common/thread_local/thread_local_impl.h | 4 +++- source/server/server.cc | 4 ++-- test/common/thread_local/thread_local_impl_test.cc | 2 -- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/source/common/thread_local/thread_local_impl.cc b/source/common/thread_local/thread_local_impl.cc index dbda69c871079..918c4431db63a 100644 --- a/source/common/thread_local/thread_local_impl.cc +++ b/source/common/thread_local/thread_local_impl.cc @@ -19,6 +19,7 @@ InstanceImpl::~InstanceImpl() { ASSERT(ThreadLocal::MainThread::isMainThread()); ASSERT(shutdown_); thread_local_data_.data_.clear(); + ThreadLocal::MainThread::clear(); } SlotPtr InstanceImpl::allocateSlot() { diff --git a/source/common/thread_local/thread_local_impl.h b/source/common/thread_local/thread_local_impl.h index 24aecfafdf5d5..d32250d2ddc1a 100644 --- a/source/common/thread_local/thread_local_impl.h +++ b/source/common/thread_local/thread_local_impl.h @@ -19,7 +19,9 @@ namespace ThreadLocal { */ class InstanceImpl : Logger::Loggable, public NonCopyable, public Instance { public: - InstanceImpl() {} + InstanceImpl() { + ThreadLocal::MainThread::init(); + } ~InstanceImpl() override; // ThreadLocal::Instance diff --git a/source/server/server.cc b/source/server/server.cc index cc462e7e1bcbb..e51e30343a34e 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -88,7 +88,7 @@ InstanceImpl::InstanceImpl( grpc_context_(store.symbolTable()), http_context_(store.symbolTable()), router_context_(store.symbolTable()), process_context_(std::move(process_context)), server_contexts_(*this) { - ThreadLocal::MainThread::init(); + // ThreadLocal::MainThread::init(); try { if (!options.logPath().empty()) { try { @@ -138,7 +138,7 @@ InstanceImpl::~InstanceImpl() { ENVOY_LOG(debug, "destroying listener manager"); listener_manager_.reset(); ENVOY_LOG(debug, "destroyed listener manager"); - ThreadLocal::MainThread::clear(); + // ThreadLocal::MainThread::clear(); } Upstream::ClusterManager& InstanceImpl::clusterManager() { return *config_.clusterManager(); } diff --git a/test/common/thread_local/thread_local_impl_test.cc b/test/common/thread_local/thread_local_impl_test.cc index c37dd221e0501..59bdd6d0080b7 100644 --- a/test/common/thread_local/thread_local_impl_test.cc +++ b/test/common/thread_local/thread_local_impl_test.cc @@ -25,8 +25,6 @@ class TestThreadLocalObject : public ThreadLocalObject { class ThreadLocalInstanceImplTest : public testing::Test { public: ThreadLocalInstanceImplTest() { - ThreadLocal::MainThread::clear(); - ThreadLocal::MainThread::init(); tls_.registerThread(main_dispatcher_, true); EXPECT_EQ(&main_dispatcher_, &tls_.dispatcher()); EXPECT_CALL(thread_dispatcher_, post(_)); From fadce0affedc8603a0ef43968faf0ce48da0c5a7 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Thu, 17 Dec 2020 00:44:04 -0600 Subject: [PATCH 28/47] fix format Signed-off-by: chaoqin-li1123 --- source/common/thread_local/thread_local_impl.h | 4 +--- source/server/server.cc | 4 ---- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/source/common/thread_local/thread_local_impl.h b/source/common/thread_local/thread_local_impl.h index d32250d2ddc1a..f5f7a6de07382 100644 --- a/source/common/thread_local/thread_local_impl.h +++ b/source/common/thread_local/thread_local_impl.h @@ -19,9 +19,7 @@ namespace ThreadLocal { */ class InstanceImpl : Logger::Loggable, public NonCopyable, public Instance { public: - InstanceImpl() { - ThreadLocal::MainThread::init(); - } + InstanceImpl() { ThreadLocal::MainThread::init(); } ~InstanceImpl() override; // ThreadLocal::Instance diff --git a/source/server/server.cc b/source/server/server.cc index e51e30343a34e..96f364d79181e 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -88,7 +88,6 @@ InstanceImpl::InstanceImpl( grpc_context_(store.symbolTable()), http_context_(store.symbolTable()), router_context_(store.symbolTable()), process_context_(std::move(process_context)), server_contexts_(*this) { - // ThreadLocal::MainThread::init(); try { if (!options.logPath().empty()) { try { @@ -138,7 +137,6 @@ InstanceImpl::~InstanceImpl() { ENVOY_LOG(debug, "destroying listener manager"); listener_manager_.reset(); ENVOY_LOG(debug, "destroyed listener manager"); - // ThreadLocal::MainThread::clear(); } Upstream::ClusterManager& InstanceImpl::clusterManager() { return *config_.clusterManager(); } @@ -326,8 +324,6 @@ void InstanceImpl::initialize(const Options& options, ENVOY_LOG(info, " {}: {}", ext.first, absl::StrJoin(ext.second->registeredNames(), ", ")); } - - // Handle configuration that needs to take place prior to the main configuration load. InstanceUtil::loadBootstrapConfig(bootstrap_, options, messageValidationContext().staticValidationVisitor(), *api_); From 6d6947bb7bf0fd1deec1d52db6fa0804810ac5ad Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Thu, 17 Dec 2020 10:41:06 -0600 Subject: [PATCH 29/47] fix memory leak Signed-off-by: chaoqin-li1123 --- include/envoy/thread_local/thread_local.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/include/envoy/thread_local/thread_local.h b/include/envoy/thread_local/thread_local.h index 8bd39fb044e15..6ee2d1ebfa444 100644 --- a/include/envoy/thread_local/thread_local.h +++ b/include/envoy/thread_local/thread_local.h @@ -231,14 +231,17 @@ class Instance : public SlotAllocator { struct MainThread { using MainThreadSingleton = InjectableSingleton; - MainThread() : main_thread_id_{std::this_thread::get_id()} {} bool inMainThread() { return main_thread_id_ == std::this_thread::get_id(); } - static void init() { MainThreadSingleton::initialize(new MainThread()); } + static void init() { + main_thread_ = std::make_unique(); + MainThreadSingleton::initialize(main_thread_.get()); + } static void clear() { MainThreadSingleton::clear(); } static bool isMainThread() { return MainThreadSingleton::get().inMainThread(); } private: - std::thread::id main_thread_id_; + std::thread::id main_thread_id_{std::this_thread::get_id()}; + static std::unique_ptr main_thread_; }; #define envoy_try \ From 1caa503f0ec89a5cb6526f307d3c94324262b3a3 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Thu, 17 Dec 2020 10:52:36 -0600 Subject: [PATCH 30/47] fix memory leak Signed-off-by: chaoqin-li1123 --- include/envoy/thread_local/thread_local.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/envoy/thread_local/thread_local.h b/include/envoy/thread_local/thread_local.h index 6ee2d1ebfa444..3bc8f9508a392 100644 --- a/include/envoy/thread_local/thread_local.h +++ b/include/envoy/thread_local/thread_local.h @@ -244,6 +244,8 @@ struct MainThread { static std::unique_ptr main_thread_; }; +std::unique_ptr MainThread::main_thread_; + #define envoy_try \ ASSERT(ThreadLocal::MainThread::isMainThread()); \ try From 4df058b7c74752911e52ee3ac5b3c09958f2606f Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Thu, 17 Dec 2020 10:58:54 -0600 Subject: [PATCH 31/47] fix memory leak Signed-off-by: chaoqin-li1123 --- include/envoy/thread_local/thread_local.h | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/include/envoy/thread_local/thread_local.h b/include/envoy/thread_local/thread_local.h index 3bc8f9508a392..d8d4b23b79d96 100644 --- a/include/envoy/thread_local/thread_local.h +++ b/include/envoy/thread_local/thread_local.h @@ -233,19 +233,17 @@ struct MainThread { using MainThreadSingleton = InjectableSingleton; bool inMainThread() { return main_thread_id_ == std::this_thread::get_id(); } static void init() { - main_thread_ = std::make_unique(); - MainThreadSingleton::initialize(main_thread_.get()); + static MainThread main_thread; + main_thread = MainThread(); + MainThreadSingleton::initialize(&main_thread); } static void clear() { MainThreadSingleton::clear(); } static bool isMainThread() { return MainThreadSingleton::get().inMainThread(); } private: std::thread::id main_thread_id_{std::this_thread::get_id()}; - static std::unique_ptr main_thread_; }; -std::unique_ptr MainThread::main_thread_; - #define envoy_try \ ASSERT(ThreadLocal::MainThread::isMainThread()); \ try From cf9592662b4e6932c4bb87069f87cbd2d2a3b0c7 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Thu, 17 Dec 2020 11:14:35 -0600 Subject: [PATCH 32/47] fix format Signed-off-by: chaoqin-li1123 --- include/envoy/common/exception.h | 3 --- include/envoy/thread_local/thread_local.h | 4 ++-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/include/envoy/common/exception.h b/include/envoy/common/exception.h index 4c2f2b2025c3b..d5b9255ead1e2 100644 --- a/include/envoy/common/exception.h +++ b/include/envoy/common/exception.h @@ -2,16 +2,13 @@ #include #include -#include namespace Envoy { /** * Base class for all envoy exceptions. */ - class EnvoyException : public std::runtime_error { public: EnvoyException(const std::string& message) : std::runtime_error(message) {} }; - } // namespace Envoy diff --git a/include/envoy/thread_local/thread_local.h b/include/envoy/thread_local/thread_local.h index d8d4b23b79d96..c492cb9343603 100644 --- a/include/envoy/thread_local/thread_local.h +++ b/include/envoy/thread_local/thread_local.h @@ -232,10 +232,10 @@ class Instance : public SlotAllocator { struct MainThread { using MainThreadSingleton = InjectableSingleton; bool inMainThread() { return main_thread_id_ == std::this_thread::get_id(); } - static void init() { + static void init() { static MainThread main_thread; main_thread = MainThread(); - MainThreadSingleton::initialize(&main_thread); + MainThreadSingleton::initialize(&main_thread); } static void clear() { MainThreadSingleton::clear(); } static bool isMainThread() { return MainThreadSingleton::get().inMainThread(); } From d4fa9ffa05ec34e714f700b114bdc4fe5ab53e0a Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Thu, 17 Dec 2020 19:30:38 -0600 Subject: [PATCH 33/47] clean up Signed-off-by: chaoqin-li1123 --- include/envoy/thread_local/thread_local.h | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/include/envoy/thread_local/thread_local.h b/include/envoy/thread_local/thread_local.h index c492cb9343603..036a7825f4845 100644 --- a/include/envoy/thread_local/thread_local.h +++ b/include/envoy/thread_local/thread_local.h @@ -231,22 +231,17 @@ class Instance : public SlotAllocator { struct MainThread { using MainThreadSingleton = InjectableSingleton; - bool inMainThread() { return main_thread_id_ == std::this_thread::get_id(); } - static void init() { - static MainThread main_thread; - main_thread = MainThread(); - MainThreadSingleton::initialize(&main_thread); + bool inMainThread() const { return main_thread_id_ == std::this_thread::get_id(); } + static void init() { MainThreadSingleton::initialize(new MainThread()); } + static void clear() { + free(MainThreadSingleton::getExisting()); + MainThreadSingleton::clear(); } - static void clear() { MainThreadSingleton::clear(); } static bool isMainThread() { return MainThreadSingleton::get().inMainThread(); } private: std::thread::id main_thread_id_{std::this_thread::get_id()}; }; -#define envoy_try \ - ASSERT(ThreadLocal::MainThread::isMainThread()); \ - try - } // namespace ThreadLocal } // namespace Envoy From 0df06507b7d62753e4a7c7cfe4dd53e3f5b129b4 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Wed, 23 Dec 2020 12:03:33 -0600 Subject: [PATCH 34/47] move main thread utility to thread.h Signed-off-by: chaoqin-li1123 --- include/envoy/thread/BUILD | 5 ++++- include/envoy/thread/thread.h | 14 ++++++++++++++ include/envoy/thread_local/BUILD | 1 - include/envoy/thread_local/thread_local.h | 14 -------------- .../common/thread_local/thread_local_impl.cc | 18 +++++++++--------- source/common/thread_local/thread_local_impl.h | 2 +- source/server/server.cc | 2 +- 7 files changed, 29 insertions(+), 27 deletions(-) diff --git a/include/envoy/thread/BUILD b/include/envoy/thread/BUILD index d239377668505..0bd900efb3596 100644 --- a/include/envoy/thread/BUILD +++ b/include/envoy/thread/BUILD @@ -11,5 +11,8 @@ envoy_package() envoy_cc_library( name = "thread_interface", hdrs = ["thread.h"], - deps = ["//source/common/common:thread_annotations"], + deps = [ + "//source/common/common:thread_annotations", + "//source/common/singleton:threadsafe_singleton", + ], ) diff --git a/include/envoy/thread/thread.h b/include/envoy/thread/thread.h index 6a2cce96eeb0f..bf3c8dbc189a7 100644 --- a/include/envoy/thread/thread.h +++ b/include/envoy/thread/thread.h @@ -97,5 +97,19 @@ class ABSL_LOCKABLE BasicLockable { virtual void unlock() ABSL_UNLOCK_FUNCTION() PURE; }; +struct MainThread { + using MainThreadSingleton = InjectableSingleton; + bool inMainThread() const { return main_thread_id_ == std::this_thread::get_id(); } + static void init() { MainThreadSingleton::initialize(new MainThread()); } + static void clear() { + free(MainThreadSingleton::getExisting()); + MainThreadSingleton::clear(); + } + static bool isMainThread() { return MainThreadSingleton::get().inMainThread(); } + +private: + std::thread::id main_thread_id_{std::this_thread::get_id()}; +}; + } // namespace Thread } // namespace Envoy diff --git a/include/envoy/thread_local/BUILD b/include/envoy/thread_local/BUILD index ab8e5f74a470d..9a5e612354943 100644 --- a/include/envoy/thread_local/BUILD +++ b/include/envoy/thread_local/BUILD @@ -15,7 +15,6 @@ envoy_cc_library( ":thread_local_object", "//include/envoy/event:dispatcher_interface", "//source/common/common:assert_lib", - "//source/common/singleton:threadsafe_singleton", ], ) diff --git a/include/envoy/thread_local/thread_local.h b/include/envoy/thread_local/thread_local.h index 036a7825f4845..d1b7e62d29fe6 100644 --- a/include/envoy/thread_local/thread_local.h +++ b/include/envoy/thread_local/thread_local.h @@ -229,19 +229,5 @@ class Instance : public SlotAllocator { virtual Event::Dispatcher& dispatcher() PURE; }; -struct MainThread { - using MainThreadSingleton = InjectableSingleton; - bool inMainThread() const { return main_thread_id_ == std::this_thread::get_id(); } - static void init() { MainThreadSingleton::initialize(new MainThread()); } - static void clear() { - free(MainThreadSingleton::getExisting()); - MainThreadSingleton::clear(); - } - static bool isMainThread() { return MainThreadSingleton::get().inMainThread(); } - -private: - std::thread::id main_thread_id_{std::this_thread::get_id()}; -}; - } // namespace ThreadLocal } // namespace Envoy diff --git a/source/common/thread_local/thread_local_impl.cc b/source/common/thread_local/thread_local_impl.cc index 918c4431db63a..9bf485419745b 100644 --- a/source/common/thread_local/thread_local_impl.cc +++ b/source/common/thread_local/thread_local_impl.cc @@ -16,14 +16,14 @@ namespace ThreadLocal { thread_local InstanceImpl::ThreadLocalData InstanceImpl::thread_local_data_; InstanceImpl::~InstanceImpl() { - ASSERT(ThreadLocal::MainThread::isMainThread()); + ASSERT(Thread::MainThread::isMainThread()); ASSERT(shutdown_); thread_local_data_.data_.clear(); - ThreadLocal::MainThread::clear(); + Thread::MainThread::clear(); } SlotPtr InstanceImpl::allocateSlot() { - ASSERT(ThreadLocal::MainThread::isMainThread()); + ASSERT(Thread::MainThread::isMainThread()); ASSERT(!shutdown_); if (free_slot_indexes_.empty()) { @@ -92,7 +92,7 @@ void InstanceImpl::SlotImpl::runOnAllThreads(const UpdateCb& cb) { } void InstanceImpl::SlotImpl::set(InitializeCb cb) { - ASSERT(ThreadLocal::MainThread::isMainThread()); + ASSERT(Thread::MainThread::isMainThread()); ASSERT(!parent_.shutdown_); for (Event::Dispatcher& dispatcher : parent_.registered_threads_) { @@ -106,7 +106,7 @@ void InstanceImpl::SlotImpl::set(InitializeCb cb) { } void InstanceImpl::registerThread(Event::Dispatcher& dispatcher, bool main_thread) { - ASSERT(ThreadLocal::MainThread::isMainThread()); + ASSERT(Thread::MainThread::isMainThread()); ASSERT(!shutdown_); if (main_thread) { @@ -120,7 +120,7 @@ void InstanceImpl::registerThread(Event::Dispatcher& dispatcher, bool main_threa } void InstanceImpl::removeSlot(uint32_t slot) { - ASSERT(ThreadLocal::MainThread::isMainThread()); + ASSERT(Thread::MainThread::isMainThread()); // When shutting down, we do not post slot removals to other threads. This is because the other // threads have already shut down and the dispatcher is no longer alive. There is also no reason @@ -147,7 +147,7 @@ void InstanceImpl::removeSlot(uint32_t slot) { } void InstanceImpl::runOnAllThreads(Event::PostCb cb) { - ASSERT(ThreadLocal::MainThread::isMainThread()); + ASSERT(Thread::MainThread::isMainThread()); ASSERT(!shutdown_); for (Event::Dispatcher& dispatcher : registered_threads_) { @@ -159,7 +159,7 @@ void InstanceImpl::runOnAllThreads(Event::PostCb cb) { } void InstanceImpl::runOnAllThreads(Event::PostCb cb, Event::PostCb all_threads_complete_cb) { - ASSERT(ThreadLocal::MainThread::isMainThread()); + ASSERT(Thread::MainThread::isMainThread()); ASSERT(!shutdown_); // Handle main thread first so that when the last worker thread wins, we could just call the // all_threads_complete_cb method. Parallelism of main thread execution is being traded off @@ -186,7 +186,7 @@ void InstanceImpl::setThreadLocal(uint32_t index, ThreadLocalObjectSharedPtr obj } void InstanceImpl::shutdownGlobalThreading() { - ASSERT(ThreadLocal::MainThread::isMainThread()); + ASSERT(Thread::MainThread::isMainThread()); ASSERT(!shutdown_); shutdown_ = true; } diff --git a/source/common/thread_local/thread_local_impl.h b/source/common/thread_local/thread_local_impl.h index f5f7a6de07382..e098723d72321 100644 --- a/source/common/thread_local/thread_local_impl.h +++ b/source/common/thread_local/thread_local_impl.h @@ -19,7 +19,7 @@ namespace ThreadLocal { */ class InstanceImpl : Logger::Loggable, public NonCopyable, public Instance { public: - InstanceImpl() { ThreadLocal::MainThread::init(); } + InstanceImpl() { Thread::MainThread::init(); } ~InstanceImpl() override; // ThreadLocal::Instance diff --git a/source/server/server.cc b/source/server/server.cc index 96f364d79181e..d90959b55c15e 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -807,7 +807,7 @@ InstanceImpl::registerCallback(Stage stage, StageCallbackWithCompletion callback } void InstanceImpl::notifyCallbacksForStage(Stage stage, Event::PostCb completion_cb) { - ASSERT(ThreadLocal::MainThread::isMainThread()); + ASSERT(Thread::MainThread::isMainThread()); const auto it = stage_callbacks_.find(stage); if (it != stage_callbacks_.end()) { for (const StageCallback& callback : it->second) { From 86c21c91303d2680d26eed85d4079580794ad13b Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Wed, 23 Dec 2020 12:19:39 -0600 Subject: [PATCH 35/47] move main thread utility to thread.h Signed-off-by: chaoqin-li1123 --- include/envoy/thread/BUILD | 1 - include/envoy/thread/thread.h | 14 -------------- source/common/common/BUILD | 1 + source/common/common/thread.h | 14 ++++++++++++++ 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/include/envoy/thread/BUILD b/include/envoy/thread/BUILD index 0bd900efb3596..5f3488c6de294 100644 --- a/include/envoy/thread/BUILD +++ b/include/envoy/thread/BUILD @@ -13,6 +13,5 @@ envoy_cc_library( hdrs = ["thread.h"], deps = [ "//source/common/common:thread_annotations", - "//source/common/singleton:threadsafe_singleton", ], ) diff --git a/include/envoy/thread/thread.h b/include/envoy/thread/thread.h index bf3c8dbc189a7..6a2cce96eeb0f 100644 --- a/include/envoy/thread/thread.h +++ b/include/envoy/thread/thread.h @@ -97,19 +97,5 @@ class ABSL_LOCKABLE BasicLockable { virtual void unlock() ABSL_UNLOCK_FUNCTION() PURE; }; -struct MainThread { - using MainThreadSingleton = InjectableSingleton; - bool inMainThread() const { return main_thread_id_ == std::this_thread::get_id(); } - static void init() { MainThreadSingleton::initialize(new MainThread()); } - static void clear() { - free(MainThreadSingleton::getExisting()); - MainThreadSingleton::clear(); - } - static bool isMainThread() { return MainThreadSingleton::get().inMainThread(); } - -private: - std::thread::id main_thread_id_{std::this_thread::get_id()}; -}; - } // namespace Thread } // namespace Envoy diff --git a/source/common/common/BUILD b/source/common/common/BUILD index f5632b8fee279..bf7701a21fa88 100644 --- a/source/common/common/BUILD +++ b/source/common/common/BUILD @@ -320,6 +320,7 @@ envoy_cc_library( external_deps = ["abseil_synchronization"], deps = envoy_cc_platform_dep("thread_impl_lib") + [ ":non_copyable", + "//source/common/singleton:threadsafe_singleton", ], ) diff --git a/source/common/common/thread.h b/source/common/common/thread.h index 4808d391dfbdc..3666059766380 100644 --- a/source/common/common/thread.h +++ b/source/common/common/thread.h @@ -168,5 +168,19 @@ class AtomicPtr : private AtomicPtrArray { T* get(const MakeObject& make_object) { return BaseClass::get(0, make_object); } }; +struct MainThread { + using MainThreadSingleton = InjectableSingleton; + bool inMainThread() const { return main_thread_id_ == std::this_thread::get_id(); } + static void init() { MainThreadSingleton::initialize(new MainThread()); } + static void clear() { + free(MainThreadSingleton::getExisting()); + MainThreadSingleton::clear(); + } + static bool isMainThread() { return MainThreadSingleton::get().inMainThread(); } + +private: + std::thread::id main_thread_id_{std::this_thread::get_id()}; +}; + } // namespace Thread } // namespace Envoy From fff8dc9fe2b0a8119208ddc5383655c138de20e2 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Wed, 23 Dec 2020 12:21:45 -0600 Subject: [PATCH 36/47] move main thread utility to thread.h Signed-off-by: chaoqin-li1123 --- include/envoy/thread_local/thread_local.h | 1 - source/common/common/thread.h | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/include/envoy/thread_local/thread_local.h b/include/envoy/thread_local/thread_local.h index d1b7e62d29fe6..8ff2ca99c8c00 100644 --- a/include/envoy/thread_local/thread_local.h +++ b/include/envoy/thread_local/thread_local.h @@ -10,7 +10,6 @@ #include "envoy/thread_local/thread_local_object.h" #include "common/common/assert.h" -#include "common/singleton/threadsafe_singleton.h" namespace Envoy { namespace ThreadLocal { diff --git a/source/common/common/thread.h b/source/common/common/thread.h index 3666059766380..8c9d991268aa0 100644 --- a/source/common/common/thread.h +++ b/source/common/common/thread.h @@ -8,6 +8,7 @@ #include "envoy/thread/thread.h" #include "common/common/non_copyable.h" +#include "common/singleton/threadsafe_singleton.h" #include "absl/synchronization/mutex.h" From 8ca36338ca730fc15441054f626b35ac6a0cfa6e Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Wed, 23 Dec 2020 17:43:28 -0600 Subject: [PATCH 37/47] format Signed-off-by: chaoqin-li1123 --- include/envoy/thread/BUILD | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/include/envoy/thread/BUILD b/include/envoy/thread/BUILD index 5f3488c6de294..d239377668505 100644 --- a/include/envoy/thread/BUILD +++ b/include/envoy/thread/BUILD @@ -11,7 +11,5 @@ envoy_package() envoy_cc_library( name = "thread_interface", hdrs = ["thread.h"], - deps = [ - "//source/common/common:thread_annotations", - ], + deps = ["//source/common/common:thread_annotations"], ) From 82ce44faaa9d58a882b0f8d4cc4c77bd851f05d4 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Tue, 5 Jan 2021 16:24:11 -0600 Subject: [PATCH 38/47] add unit test Signed-off-by: chaoqin-li1123 --- test/common/thread_local/thread_local_impl_test.cc | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/common/thread_local/thread_local_impl_test.cc b/test/common/thread_local/thread_local_impl_test.cc index 59bdd6d0080b7..24cc993d9d19b 100644 --- a/test/common/thread_local/thread_local_impl_test.cc +++ b/test/common/thread_local/thread_local_impl_test.cc @@ -15,6 +15,19 @@ using testing::ReturnPointee; namespace Envoy { namespace ThreadLocal { +Test(MainThreadVerificationTest, All) { + EXPECT_DEATH(Thread::MainThread::isMainThread(), + "InjectableSingleton used prior to initialization"); + { + EXPECT_DEATH(Thread::MainThread::isMainThread(), + "InjectableSingleton used prior to initialization"); + InstanceImpl tls_; + ASSERT(Thread::MainThread::isMainThread()); + } + EXPECT_DEATH(Thread::MainThread::isMainThread(), + "InjectableSingleton used prior to initialization"); +} + class TestThreadLocalObject : public ThreadLocalObject { public: ~TestThreadLocalObject() override { onDestroy(); } From c05f2cd960669052686483cb264d415d2e3502ed Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Tue, 5 Jan 2021 16:29:05 -0600 Subject: [PATCH 39/47] format Signed-off-by: chaoqin-li1123 --- test/common/thread_local/thread_local_impl_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/thread_local/thread_local_impl_test.cc b/test/common/thread_local/thread_local_impl_test.cc index 24cc993d9d19b..557afdf1c98fc 100644 --- a/test/common/thread_local/thread_local_impl_test.cc +++ b/test/common/thread_local/thread_local_impl_test.cc @@ -21,7 +21,7 @@ Test(MainThreadVerificationTest, All) { { EXPECT_DEATH(Thread::MainThread::isMainThread(), "InjectableSingleton used prior to initialization"); - InstanceImpl tls_; + InstanceImpl tls; ASSERT(Thread::MainThread::isMainThread()); } EXPECT_DEATH(Thread::MainThread::isMainThread(), From f9b3b1b0223834fa3fffd9582fe523523a28c781 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Tue, 5 Jan 2021 16:54:26 -0600 Subject: [PATCH 40/47] fix typo Signed-off-by: chaoqin-li1123 --- test/common/thread_local/thread_local_impl_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/thread_local/thread_local_impl_test.cc b/test/common/thread_local/thread_local_impl_test.cc index 557afdf1c98fc..706111f9f8a39 100644 --- a/test/common/thread_local/thread_local_impl_test.cc +++ b/test/common/thread_local/thread_local_impl_test.cc @@ -15,7 +15,7 @@ using testing::ReturnPointee; namespace Envoy { namespace ThreadLocal { -Test(MainThreadVerificationTest, All) { +TEST(MainThreadVerificationTest, All) { EXPECT_DEATH(Thread::MainThread::isMainThread(), "InjectableSingleton used prior to initialization"); { From 0fa4c909bdad9384f095e5a40f3c435dd84e1dca Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Tue, 5 Jan 2021 17:03:07 -0600 Subject: [PATCH 41/47] fix test Signed-off-by: chaoqin-li1123 --- test/common/thread_local/thread_local_impl_test.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/test/common/thread_local/thread_local_impl_test.cc b/test/common/thread_local/thread_local_impl_test.cc index 706111f9f8a39..98fc77853a246 100644 --- a/test/common/thread_local/thread_local_impl_test.cc +++ b/test/common/thread_local/thread_local_impl_test.cc @@ -23,6 +23,7 @@ TEST(MainThreadVerificationTest, All) { "InjectableSingleton used prior to initialization"); InstanceImpl tls; ASSERT(Thread::MainThread::isMainThread()); + tls_.shutdownThread(); } EXPECT_DEATH(Thread::MainThread::isMainThread(), "InjectableSingleton used prior to initialization"); From 68584ce51019e6bdcd39ff1fc81b544c47df86fc Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Tue, 5 Jan 2021 17:04:11 -0600 Subject: [PATCH 42/47] fix test Signed-off-by: chaoqin-li1123 --- test/common/thread_local/thread_local_impl_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/thread_local/thread_local_impl_test.cc b/test/common/thread_local/thread_local_impl_test.cc index 98fc77853a246..e6554f30f0131 100644 --- a/test/common/thread_local/thread_local_impl_test.cc +++ b/test/common/thread_local/thread_local_impl_test.cc @@ -23,7 +23,7 @@ TEST(MainThreadVerificationTest, All) { "InjectableSingleton used prior to initialization"); InstanceImpl tls; ASSERT(Thread::MainThread::isMainThread()); - tls_.shutdownThread(); + tls.shutdownThread(); } EXPECT_DEATH(Thread::MainThread::isMainThread(), "InjectableSingleton used prior to initialization"); From 6fc21a6c6098fa121e71cde7499d909c45ff81d1 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Tue, 5 Jan 2021 17:06:37 -0600 Subject: [PATCH 43/47] fix test Signed-off-by: chaoqin-li1123 --- test/common/thread_local/thread_local_impl_test.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/test/common/thread_local/thread_local_impl_test.cc b/test/common/thread_local/thread_local_impl_test.cc index e6554f30f0131..ec954aff2f0f2 100644 --- a/test/common/thread_local/thread_local_impl_test.cc +++ b/test/common/thread_local/thread_local_impl_test.cc @@ -23,6 +23,7 @@ TEST(MainThreadVerificationTest, All) { "InjectableSingleton used prior to initialization"); InstanceImpl tls; ASSERT(Thread::MainThread::isMainThread()); + tls.shutdownGlobalThreading(); tls.shutdownThread(); } EXPECT_DEATH(Thread::MainThread::isMainThread(), From 022efb50841c0d090201c6c92f5b4f4407589ee9 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Tue, 5 Jan 2021 20:47:27 -0600 Subject: [PATCH 44/47] add comment Signed-off-by: chaoqin-li1123 --- test/common/thread_local/thread_local_impl_test.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/common/thread_local/thread_local_impl_test.cc b/test/common/thread_local/thread_local_impl_test.cc index ec954aff2f0f2..3e5ab4946395b 100644 --- a/test/common/thread_local/thread_local_impl_test.cc +++ b/test/common/thread_local/thread_local_impl_test.cc @@ -16,16 +16,19 @@ namespace Envoy { namespace ThreadLocal { TEST(MainThreadVerificationTest, All) { + // Main thread utility is initialized in the constructor of tls instance. Call to main thread verification will fail before that. EXPECT_DEATH(Thread::MainThread::isMainThread(), "InjectableSingleton used prior to initialization"); { EXPECT_DEATH(Thread::MainThread::isMainThread(), "InjectableSingleton used prior to initialization"); InstanceImpl tls; + // Call to main thread verification should succeed after tls instance has been intialized. ASSERT(Thread::MainThread::isMainThread()); tls.shutdownGlobalThreading(); tls.shutdownThread(); } + // Main thread utility is cleared in the constructor of tls instance. Call to main thread verification will fail after that. EXPECT_DEATH(Thread::MainThread::isMainThread(), "InjectableSingleton used prior to initialization"); } From dbcd0d65bdb82160b4d4a75a68207e5768396640 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Tue, 5 Jan 2021 20:49:09 -0600 Subject: [PATCH 45/47] add comment Signed-off-by: chaoqin-li1123 --- test/common/thread_local/thread_local_impl_test.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/common/thread_local/thread_local_impl_test.cc b/test/common/thread_local/thread_local_impl_test.cc index 3e5ab4946395b..db6350863592f 100644 --- a/test/common/thread_local/thread_local_impl_test.cc +++ b/test/common/thread_local/thread_local_impl_test.cc @@ -16,7 +16,8 @@ namespace Envoy { namespace ThreadLocal { TEST(MainThreadVerificationTest, All) { - // Main thread utility is initialized in the constructor of tls instance. Call to main thread verification will fail before that. + // Main thread singleton is initialized in the constructor of tls instance. Call to main thread + // verification will fail before that. EXPECT_DEATH(Thread::MainThread::isMainThread(), "InjectableSingleton used prior to initialization"); { @@ -28,7 +29,8 @@ TEST(MainThreadVerificationTest, All) { tls.shutdownGlobalThreading(); tls.shutdownThread(); } - // Main thread utility is cleared in the constructor of tls instance. Call to main thread verification will fail after that. + // Main thread singleton is cleared in the constructor of tls instance. Call to main thread + // verification will fail after that. EXPECT_DEATH(Thread::MainThread::isMainThread(), "InjectableSingleton used prior to initialization"); } From b4918e004bf3bbf729ca82cd7ef81002521d793a Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Tue, 5 Jan 2021 20:50:05 -0600 Subject: [PATCH 46/47] add comment Signed-off-by: chaoqin-li1123 --- test/common/thread_local/thread_local_impl_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/thread_local/thread_local_impl_test.cc b/test/common/thread_local/thread_local_impl_test.cc index db6350863592f..222ae22bbfda5 100644 --- a/test/common/thread_local/thread_local_impl_test.cc +++ b/test/common/thread_local/thread_local_impl_test.cc @@ -29,7 +29,7 @@ TEST(MainThreadVerificationTest, All) { tls.shutdownGlobalThreading(); tls.shutdownThread(); } - // Main thread singleton is cleared in the constructor of tls instance. Call to main thread + // Main thread singleton is cleared in the destructor of tls instance. Call to main thread // verification will fail after that. EXPECT_DEATH(Thread::MainThread::isMainThread(), "InjectableSingleton used prior to initialization"); From 48997e186f0d994a49352abbc694a56586741486 Mon Sep 17 00:00:00 2001 From: chaoqin-li1123 Date: Tue, 5 Jan 2021 21:56:09 -0600 Subject: [PATCH 47/47] fix spelling Signed-off-by: chaoqin-li1123 --- test/common/thread_local/thread_local_impl_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/thread_local/thread_local_impl_test.cc b/test/common/thread_local/thread_local_impl_test.cc index 222ae22bbfda5..de6c67318c6a1 100644 --- a/test/common/thread_local/thread_local_impl_test.cc +++ b/test/common/thread_local/thread_local_impl_test.cc @@ -24,7 +24,7 @@ TEST(MainThreadVerificationTest, All) { EXPECT_DEATH(Thread::MainThread::isMainThread(), "InjectableSingleton used prior to initialization"); InstanceImpl tls; - // Call to main thread verification should succeed after tls instance has been intialized. + // Call to main thread verification should succeed after tls instance has been initialized. ASSERT(Thread::MainThread::isMainThread()); tls.shutdownGlobalThreading(); tls.shutdownThread();