Fix for issue 10488 standalone/basic_debugger_test flaky

runtime/bin/dbg_connection.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "bin/dbg_connection.h"
 #include "bin/dbg_message.h"
 #include "bin/dartutils.h"
 #include "bin/log.h"
 #include "bin/socket.h"
 #include "bin/thread.h"
 #include "bin/utils.h"
 
 #include "platform/globals.h"
 #include "platform/json.h"
 #include "platform/thread.h"
 #include "platform/utils.h"
 
 #include "include/dart_api.h"
 
 
 namespace dart {
 namespace bin {
 
 int DebuggerConnectionHandler::listener_fd_ = -1;
-dart::Monitor DebuggerConnectionHandler::handler_lock_;
+dart::Monitor* DebuggerConnectionHandler::handler_lock_ = NULL;
 
 // TODO(asiva): Remove this once we have support for multiple debugger
 // connections. For now we just store the single debugger connection
 // handler in a static variable.
 static DebuggerConnectionHandler* singleton_handler = NULL;
 
 
 class MessageBuffer {
  public:
   explicit MessageBuffer(int fd);
@@ skipping 237 matching lines @@
     delete msgbuf_;
     msgbuf_ = NULL;
   }
   // TODO(hausner): Need to tell the VM debugger object to remove all
   // breakpoints.
 }
 
 
 void DebuggerConnectionHandler::StartHandler(const char* address,
                                              int port_number) {
-  MonitorLocker ml(&handler_lock_);
+  handler_lock_ = new dart::Monitor();

[hausner, 2013/05/08 15:26:48]

What if two threads call this function by accident? Then there will be two
locks, and one thread may have lock 1 while the second one grabs lock 2.

Not entirely safe, but maybe good enough: ASSERT(handler_lock_ == NULL) before
allocating the lock.

[siva, 2013/05/08 20:48:12]

I changed the initialization of handler_lock_ into a static initialization, that
should protect against multiple invocations of StartHandler.

On 2013/05/08 15:26:48, hausner wrote:

+  ASSERT(handler_lock_ != NULL);
+
+  MonitorLocker ml(handler_lock_);
   if (listener_fd_ != -1) {
     return;  // The debugger connection handler was already started.
   }
 
   // First setup breakpoint, exception and delayed breakpoint handlers.
   DbgMsgQueueList::Initialize();
 
   // Initialize the socket implementation.
   if (!Socket::Initialize()) {
     FATAL("Failed initializing socket implementation.");
   }
 
   // Now setup a listener socket and start a thread which will
   // listen, accept connections from debuggers, read and handle/dispatch
   // debugger commands received on these connections.
   ASSERT(listener_fd_ == -1);
 
   OSError *os_error;
   SocketAddresses* addresses = Socket::LookupAddress(address, -1, &os_error);
   listener_fd_ = ServerSocket::CreateBindListen(
       addresses->GetAt(0)->addr(), port_number, 1);
   DebuggerConnectionImpl::StartHandler(port_number);
 }
 
 
 void DebuggerConnectionHandler::WaitForConnection() {
-  MonitorLocker ml(&handler_lock_);
+  ASSERT(handler_lock_ != NULL);
+  MonitorLocker ml(handler_lock_);
   while (!IsConnected()) {
     dart::Monitor::WaitResult res = ml.Wait();
     ASSERT(res == dart::Monitor::kNotified);
   }
 }
 
 
 void DebuggerConnectionHandler::SendMsg(int debug_fd, dart::TextBuffer* msg) {
-  MonitorLocker ml(&handler_lock_);
+  ASSERT(handler_lock_ != NULL);
+  MonitorLocker ml(handler_lock_);
   SendMsgHelper(debug_fd, msg);
 }
 
 
 void DebuggerConnectionHandler::BroadcastMsg(dart::TextBuffer* msg) {
-  MonitorLocker ml(&handler_lock_);
+  ASSERT(handler_lock_ != NULL);
+  MonitorLocker ml(handler_lock_);
   // TODO(asiva): Once we support connection to multiple debuggers
   // we need to send the message to all of them.
   ASSERT(singleton_handler != NULL);
   SendMsgHelper(singleton_handler->debug_fd(), msg);
 }
 
 
 void DebuggerConnectionHandler::SendMsgHelper(int debug_fd,
                                               dart::TextBuffer* msg) {
   ASSERT(debug_fd >= 0);
@@ skipping 28 matching lines @@
       DebuggerConnectionImpl::Send(debug_fd, msg->buf(), msg->length());
   ASSERT(msg->length() == bytes_written);
   // TODO(hausner): Error checking. Probably just shut down the debugger
   // session if we there is an error while writing.
 }
 
 
 void DebuggerConnectionHandler::AcceptDbgConnection(int debug_fd) {
   AddNewDebuggerConnection(debug_fd);
   {
-    MonitorLocker ml(&handler_lock_);
+    ASSERT(handler_lock_ != NULL);
+    MonitorLocker ml(handler_lock_);
     ml.NotifyAll();
   }
   // TODO(asiva): Once we implement support for multiple connections
   // we should have a different callback for wakeups on fds which
   // are not the listener_fd_.
   // In that callback we would lookup the handler object
   // corresponding to that fd and invoke HandleMessages on it.
   // For now we run that code here.
   DebuggerConnectionHandler* handler = GetDebuggerConnectionHandler(debug_fd);
   if (handler != NULL) {
@@ skipping 60 matching lines @@
 
 
 bool DebuggerConnectionHandler::IsConnected() {
   // TODO(asiva): Support multiple debugger connections.
   // Return true if a connection has been established.
   return singleton_handler != NULL;
 }
 
 }  // namespace bin
 }  // namespace dart