diff --git a/source/exe/admin_response.cc b/source/exe/admin_response.cc index 42ddb0f40be5..c5745217e7e4 100644 --- a/source/exe/admin_response.cc +++ b/source/exe/admin_response.cc @@ -8,16 +8,15 @@ namespace Envoy { AdminResponse::AdminResponse(Server::Instance& server, absl::string_view path, - absl::string_view method, - TerminateNotifierSharedPtr terminate_notifier) - : server_(server), opt_admin_(server.admin()), terminate_notifier_(terminate_notifier) { + absl::string_view method, SharedPtrSet response_set) + : server_(server), opt_admin_(server.admin()), shared_response_set_(response_set) { request_headers_->setMethod(method); request_headers_->setPath(path); } AdminResponse::~AdminResponse() { cancel(); - terminate_notifier_->detachResponse(this); + shared_response_set_->detachResponse(this); } void AdminResponse::getHeaders(HeadersFn fn) { @@ -161,7 +160,7 @@ void AdminResponse::sendErrorLockHeld() ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_) { } } -void TerminateNotifier::terminateAdminRequests() { +void AdminResponse::PtrSet::terminateAdminRequests() { ASSERT_IS_MAIN_OR_TEST_THREAD(); absl::MutexLock lock(&mutex_); @@ -176,7 +175,7 @@ void TerminateNotifier::terminateAdminRequests() { response_set_.clear(); } -void TerminateNotifier::attachResponse(AdminResponse* response) { +void AdminResponse::PtrSet::attachResponse(AdminResponse* response) { absl::MutexLock lock(&mutex_); if (accepting_admin_requests_) { response_set_.insert(response); @@ -185,7 +184,7 @@ void TerminateNotifier::attachResponse(AdminResponse* response) { } } -void TerminateNotifier::detachResponse(AdminResponse* response) { +void AdminResponse::PtrSet::detachResponse(AdminResponse* response) { absl::MutexLock lock(&mutex_); response_set_.erase(response); } diff --git a/source/exe/admin_response.h b/source/exe/admin_response.h index 3c8dbc811106..931a1ed9292f 100644 --- a/source/exe/admin_response.h +++ b/source/exe/admin_response.h @@ -14,28 +14,6 @@ namespace Envoy { class AdminResponse; -// AdminResponse can outlive MainCommonBase. But AdminResponse needs a -// reliable way of knowing whether MainCommonBase is alive, so we do this with -// TerminateNotifier, which is held by MainCommonBase and all the active -// AdminResponses via shared_ptr. This gives MainCommonBase a reliable way of -// notifying all active responses that it is being shut down, and thus all -// responses need to be terminated. And it gives a reliable way for -// AdminResponse to detach itself, even if MainCommonBase is already deleted. -class TerminateNotifier { -public: - void detachResponse(AdminResponse*); - void attachResponse(AdminResponse*); - - // Called after the server run-loop finishes; any outstanding streaming admin requests - // will otherwise hang as the main-thread dispatcher loop will no longer run. - void terminateAdminRequests(); - - mutable absl::Mutex mutex_; - absl::flat_hash_set response_set_ ABSL_GUARDED_BY(mutex_); - bool accepting_admin_requests_ ABSL_GUARDED_BY(mutex_) = true; -}; -using TerminateNotifierSharedPtr = std::shared_ptr; - // Holds context for a streaming response from the admin system, enabling // flow-control into another system. This is particularly important when the // generated response is very large, such that holding it in memory may cause @@ -55,8 +33,31 @@ using TerminateNotifierSharedPtr = std::shared_ptr; // cancel() is called, no further callbacks will be called by the response. class AdminResponse : public std::enable_shared_from_this { public: + // AdminResponse can outlive MainCommonBase. But AdminResponse needs a + // reliable way of knowing whether MainCommonBase is alive, so we do this with + // PtrSet, which is held by MainCommonBase and all the active AdminResponse, + // which is held by MainCommonBase and by AdminResponse via shared_ptr. This + // gives MainCommonBase a reliable way of notifying all active responses that + // it is being shut down, and thus all responses need to be terminated. And it + // gives a reliable way for AdminResponse to detach itself, even if + // MainCommonBase is already deleted. + class PtrSet { + public: + void detachResponse(AdminResponse*); + void attachResponse(AdminResponse*); + + // Called after the server run-loop finishes; any outstanding streaming admin requests + // will otherwise hang as the main-thread dispatcher loop will no longer run. + void terminateAdminRequests(); + + mutable absl::Mutex mutex_; + absl::flat_hash_set response_set_ ABSL_GUARDED_BY(mutex_); + bool accepting_admin_requests_ ABSL_GUARDED_BY(mutex_) = true; + }; + using SharedPtrSet = std::shared_ptr; + AdminResponse(Server::Instance& server, absl::string_view path, absl::string_view method, - TerminateNotifierSharedPtr terminate_notifier); + SharedPtrSet response_set); ~AdminResponse(); /** @@ -112,9 +113,6 @@ class AdminResponse : public std::enable_shared_from_this { bool cancelled() const; private: - friend class MainCommonBase; - friend class TerminateNotifier; - /** * Called when the server is terminated. This calls any outstanding * callbacks to be called. If nextChunk is called after termination, @@ -159,7 +157,7 @@ class AdminResponse : public std::enable_shared_from_this { BodyFn body_fn_ ABSL_GUARDED_BY(mutex_); mutable absl::Mutex mutex_; - TerminateNotifierSharedPtr terminate_notifier_; + SharedPtrSet shared_response_set_; }; using AdminResponseSharedPtr = std::shared_ptr; diff --git a/source/exe/main_common.cc b/source/exe/main_common.cc index bdf26c324f58..87b3342f3335 100644 --- a/source/exe/main_common.cc +++ b/source/exe/main_common.cc @@ -60,7 +60,7 @@ MainCommonBase::MainCommonBase(const Server::Options& options, Event::TimeSystem std::move(process_context), createFunction()) #ifdef ENVOY_ADMIN_FUNCTIONALITY , - terminate_notifier_(std::make_shared()) + shared_response_set_(std::make_shared()) #endif { } @@ -74,7 +74,7 @@ bool MainCommonBase::run() { case Server::Mode::Serve: runServer(); #ifdef ENVOY_ADMIN_FUNCTIONALITY - terminate_notifier_->terminateAdminRequests(); + shared_response_set_->terminateAdminRequests(); #endif ret = true; break; @@ -114,8 +114,8 @@ void MainCommonBase::adminRequest(absl::string_view path_and_query, absl::string AdminResponseSharedPtr MainCommonBase::adminRequest(absl::string_view path_and_query, absl::string_view method) { auto response = - std::make_shared(*server(), path_and_query, method, terminate_notifier_); - terminate_notifier_->attachResponse(response.get()); + std::make_shared(*server(), path_and_query, method, shared_response_set_); + shared_response_set_->attachResponse(response.get()); return response; } #endif diff --git a/source/exe/main_common.h b/source/exe/main_common.h index ade0915aaada..c70897e22bbd 100644 --- a/source/exe/main_common.h +++ b/source/exe/main_common.h @@ -60,7 +60,7 @@ class MainCommonBase : public StrippedMainBase { void detachResponse(AdminResponse*); private: - TerminateNotifierSharedPtr terminate_notifier_; + AdminResponse::SharedPtrSet shared_response_set_; #endif };