inspector: code cleanup

Remove some unused code from the WS server implementation and switch to
smart pointers where possible.

PR-URL: https://github.com/nodejs/node/pull/21070
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

1 files changed, 34 insertions, 112 deletions

diff --git a/src/inspector_socket_server.cc b/src/inspector_socket_server.cc
index 42082c9c849..d5eb1b75c81 100644
--- a/src/inspector_socket_server.cc
+++ b/src/inspector_socket_server.cc
@@ -156,43 +156,6 @@ std::string FormatWsAddress(const std::string& host, int port,
   return FormatAddress(FormatHostPort(host, port), target_id, include_protocol);
 }
 
-class Closer {
- public:
-  explicit Closer(InspectorSocketServer* server) : server_(server),
-                                                   close_count_(0) { }
-
-  void AddCallback(InspectorSocketServer::ServerCallback callback) {
-    if (callback == nullptr)
-      return;
-    callbacks_.insert(callback);
-  }
-
-  void DecreaseExpectedCount() {
-    --close_count_;
-    NotifyIfDone();
-  }
-
-  void IncreaseExpectedCount() {
-    ++close_count_;
-  }
-
-  void NotifyIfDone() {
-    if (close_count_ == 0) {
-      for (auto callback : callbacks_) {
-        callback(server_);
-      }
-      InspectorSocketServer* server = server_;
-      delete server->closer_;
-      server->closer_ = nullptr;
-    }
-  }
-
- private:
-  InspectorSocketServer* server_;
-  std::set<InspectorSocketServer::ServerCallback> callbacks_;
-  int close_count_;
-};
-
 class SocketSession {
  public:
   SocketSession(InspectorSocketServer* server, int id, int server_port);
@@ -250,45 +213,38 @@ class SocketSession {
 
 class ServerSocket {
  public:
-  static int Listen(InspectorSocketServer* inspector_server,
-                    sockaddr* addr, uv_loop_t* loop);
+  explicit ServerSocket(InspectorSocketServer* server)
+                        : tcp_socket_(uv_tcp_t()), server_(server) {}
+  int Listen(sockaddr* addr, uv_loop_t* loop);
   void Close() {
-    uv_close(reinterpret_cast<uv_handle_t*>(&tcp_socket_),
-             SocketClosedCallback);
+    uv_close(reinterpret_cast<uv_handle_t*>(&tcp_socket_), FreeOnCloseCallback);
   }
   int port() const { return port_; }
 
  private:
-  explicit ServerSocket(InspectorSocketServer* server)
-      : tcp_socket_(uv_tcp_t()), server_(server), port_(-1) {}
   template <typename UvHandle>
   static ServerSocket* FromTcpSocket(UvHandle* socket) {
     return node::ContainerOf(&ServerSocket::tcp_socket_,
                              reinterpret_cast<uv_tcp_t*>(socket));
   }
   static void SocketConnectedCallback(uv_stream_t* tcp_socket, int status);
-  static void SocketClosedCallback(uv_handle_t* tcp_socket);
   static void FreeOnCloseCallback(uv_handle_t* tcp_socket_) {
     delete FromTcpSocket(tcp_socket_);
   }
   int DetectPort();
+  ~ServerSocket() = default;
 
   uv_tcp_t tcp_socket_;
   InspectorSocketServer* server_;
-  int port_;
+  int port_ = -1;
 };
 
-InspectorSocketServer::InspectorSocketServer(SocketServerDelegate* delegate,
-                                             uv_loop_t* loop,
-                                             const std::string& host,
-                                             int port,
-                                             FILE* out) : loop_(loop),
-                                                          delegate_(delegate),
-                                                          host_(host),
-                                                          port_(port),
-                                                          closer_(nullptr),
-                                                          next_session_id_(0),
-                                                          out_(out) {
+InspectorSocketServer::InspectorSocketServer(
+    std::unique_ptr<SocketServerDelegate> delegate, uv_loop_t* loop,
+    const std::string& host, int port, FILE* out)
+    : loop_(loop), delegate_(std::move(delegate)), host_(host), port_(port),
+      next_session_id_(0), out_(out) {
+  delegate_->AssignServer(this);
   state_ = ServerState::kNew;
 }
 
@@ -328,7 +284,7 @@ void InspectorSocketServer::SessionTerminated(int session_id) {
                                 delegate_->GetTargetIds(), out_);
     }
     if (state_ == ServerState::kStopped) {
-      delegate_->ServerDone();
+      delegate_.reset();
     }
   }
 }
@@ -389,7 +345,11 @@ void InspectorSocketServer::SendListResponse(InspectorSocket* socket,
 }
 
 bool InspectorSocketServer::Start() {
+  CHECK_NE(delegate_, nullptr);
   CHECK_EQ(state_, ServerState::kNew);
+  std::unique_ptr<SocketServerDelegate> delegate_holder;
+  // We will return it if startup is successful
+  delegate_.swap(delegate_holder);
   struct addrinfo hints;
   memset(&hints, 0, sizeof(hints));
   hints.ai_flags = AI_NUMERICSERV;
@@ -407,13 +367,13 @@ bool InspectorSocketServer::Start() {
   }
   for (addrinfo* address = req.addrinfo; address != nullptr;
        address = address->ai_next) {
-    err = ServerSocket::Listen(this, address->ai_addr, loop_);
+    auto server_socket = ServerSocketPtr(new ServerSocket(this));
+    err = server_socket->Listen(address->ai_addr, loop_);
+    if (err == 0)
+      server_sockets_.push_back(std::move(server_socket));
   }
   uv_freeaddrinfo(req.addrinfo);
 
-  if (!connected_sessions_.empty()) {
-    return true;
-  }
   // We only show error if we failed to start server on all addresses. We only
   // show one error, for the last address.
   if (server_sockets_.empty()) {
@@ -424,6 +384,7 @@ bool InspectorSocketServer::Start() {
     }
     return false;
   }
+  delegate_.swap(delegate_holder);
   state_ = ServerState::kRunning;
   // getaddrinfo sorts the addresses, so the first port is most relevant.
   PrintDebuggerReadyMessage(host_, server_sockets_[0]->port(),
@@ -431,17 +392,12 @@ bool InspectorSocketServer::Start() {
   return true;
 }
 
-void InspectorSocketServer::Stop(ServerCallback cb) {
+void InspectorSocketServer::Stop() {
   CHECK_EQ(state_, ServerState::kRunning);
-  if (closer_ == nullptr) {
-    closer_ = new Closer(this);
-  }
-  closer_->AddCallback(cb);
-  closer_->IncreaseExpectedCount();
-  state_ = ServerState::kStopping;
-  for (ServerSocket* server_socket : server_sockets_)
-    server_socket->Close();
-  closer_->NotifyIfDone();
+  state_ = ServerState::kStopped;
+  server_sockets_.clear();
+  if (done())
+    delegate_.reset();
 }
 
 void InspectorSocketServer::TerminateConnections() {
@@ -455,28 +411,6 @@ bool InspectorSocketServer::TargetExists(const std::string& id) {
   return found != target_ids.end();
 }
 
-void InspectorSocketServer::ServerSocketListening(ServerSocket* server_socket) {
-  server_sockets_.push_back(server_socket);
-}
-
-void InspectorSocketServer::ServerSocketClosed(ServerSocket* server_socket) {
-  CHECK_EQ(state_, ServerState::kStopping);
-
-  server_sockets_.erase(std::remove(server_sockets_.begin(),
-                                    server_sockets_.end(), server_socket),
-                        server_sockets_.end());
-  if (!server_sockets_.empty())
-    return;
-
-  if (closer_ != nullptr) {
-    closer_->DecreaseExpectedCount();
-  }
-  if (connected_sessions_.empty()) {
-    delegate_->ServerDone();
-  }
-  state_ = ServerState::kStopped;
-}
-
 int InspectorSocketServer::Port() const {
   if (!server_sockets_.empty()) {
     return server_sockets_[0]->port();
@@ -525,6 +459,10 @@ void InspectorSocketServer::Send(int session_id, const std::string& message) {
   }
 }
 
+void InspectorSocketServer::CloseServerSocket(ServerSocket* server) {
+  server->Close();
+}
+
 // InspectorSession tracking
 SocketSession::SocketSession(InspectorSocketServer* server, int id,
                              int server_port)
@@ -569,11 +507,8 @@ int ServerSocket::DetectPort() {
   return err;
 }
 
-// static
-int ServerSocket::Listen(InspectorSocketServer* inspector_server,
-                         sockaddr* addr, uv_loop_t* loop) {
-  ServerSocket* server_socket = new ServerSocket(inspector_server);
-  uv_tcp_t* server = &server_socket->tcp_socket_;
+int ServerSocket::Listen(sockaddr* addr, uv_loop_t* loop) {
+  uv_tcp_t* server = &tcp_socket_;
   CHECK_EQ(0, uv_tcp_init(loop, server));
   int err = uv_tcp_bind(server, addr, 0);
   if (err == 0) {
@@ -582,12 +517,7 @@ int ServerSocket::Listen(InspectorSocketServer* inspector_server,
                     ServerSocket::SocketConnectedCallback);
   }
   if (err == 0) {
-    err = server_socket->DetectPort();
-  }
-  if (err == 0) {
-    inspector_server->ServerSocketListening(server_socket);
-  } else {
-    uv_close(reinterpret_cast<uv_handle_t*>(server), FreeOnCloseCallback);
+    err = DetectPort();
   }
   return err;
 }
@@ -601,13 +531,5 @@ void ServerSocket::SocketConnectedCallback(uv_stream_t* tcp_socket,
     server_socket->server_->Accept(server_socket->port_, tcp_socket);
   }
 }
-
-// static
-void ServerSocket::SocketClosedCallback(uv_handle_t* tcp_socket) {
-  ServerSocket* server_socket = ServerSocket::FromTcpSocket(tcp_socket);
-  server_socket->server_->ServerSocketClosed(server_socket);
-  delete server_socket;
-}
-
 }  // namespace inspector
 }  // namespace node