remove no-longer-needed 'friend' declarations and rename TerminateNot…

…ifier per discussion

Signed-off-by: Joshua Marantz <[email protected]>

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);
 }

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<AdminResponse*> response_set_ ABSL_GUARDED_BY(mutex_);
-  bool accepting_admin_requests_ ABSL_GUARDED_BY(mutex_) = true;
-};
-using TerminateNotifierSharedPtr = std::shared_ptr<TerminateNotifier>;
-
 // 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<TerminateNotifier>;
 // cancel() is called, no further callbacks will be called by the response.
 class AdminResponse : public std::enable_shared_from_this<AdminResponse> {
 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<AdminResponse*> response_set_ ABSL_GUARDED_BY(mutex_);
+    bool accepting_admin_requests_ ABSL_GUARDED_BY(mutex_) = true;
+  };
+  using SharedPtrSet = std::shared_ptr<PtrSet>;
+
   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<AdminResponse> {
   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<AdminResponse> {
   BodyFn body_fn_ ABSL_GUARDED_BY(mutex_);
   mutable absl::Mutex mutex_;
 
-  TerminateNotifierSharedPtr terminate_notifier_;
+  SharedPtrSet shared_response_set_;
 };
 using AdminResponseSharedPtr = std::shared_ptr<AdminResponse>;
 

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<TerminateNotifier>())
+      shared_response_set_(std::make_shared<AdminResponse::PtrSet>())
 #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<AdminResponse>(*server(), path_and_query, method, terminate_notifier_);
-  terminate_notifier_->attachResponse(response.get());
+      std::make_shared<AdminResponse>(*server(), path_and_query, method, shared_response_set_);
+  shared_response_set_->attachResponse(response.get());
   return response;
 }
 #endif

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
 };