Better handling of startup and shutdown of the debugger agent.

During bind and listen socket errors are now handled. If the listen socket is occoupied the agent will retry its bind operation until success or shutdown.

Added orderly shutdown of the debugger agent both with and without a client connected.
Review URL: http://codereview.chromium.org/50007

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@1552 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/debug-agent.cc

@@ -42,25 +42,49 @@ void DebuggerAgentMessageHandler(const uint16_t* message, int length,
 
 // Debugger agent main thread.
 void DebuggerAgent::Run() {
-  // Create a server socket and bind it to the requested port.
-  server_ = OS::CreateSocket();
-  server_->Bind(port_);
+  const int kOneSecondInMicros = 1000000;
+
+  // First bind the socket to the requested port.
+  bool bound = false;
+  while (!bound && !terminate_) {
+    bound = server_->Bind(port_);
+
+    // If an error occoured wait a bit before retrying. The most common error
+    // would be that the port is already in use so this avoids a busy loop and
+    // make the agent take over the port when it becomes free.
+    if (!bound) {
+      terminate_now_->Wait(kOneSecondInMicros);
+    }
+  }
 
+  // Accept connections on the bound port.
   while (!terminate_) {
-    // Listen for new connections.
-    server_->Listen(1);
-
-    // Accept the new connection.
-    Socket* client = server_->Accept();
-
-    // Create and start a new session.
-    CreateSession(client);
+    bool ok = server_->Listen(1);
+    if (ok) {
+      // Accept the new connection.
+      Socket* client = server_->Accept();
+      ok = client != NULL;
+      if (ok) {
+        // Create and start a new session.
+        CreateSession(client);
+      }
+    }
   }
 }
 
 
 void DebuggerAgent::Shutdown() {
-  delete server_;
+  // Set the termination flag.
+  terminate_ = true;
+
+  // Signal termination and make the server exit either its listen call or its
+  // binding loop. This makes sure that no new sessions can be established.
+  terminate_now_->Signal();
+  server_->Shutdown();
+  Join();
+
+  // Close existing session if any.
+  CloseSession();
 }
 
 
@@ -83,6 +107,19 @@ void DebuggerAgent::CreateSession(Socket* client) {
 }
 
 
+void DebuggerAgent::CloseSession() {
+  ScopedLock with(session_access_);
+
+  // Terminate the session.
+  if (session_ != NULL) {
+    session_->Shutdown();
+    session_->Join();
+    delete session_;
+    session_ = NULL;
+  }
+}
+
+
 void DebuggerAgent::DebuggerMessage(const uint16_t* message, int length) {
   ScopedLock with(session_access_);
 
@@ -94,15 +131,17 @@ void DebuggerAgent::DebuggerMessage(const uint16_t* message, int length) {
 }
 
 
-void DebuggerAgent::SessionClosed(DebuggerAgentSession* session) {
-  ScopedLock with(session_access_);
+void DebuggerAgent::OnSessionClosed(DebuggerAgentSession* session) {
+  // Don't do anything during termination.
+  if (terminate_) {
+    return;
+  }
 
   // Terminate the session.
+  ScopedLock with(session_access_);
   ASSERT(session == session_);
   if (session == session_) {
-    session->Join();
-    delete session;
-    session_ = NULL;
+    CloseSession();
   }
 }
 
@@ -113,7 +152,7 @@ void DebuggerAgentSession::Run() {
     SmartPointer<char> message = DebuggerAgentUtil::ReceiveMessage(client_);
     if (*message == NULL) {
       // Session is closed.
-      agent_->SessionClosed(this);
+      agent_->OnSessionClosed(this);
       return;
     }
 
@@ -142,6 +181,12 @@ void DebuggerAgentSession::DebuggerMessage(Vector<uint16_t> message) {
 }
 
 
+void DebuggerAgentSession::Shutdown() {
+  // Shutdown the socket to end the blocking receive.
+  client_->Shutdown();
+}
+
+
 const char* DebuggerAgentUtil::kContentLength = "Content-Length";
 int DebuggerAgentUtil::kContentLengthSize = strlen(kContentLength);
 

src/debug-agent.h

@@ -44,8 +44,9 @@ class DebuggerAgent: public Thread {
  public:
   explicit DebuggerAgent(int port)
       : port_(port), server_(OS::CreateSocket()), terminate_(false),
-        session_access_(OS::CreateMutex()), session_(NULL) {}
-  ~DebuggerAgent() {}
+        session_access_(OS::CreateMutex()), session_(NULL),
+        terminate_now_(OS::CreateSemaphore(0)) {}
+  ~DebuggerAgent() { delete server_; }
 
   void Shutdown();
 
@@ -53,13 +54,15 @@ class DebuggerAgent: public Thread {
   void Run();
   void CreateSession(Socket* socket);
   void DebuggerMessage(const uint16_t* message, int length);
-  void SessionClosed(DebuggerAgentSession* session);
+  void CloseSession();
+  void OnSessionClosed(DebuggerAgentSession* session);
 
   int port_;  // Port to use for the agent.
   Socket* server_;  // Server socket for listen/accept.
   bool terminate_;  // Termination flag.
   Mutex* session_access_;  // Mutex guarging access to session_.
   DebuggerAgentSession* session_;  // Current active session if any.
+  Semaphore* terminate_now_;  // Semaphore to signal termination.
 
   friend class DebuggerAgentSession;
   friend void DebuggerAgentMessageHandler(const uint16_t* message, int length,
@@ -77,6 +80,7 @@ class DebuggerAgentSession: public Thread {
       : agent_(agent), client_(client) {}
 
   void DebuggerMessage(Vector<uint16_t> message);
+  void Shutdown();
 
  private:
   void Run();
@@ -84,7 +88,7 @@ class DebuggerAgentSession: public Thread {
   void DebuggerMessage(Vector<char> message);
 
   DebuggerAgent* agent_;
-  const Socket* client_;
+  Socket* client_;
 
   DISALLOW_COPY_AND_ASSIGN(DebuggerAgentSession);
 };

src/debug.cc

@@ -1839,6 +1839,16 @@ bool Debugger::StartAgent(int port) {
 }
 
 
+void Debugger::StopAgent() {
+  if (agent_ != NULL) {
+    agent_->Shutdown();
+    agent_->Join();
+    delete agent_;
+    agent_ = NULL;
+  }
+}
+
+
 DebugMessageThread::DebugMessageThread()
     : host_running_(true),
       command_queue_(kQueueInitialSize),

src/debug.h

@@ -443,6 +443,9 @@ class Debugger {
   // Start the debugger agent listening on the provided port.
   static bool StartAgent(int port);
 
+  // Stop the debugger agent.
+  static void StopAgent();
+
   inline static bool EventActive(v8::DebugEvent event) {
     // Currently argument event is not used.
     return !Debugger::compiling_natives_ && Debugger::debugger_active_;

test/cctest/test-debug.cc

@@ -3822,3 +3822,44 @@ TEST(DebuggerHostDispatch) {
   // The host dispatch callback should be called.
   CHECK_EQ(1, host_dispatch_hit_count);
 }
+
+
+TEST(DebuggerAgent) {
+  // Make sure this port is not used by other tests to allow tests to run in
+  // parallel.
+  const int kPort = 5858;
+
+  // Make a string with the port number.
+  const int kPortBuferLen = 6;
+  char port_str[kPortBuferLen];
+  OS::SNPrintF(i::Vector<char>(port_str, kPortBuferLen), "%d", kPort);
+
+  bool ok;
+
+  // Initialize the socket library.
+  i::Socket::Setup();
+
+  // Test starting and stopping the agent without any client connection.
+  i::Debugger::StartAgent(kPort);
+  i::Debugger::StopAgent();
+
+  // Test starting the agent, connecting a client and shutting down the agent
+  // with the client connected.
+  ok = i::Debugger::StartAgent(kPort);
+  CHECK(ok);
+  i::Socket* client = i::OS::CreateSocket();
+  ok = client->Connect("localhost", port_str);
+  CHECK(ok);
+  i::Debugger::StopAgent();
+  delete client;
+
+  // Test starting and stopping the agent with the required port already
+  // occoupied.
+  i::Socket* server = i::OS::CreateSocket();
+  server->Bind(kPort);
+
+  i::Debugger::StartAgent(kPort);
+  i::Debugger::StopAgent();
+
+  delete server;
+}

test/cctest/test-sockets.cc

@@ -8,13 +8,10 @@
 using namespace ::v8::internal;
 
 
-static const char* kPort = "5858";
-static const char* kLocalhost = "localhost";
-
 class SocketListenerThread : public Thread {
  public:
-  explicit SocketListenerThread(int data_size)
-      : data_size_(data_size), server_(NULL), client_(NULL),
+  explicit SocketListenerThread(int port, int data_size)
+      : port_(port), data_size_(data_size), server_(NULL), client_(NULL),
         listening_(OS::CreateSemaphore(0)) {
     data_ = new char[data_size_];
   }
@@ -31,6 +28,7 @@ class SocketListenerThread : public Thread {
   char* data() { return data_; }
 
  private:
+  int port_;
   char* data_;
   int data_size_;
   Socket* server_;  // Server socket used for bind/accept.
@@ -45,7 +43,7 @@ void SocketListenerThread::Run() {
   // Create the server socket and bind it to the requested port.
   server_ = OS::CreateSocket();
   CHECK(server_ != NULL);
-  ok = server_->Bind(5858);
+  ok = server_->Bind(port_);
   CHECK(ok);
 
   // Listen for new connections.
@@ -78,18 +76,25 @@ static bool SendAll(Socket* socket, const char* data, int len) {
 }
 
 
-static void SendAndReceive(char *data, int len) {
+static void SendAndReceive(int port, char *data, int len) {
+  static const char* kLocalhost = "localhost";
+
   bool ok;
 
+  // Make a string with the port number.
+  const int kPortBuferLen = 6;
+  char port_str[kPortBuferLen];
+  OS::SNPrintF(Vector<char>(port_str, kPortBuferLen), "%d", port);
+
   // Create a socket listener.
-  SocketListenerThread* listener = new SocketListenerThread(len);
+  SocketListenerThread* listener = new SocketListenerThread(port, len);
   listener->Start();
   listener->WaitForListening();
 
   // Connect and write some data.
   Socket* client = OS::CreateSocket();
   CHECK(client != NULL);
-  ok = client->Connect(kLocalhost, kPort);
+  ok = client->Connect(kLocalhost, port_str);
   CHECK(ok);
 
   // Send all the data.
@@ -112,6 +117,10 @@ static void SendAndReceive(char *data, int len) {
 
 
 TEST(Socket) {
+  // Make sure this port is not used by other tests to allow tests to run in
+  // parallel.
+  static const int kPort = 5859;
+
   bool ok;
 
   // Initialize socket support.
@@ -121,7 +130,7 @@ TEST(Socket) {
   // Send and receive some data.
   static const int kBufferSizeSmall = 20;
   char small_data[kBufferSizeSmall + 1] = "1234567890abcdefghij";
-  SendAndReceive(small_data, kBufferSizeSmall);
+  SendAndReceive(kPort, small_data, kBufferSizeSmall);
 
   // Send and receive some more data.
   static const int kBufferSizeMedium = 10000;
@@ -129,7 +138,7 @@ TEST(Socket) {
   for (int i = 0; i < kBufferSizeMedium; i++) {
     medium_data[i] = i % 256;
   }
-  SendAndReceive(medium_data, kBufferSizeMedium);
+  SendAndReceive(kPort, medium_data, kBufferSizeMedium);
   delete[] medium_data;
 
   // Send and receive even more data.
@@ -138,7 +147,7 @@ TEST(Socket) {
   for (int i = 0; i < kBufferSizeLarge; i++) {
     large_data[i] = i % 256;
   }
-  SendAndReceive(large_data, kBufferSizeLarge);
+  SendAndReceive(kPort, large_data, kBufferSizeLarge);
   delete[] large_data;
 }