Removed the debug message thread.
authorsgjesse@chromium.org <sgjesse@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Fri, 24 Apr 2009 12:05:40 +0000 (12:05 +0000)
committersgjesse@chromium.org <sgjesse@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Fri, 24 Apr 2009 12:05:40 +0000 (12:05 +0000)
The debug message thread was introduced to make it possible to have the message handler callback be called from a different thread than the thread running V8 where the debug event occoured, but it never had any practical use, and prevents providing information to the message handler which is only available from the V8 thread.

In the future any thread decoupling will have do be done by the embedder.

This also removes the queue used for outbound messages.

Renamed the class Message to CommandMessage as it is only used for debugger commands from the client. Related message queue classes has also been renamed.
Review URL: http://codereview.chromium.org/93118

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

include/v8-debug.h
src/api.cc
src/debug-agent.cc
src/debug.cc
src/debug.h
src/v8.cc
test/cctest/test-debug.cc

index debeac1..5235495 100644 (file)
@@ -138,9 +138,10 @@ class EXPORT Debug {
   // Break execution of JavaScript.
   static void DebugBreak();
 
-  // Message based interface. The message protocol is JSON.
+  // Message based interface. The message protocol is JSON. NOTE the message
+  // handler thread is not supported any more parameter must be false.
   static void SetMessageHandler(MessageHandler handler,
-                                bool message_handler_thread = true);
+                                bool message_handler_thread = false);
   static void SendCommand(const uint16_t* command, int length,
                           ClientData* client_data = NULL);
 
index 5107e0f..02eeb24 100644 (file)
@@ -3264,7 +3264,10 @@ void Debug::SetMessageHandler(v8::Debug::MessageHandler handler,
                               bool message_handler_thread) {
   EnsureInitialized("v8::Debug::SetMessageHandler");
   ENTER_V8;
-  i::Debugger::SetMessageHandler(handler, message_handler_thread);
+  // Message handler thread not supported any more. Parameter temporally left in
+  // the API for client compatability reasons.
+  CHECK(!message_handler_thread);
+  i::Debugger::SetMessageHandler(handler);
 }
 
 
index 9838746..0fab188 100644 (file)
@@ -107,7 +107,7 @@ void DebuggerAgent::CreateSession(Socket* client) {
 
   // Create a new session and hook up the debug message handler.
   session_ = new DebuggerAgentSession(this, client);
-  v8::Debug::SetMessageHandler(DebuggerAgentMessageHandler, this);
+  v8::Debug::SetMessageHandler(DebuggerAgentMessageHandler);
   session_->Start();
 }
 
index 32a96a8..239a373 100644 (file)
@@ -1420,16 +1420,13 @@ Handle<Object> Debugger::event_listener_data_ = Handle<Object>();
 bool Debugger::compiling_natives_ = false;
 bool Debugger::is_loading_debugger_ = false;
 bool Debugger::never_unload_debugger_ = false;
-DebugMessageThread* Debugger::message_thread_ = NULL;
 v8::Debug::MessageHandler Debugger::message_handler_ = NULL;
 bool Debugger::message_handler_cleared_ = false;
 v8::Debug::HostDispatchHandler Debugger::host_dispatch_handler_ = NULL;
 int Debugger::host_dispatch_micros_ = 100 * 1000;
 DebuggerAgent* Debugger::agent_ = NULL;
-LockingMessageQueue Debugger::command_queue_(kQueueInitialSize);
-LockingMessageQueue Debugger::message_queue_(kQueueInitialSize);
+LockingCommandMessageQueue Debugger::command_queue_(kQueueInitialSize);
 Semaphore* Debugger::command_received_ = OS::CreateSemaphore(0);
-Semaphore* Debugger::message_received_ = OS::CreateSemaphore(0);
 
 
 Handle<Object> Debugger::MakeJSObject(Vector<const char> constructor_name,
@@ -1818,7 +1815,7 @@ void Debugger::NotifyMessageHandler(v8::DebugEvent event,
   // Notify the debugger that a debug event has occurred unless auto continue is
   // active in which case no event is send.
   if (!auto_continue) {
-    bool success = SendEventMessage(event_data);
+    bool success = InvokeMessageHandlerWithEvent(event_data);
     if (!success) {
       // If failed to notify debugger just continue running.
       return;
@@ -1845,7 +1842,7 @@ void Debugger::NotifyMessageHandler(v8::DebugEvent event,
     StackGuard::Continue(DEBUGCOMMAND);
 
     // Get the command from the queue.
-    Message command = command_queue_.Get();
+    CommandMessage command = command_queue_.Get();
     Logger::DebugTag("Got request from command queue, in interactive loop.");
     if (!Debugger::IsDebuggerActive()) {
       // Delete command text and user data.
@@ -1900,7 +1897,7 @@ void Debugger::NotifyMessageHandler(v8::DebugEvent event,
     }
 
     // Return the result.
-    SendMessage(Message::NewOutput(response, command.client_data()));
+    InvokeMessageHandler(response, command.client_data());
 
     // Return from debug event processing if either the VM is put into the
     // runnning state (through a continue command) or auto continue is active
@@ -1946,17 +1943,11 @@ void Debugger::SetEventListener(Handle<Object> callback,
 }
 
 
-void Debugger::SetMessageHandler(v8::Debug::MessageHandler handler,
-                                 bool message_handler_thread) {
+void Debugger::SetMessageHandler(v8::Debug::MessageHandler handler) {
   ScopedLock with(debugger_access_);
 
   message_handler_ = handler;
-  if (handler != NULL) {
-    if (!message_thread_ && message_handler_thread) {
-      message_thread_ = new DebugMessageThread();
-      message_thread_->Start();
-    }
-  } else {
+  if (handler == NULL) {
     // Indicate that the message handler was recently cleared.
     message_handler_cleared_ = true;
 
@@ -1980,34 +1971,25 @@ void Debugger::SetHostDispatchHandler(v8::Debug::HostDispatchHandler handler,
 // public API. Messages are kept internally as Vector<uint16_t> strings, which
 // are allocated in various places and deallocated by the calling function
 // sometime after this call.
-void Debugger::InvokeMessageHandler(Message message) {
+void Debugger::InvokeMessageHandler(v8::Handle<v8::String> output,
+                                    v8::Debug::ClientData* data) {
   ScopedLock with(debugger_access_);
 
   if (message_handler_ != NULL) {
-    message_handler_(message.text().start(),
-                     message.text().length(),
-                     message.client_data());
-  }
-  message.Dispose();
-}
+    Vector<uint16_t> text = Vector<uint16_t>::New(output->Length());
+    output->Write(text.start(), 0, output->Length());
 
+    message_handler_(text.start(),
+                     text.length(),
+                     data);
 
-void Debugger::SendMessage(Message message) {
-  if (message_thread_ == NULL) {
-    // If there is no message thread just invoke the message handler from the
-    // V8 thread.
-    InvokeMessageHandler(message);
-  } else {
-    // Put the message coming from V8 on the queue. The text and user data will
-    // be destroyed by the message thread.
-    Logger::DebugTag("Put message on event message_queue.");
-    message_queue_.Put(message);
-    message_received_->Signal();
+    text.Dispose();
   }
+  delete data;
 }
 
 
-bool Debugger::SendEventMessage(Handle<Object> event_data) {
+bool Debugger::InvokeMessageHandlerWithEvent(Handle<Object> event_data) {
   v8::HandleScope scope;
   // Call toJSONProtocol on the debug event object.
   v8::Local<v8::Object> api_event_data =
@@ -2024,11 +2006,10 @@ bool Debugger::SendEventMessage(Handle<Object> event_data) {
       if (FLAG_trace_debug_json) {
         PrintLn(json_event_string);
       }
-      SendMessage(Message::NewOutput(
-          json_event_string,
-          NULL /* no user data since there was no request */));
+      InvokeMessageHandler(json_event_string,
+                           NULL /* no user data since there was no request */);
     } else {
-      SendMessage(Message::NewEmptyMessage());
+      InvokeMessageHandler(v8::String::Empty(), NULL);
     }
   } else {
     PrintLn(try_catch.Exception());
@@ -2041,13 +2022,11 @@ bool Debugger::SendEventMessage(Handle<Object> event_data) {
 // Puts a command coming from the public API on the queue.  Creates
 // a copy of the command string managed by the debugger.  Up to this
 // point, the command data was managed by the API client.  Called
-// by the API client thread.  This is where the API client hands off
-// processing of the command to the DebugMessageThread thread.
-// The new copy of the command is destroyed in HandleCommand().
+// by the API client thread.
 void Debugger::ProcessCommand(Vector<const uint16_t> command,
                               v8::Debug::ClientData* client_data) {
   // Need to cast away const.
-  Message message = Message::NewCommand(
+  CommandMessage message = CommandMessage::New(
       Vector<uint16_t>(const_cast<uint16_t*>(command.start()),
                        command.length()),
       client_data);
@@ -2122,99 +2101,51 @@ void Debugger::StopAgent() {
 }
 
 
-void Debugger::TearDown() {
-  if (message_thread_ != NULL) {
-    message_thread_->Stop();
-    delete message_thread_;
-    message_thread_ = NULL;
-  }
-}
-
-
-void DebugMessageThread::Run() {
-  // Sends debug events to an installed debugger message callback.
-  while (keep_running_) {
-    // Wait and Get are paired so that semaphore count equals queue length.
-    Debugger::message_received_->Wait();
-    Logger::DebugTag("Get message from event message_queue.");
-    Message message = Debugger::message_queue_.Get();
-    if (message.text().length() > 0) {
-      Debugger::InvokeMessageHandler(message);
-    } else {
-      message.Dispose();
-    }
-  }
+CommandMessage::CommandMessage() : text_(Vector<uint16_t>::empty()),
+                                   client_data_(NULL) {
 }
 
 
-void DebugMessageThread::Stop() {
-  keep_running_ = false;
-  Debugger::SendMessage(Message::NewEmptyMessage());
-  Join();
-}
-
-
-Message::Message() : text_(Vector<uint16_t>::empty()),
-                     client_data_(NULL) {
-}
-
-
-Message::Message(const Vector<uint16_t>& text,
-                 v8::Debug::ClientData* data)
+CommandMessage::CommandMessage(const Vector<uint16_t>& text,
+                               v8::Debug::ClientData* data)
     : text_(text),
       client_data_(data) {
 }
 
 
-Message::~Message() {
+CommandMessage::~CommandMessage() {
 }
 
 
-void Message::Dispose() {
+void CommandMessage::Dispose() {
   text_.Dispose();
   delete client_data_;
   client_data_ = NULL;
 }
 
 
-Message Message::NewCommand(const Vector<uint16_t>& command,
-                            v8::Debug::ClientData* data) {
-  return Message(command.Clone(), data);
-}
-
-
-Message Message::NewOutput(v8::Handle<v8::String> output,
-                           v8::Debug::ClientData* data) {
-  Vector<uint16_t> text;
-  if (!output.IsEmpty()) {
-    // Do not include trailing '\0'.
-    text = Vector<uint16_t>::New(output->Length());
-    output->Write(text.start(), 0, output->Length());
-  }
-  return Message(text, data);
-}
-
-
-Message Message::NewEmptyMessage() {
-  return Message();
+CommandMessage CommandMessage::New(const Vector<uint16_t>& command,
+                                   v8::Debug::ClientData* data) {
+  return CommandMessage(command.Clone(), data);
 }
 
 
-MessageQueue::MessageQueue(int size) : start_(0), end_(0), size_(size) {
-  messages_ = NewArray<Message>(size);
+CommandMessageQueue::CommandMessageQueue(int size) : start_(0), end_(0),
+                                                     size_(size) {
+  messages_ = NewArray<CommandMessage>(size);
 }
 
 
-MessageQueue::~MessageQueue() {
+CommandMessageQueue::~CommandMessageQueue() {
   while (!IsEmpty()) {
-    Message m = Get();
+    CommandMessage m = Get();
     m.Dispose();
   }
   DeleteArray(messages_);
 }
 
 
-Message MessageQueue::Get() {
+CommandMessage CommandMessageQueue::Get() {
   ASSERT(!IsEmpty());
   int result = start_;
   start_ = (start_ + 1) % size_;
@@ -2222,7 +2153,7 @@ Message MessageQueue::Get() {
 }
 
 
-void MessageQueue::Put(const Message& message) {
+void CommandMessageQueue::Put(const CommandMessage& message) {
   if ((end_ + 1) % size_ == start_) {
     Expand();
   }
@@ -2231,12 +2162,12 @@ void MessageQueue::Put(const Message& message) {
 }
 
 
-void MessageQueue::Expand() {
-  MessageQueue new_queue(size_ * 2);
+void CommandMessageQueue::Expand() {
+  CommandMessageQueue new_queue(size_ * 2);
   while (!IsEmpty()) {
     new_queue.Put(Get());
   }
-  Message* array_to_free = messages_;
+  CommandMessage* array_to_free = messages_;
   *this = new_queue;
   new_queue.messages_ = array_to_free;
   // Make the new_queue empty so that it doesn't call Dispose on any messages.
@@ -2245,38 +2176,39 @@ void MessageQueue::Expand() {
 }
 
 
-LockingMessageQueue::LockingMessageQueue(int size) : queue_(size) {
+LockingCommandMessageQueue::LockingCommandMessageQueue(int size)
+    : queue_(size) {
   lock_ = OS::CreateMutex();
 }
 
 
-LockingMessageQueue::~LockingMessageQueue() {
+LockingCommandMessageQueue::~LockingCommandMessageQueue() {
   delete lock_;
 }
 
 
-bool LockingMessageQueue::IsEmpty() const {
+bool LockingCommandMessageQueue::IsEmpty() const {
   ScopedLock sl(lock_);
   return queue_.IsEmpty();
 }
 
 
-Message LockingMessageQueue::Get() {
+CommandMessage LockingCommandMessageQueue::Get() {
   ScopedLock sl(lock_);
-  Message result = queue_.Get();
+  CommandMessage result = queue_.Get();
   Logger::DebugEvent("Get", result.text());
   return result;
 }
 
 
-void LockingMessageQueue::Put(const Message& message) {
+void LockingCommandMessageQueue::Put(const CommandMessage& message) {
   ScopedLock sl(lock_);
   queue_.Put(message);
   Logger::DebugEvent("Put", message.text());
 }
 
 
-void LockingMessageQueue::Clear() {
+void LockingCommandMessageQueue::Clear() {
   ScopedLock sl(lock_);
   queue_.Clear();
 }
index ffd3da9..8a50cd2 100644 (file)
@@ -405,70 +405,65 @@ class Debug {
 // In addition to command text it may contain a pointer to some user data
 // which are expected to be passed along with the command reponse to message
 // handler.
-class Message {
+class CommandMessage {
  public:
-  static Message NewCommand(const Vector<uint16_t>& command,
+  static CommandMessage New(const Vector<uint16_t>& command,
                             v8::Debug::ClientData* data);
-  static Message NewOutput(v8::Handle<v8::String> output,
-                           v8::Debug::ClientData* data);
-  static Message NewEmptyMessage();
-  Message();
-  ~Message();
+  CommandMessage();
+  ~CommandMessage();
 
   // Deletes user data and disposes of the text.
   void Dispose();
   Vector<uint16_t> text() const { return text_; }
   v8::Debug::ClientData* client_data() const { return client_data_; }
  private:
-  Message(const Vector<uint16_t>& text,
-          v8::Debug::ClientData* data);
+  CommandMessage(const Vector<uint16_t>& text,
+                 v8::Debug::ClientData* data);
 
   Vector<uint16_t> text_;
   v8::Debug::ClientData* client_data_;
 };
 
-// A Queue of Vector<uint16_t> objects.  A thread-safe version is
-// LockingMessageQueue, based on this class.
-class MessageQueue BASE_EMBEDDED {
+// A Queue of CommandMessage objects.  A thread-safe version is
+// LockingCommandMessageQueue, based on this class.
+class CommandMessageQueue BASE_EMBEDDED {
  public:
-  explicit MessageQueue(int size);
-  ~MessageQueue();
+  explicit CommandMessageQueue(int size);
+  ~CommandMessageQueue();
   bool IsEmpty() const { return start_ == end_; }
-  Message Get();
-  void Put(const Message& message);
+  CommandMessage Get();
+  void Put(const CommandMessage& message);
   void Clear() { start_ = end_ = 0; }  // Queue is empty after Clear().
  private:
   // Doubles the size of the message queue, and copies the messages.
   void Expand();
 
-  Message* messages_;
+  CommandMessage* messages_;
   int start_;
   int end_;
   int size_;  // The size of the queue buffer.  Queue can hold size-1 messages.
 };
 
 
-// LockingMessageQueue is a thread-safe circular buffer of Vector<uint16_t>
-// messages.  The message data is not managed by LockingMessageQueue.
+// LockingCommandMessageQueue is a thread-safe circular buffer of CommandMessage
+// messages.  The message data is not managed by LockingCommandMessageQueue.
 // Pointers to the data are passed in and out. Implemented by adding a
-// Mutex to MessageQueue.  Includes logging of all puts and gets.
-class LockingMessageQueue BASE_EMBEDDED {
+// Mutex to CommandMessageQueue.  Includes logging of all puts and gets.
+class LockingCommandMessageQueue BASE_EMBEDDED {
  public:
-  explicit LockingMessageQueue(int size);
-  ~LockingMessageQueue();
+  explicit LockingCommandMessageQueue(int size);
+  ~LockingCommandMessageQueue();
   bool IsEmpty() const;
-  Message Get();
-  void Put(const Message& message);
+  CommandMessage Get();
+  void Put(const CommandMessage& message);
   void Clear();
  private:
-  MessageQueue queue_;
+  CommandMessageQueue queue_;
   Mutex* lock_;
-  DISALLOW_COPY_AND_ASSIGN(LockingMessageQueue);
+  DISALLOW_COPY_AND_ASSIGN(LockingCommandMessageQueue);
 };
 
 
-class DebugMessageThread;
-
 class Debugger {
  public:
   static void DebugRequest(const uint16_t* json_request, int length);
@@ -503,21 +498,16 @@ class Debugger {
                                    Handle<Object> event_data,
                                    bool auto_continue);
   static void SetEventListener(Handle<Object> callback, Handle<Object> data);
-  static void SetMessageHandler(v8::Debug::MessageHandler handler,
-                                bool message_handler_thread);
-  static void TearDown();
+  static void SetMessageHandler(v8::Debug::MessageHandler handler);
   static void SetHostDispatchHandler(v8::Debug::HostDispatchHandler handler,
                                      int period);
 
   // Invoke the message handler function.
-  static void InvokeMessageHandler(Message message);
-
-  // Send a message to the message handler eiher through the message thread or
-  // directly.
-  static void SendMessage(Message message);
+  static void InvokeMessageHandler(v8::Handle<v8::String> output,
+                                   v8::Debug::ClientData* data);
 
   // Send the JSON message for a debug event.
-  static bool SendEventMessage(Handle<Object> event_data);
+  static bool InvokeMessageHandlerWithEvent(Handle<Object> event_data);
 
   // Add a debugger command to the command queue.
   static void ProcessCommand(Vector<const uint16_t> command,
@@ -568,7 +558,6 @@ class Debugger {
   static bool compiling_natives_;  // Are we compiling natives?
   static bool is_loading_debugger_;  // Are we loading the debugger?
   static bool never_unload_debugger_;  // Can we unload the debugger?
-  static DebugMessageThread* message_thread_;
   static v8::Debug::MessageHandler message_handler_;
   static bool message_handler_cleared_;  // Was message handler cleared?
   static v8::Debug::HostDispatchHandler host_dispatch_handler_;
@@ -577,32 +566,10 @@ class Debugger {
   static DebuggerAgent* agent_;
 
   static const int kQueueInitialSize = 4;
-  static LockingMessageQueue command_queue_;
-  static LockingMessageQueue message_queue_;
+  static LockingCommandMessageQueue command_queue_;
   static Semaphore* command_received_;  // Signaled for each command received.
-  static Semaphore* message_received_;  // Signalled for each message send.
 
   friend class EnterDebugger;
-  friend class DebugMessageThread;
-};
-
-
-// Thread to read messages from the message queue and invoke the debug message
-// handler in another thread as the V8 thread. This thread is started if the
-// registration of the debug message handler requested to be called in a thread
-// seperate from the V8 thread.
-class DebugMessageThread: public Thread {
- public:
-  DebugMessageThread() : keep_running_(true) {}
-  virtual ~DebugMessageThread() {}
-
-  // Main function of DebugMessageThread thread.
-  void Run();
-  void Stop();
-
- private:
-  bool keep_running_;
-  DISALLOW_COPY_AND_ASSIGN(DebugMessageThread);
 };
 
 
index fbe3d5d..c0124e4 100644 (file)
--- a/src/v8.cc
+++ b/src/v8.cc
@@ -113,10 +113,6 @@ void V8::TearDown() {
   Heap::TearDown();
   Logger::TearDown();
 
-#ifdef ENABLE_DEBUGGER_SUPPORT
-  Debugger::TearDown();
-#endif
-
   has_been_setup_ = false;
   has_been_disposed_ = true;
 }
index fea07c5..5fa15ff 100644 (file)
@@ -45,8 +45,8 @@ using ::v8::internal::JSGlobalProxy;
 using ::v8::internal::Code;
 using ::v8::internal::Debug;
 using ::v8::internal::Debugger;
-using ::v8::internal::Message;
-using ::v8::internal::MessageQueue;
+using ::v8::internal::CommandMessage;
+using ::v8::internal::CommandMessageQueue;
 using ::v8::internal::StepAction;
 using ::v8::internal::StepIn;  // From StepAction enum
 using ::v8::internal::StepNext;  // From StepAction enum
@@ -3559,25 +3559,26 @@ int TestClientData::destructor_call_counter = 0;
 TEST(MessageQueueExpandAndDestroy) {
   TestClientData::ResetCounters();
   { // Create a scope for the queue.
-    MessageQueue queue(1);
-    queue.Put(Message::NewCommand(Vector<uint16_t>::empty(),
+    CommandMessageQueue queue(1);
+    queue.Put(CommandMessage::New(Vector<uint16_t>::empty(),
                                   new TestClientData()));
-    queue.Put(Message::NewCommand(Vector<uint16_t>::empty(),
+    queue.Put(CommandMessage::New(Vector<uint16_t>::empty(),
                                   new TestClientData()));
-    queue.Put(Message::NewCommand(Vector<uint16_t>::empty(),
+    queue.Put(CommandMessage::New(Vector<uint16_t>::empty(),
                                   new TestClientData()));
     ASSERT_EQ(0, TestClientData::destructor_call_counter);
     queue.Get().Dispose();
     ASSERT_EQ(1, TestClientData::destructor_call_counter);
-    queue.Put(Message::NewCommand(Vector<uint16_t>::empty(),
+    queue.Put(CommandMessage::New(Vector<uint16_t>::empty(),
                                   new TestClientData()));
-    queue.Put(Message::NewCommand(Vector<uint16_t>::empty(),
+    queue.Put(CommandMessage::New(Vector<uint16_t>::empty(),
                                   new TestClientData()));
-    queue.Put(Message::NewCommand(Vector<uint16_t>::empty(),
+    queue.Put(CommandMessage::New(Vector<uint16_t>::empty(),
+                                  new TestClientData()));
+    queue.Put(CommandMessage::New(Vector<uint16_t>::empty(),
+                                  new TestClientData()));
+    queue.Put(CommandMessage::New(Vector<uint16_t>::empty(),
                                   new TestClientData()));
-    queue.Put(Message::NewOutput(v8::Handle<v8::String>(),
-                                 new TestClientData()));
-    queue.Put(Message::NewEmptyMessage());
     ASSERT_EQ(1, TestClientData::destructor_call_counter);
     queue.Get().Dispose();
     ASSERT_EQ(2, TestClientData::destructor_call_counter);
@@ -3606,8 +3607,7 @@ TEST(SendClientDataToHandler) {
   DebugLocalContext env;
   TestClientData::ResetCounters();
   handled_client_data_instances_count = 0;
-  v8::Debug::SetMessageHandler(MessageHandlerCountingClientData,
-                               false /* message_handler_thread */);
+  v8::Debug::SetMessageHandler(MessageHandlerCountingClientData);
   const char* source_1 = "a = 3; b = 4; c = new Object(); c.d = 5; debugger;";
   const int kBufferSize = 1000;
   uint16_t buffer[kBufferSize];