Cleaned up the Communication class when it tears down ConnectionFileDescriptor

instances to not pthread_cancel the read threads and wreak havoc on the mutex
in our ConnectionFileDescriptor class.

Also cleaned up some shutdown delays.

llvm-svn: 149355

5 files changed, 54 insertions, 39 deletions

diff --git a/lldb/include/lldb/Core/ConnectionFileDescriptor.h b/lldb/include/lldb/Core/ConnectionFileDescriptor.h
index b52a682474d..b2a29d6c6e2 100644
--- a/lldb/include/lldb/Core/ConnectionFileDescriptor.h
+++ b/lldb/include/lldb/Core/ConnectionFileDescriptor.h
@@ -105,7 +105,7 @@ protected:
     SocketAddress m_udp_send_sockaddr;
     bool m_should_close_fd; // True if this class should close the file descriptor when it goes away.
     uint32_t m_socket_timeout_usec;
-    //Mutex m_mutex;
+    Mutex m_mutex;
     
     static in_port_t
     GetSocketPort (int fd);
diff --git a/lldb/source/Core/Communication.cpp b/lldb/source/Core/Communication.cpp
index a9ca35c9db8..45850f3858b 100644
--- a/lldb/source/Core/Communication.cpp
+++ b/lldb/source/Core/Communication.cpp
@@ -65,8 +65,8 @@ void
 Communication::Clear()
 {
     SetReadThreadBytesReceivedCallback (NULL, NULL);
-    StopReadThread (NULL);
     Disconnect (NULL);
+    StopReadThread (NULL);
 }
 
 ConnectionStatus
@@ -253,7 +253,7 @@ Communication::StopReadThread (Error *error_ptr)
 
     BroadcastEvent (eBroadcastBitReadThreadShouldExit, NULL);
 
-    Host::ThreadCancel (m_read_thread, error_ptr);
+    //Host::ThreadCancel (m_read_thread, error_ptr);
 
     bool status = Host::ThreadJoin (m_read_thread, NULL, error_ptr);
     m_read_thread = LLDB_INVALID_HOST_THREAD;
@@ -383,7 +383,7 @@ Communication::ReadThread (void *p)
     // Let clients know that this thread is exiting
     comm->BroadcastEvent (eBroadcastBitReadThreadDidExit);
     comm->m_read_thread_enabled = false;
-    comm->Disconnect();
+    comm->m_read_thread = LLDB_INVALID_HOST_THREAD;
     return NULL;
 }
 
@@ -401,8 +401,8 @@ Communication::SetReadThreadBytesReceivedCallback
 void
 Communication::SetConnection (Connection *connection)
 {
-    StopReadThread(NULL);
     Disconnect (NULL);
+    StopReadThread(NULL);
     m_connection_sp.reset(connection);
 }
 
diff --git a/lldb/source/Core/ConnectionFileDescriptor.cpp b/lldb/source/Core/ConnectionFileDescriptor.cpp
index 28f72d57792..538f84be797 100644
--- a/lldb/source/Core/ConnectionFileDescriptor.cpp
+++ b/lldb/source/Core/ConnectionFileDescriptor.cpp
@@ -73,8 +73,8 @@ ConnectionFileDescriptor::ConnectionFileDescriptor () :
     m_fd_recv_type (eFDTypeFile),
     m_udp_send_sockaddr (),
     m_should_close_fd (false), 
-    m_socket_timeout_usec(0)//,
-    //m_mutex (Mutex::eMutexTypeRecursive)
+    m_socket_timeout_usec(0),
+    m_mutex (Mutex::eMutexTypeRecursive)
 {
     LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_CONNECTION |  LIBLLDB_LOG_OBJECT));
     if (log)
@@ -89,7 +89,8 @@ ConnectionFileDescriptor::ConnectionFileDescriptor (int fd, bool owns_fd) :
     m_fd_recv_type (eFDTypeFile),
     m_udp_send_sockaddr (),
     m_should_close_fd (owns_fd),
-    m_socket_timeout_usec(0)
+    m_socket_timeout_usec(0),
+    m_mutex (Mutex::eMutexTypeRecursive)
 {
     LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_CONNECTION |  LIBLLDB_LOG_OBJECT));
     if (log)
@@ -114,7 +115,7 @@ ConnectionFileDescriptor::IsConnected () const
 ConnectionStatus
 ConnectionFileDescriptor::Connect (const char *s, Error *error_ptr)
 {
-    //Mutex::Locker locker (m_mutex);
+    Mutex::Locker locker (m_mutex);
     LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_CONNECTION));
     if (log)
         log->Printf ("%p ConnectionFileDescriptor::Connect (url = '%s')", this, s);
@@ -234,6 +235,7 @@ ConnectionFileDescriptor::Connect (const char *s, Error *error_ptr)
 ConnectionStatus
 ConnectionFileDescriptor::Disconnect (Error *error_ptr)
 {
+    Mutex::Locker locker (m_mutex);
     LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_CONNECTION));
     if (log)
         log->Printf ("%p ConnectionFileDescriptor::Disconnect ()", this);
@@ -243,22 +245,25 @@ ConnectionFileDescriptor::Disconnect (Error *error_ptr)
         return eConnectionStatusSuccess;
     }
     ConnectionStatus status = eConnectionStatusSuccess;
-    if (m_fd_send == m_fd_recv)
+    if (m_fd_send >= 0 || m_fd_recv >= 0)
     {
-        // Both file descriptors are the same, only close one
-        status = Close (m_fd_send, error_ptr);
-        m_fd_recv = -1;
-    }
-    else
-    {
-        // File descriptors are the different, close both if needed
-        if (m_fd_send >= 0)
+        if (m_fd_send == m_fd_recv)
+        {
+            // Both file descriptors are the same, only close one
+            m_fd_recv = -1;
             status = Close (m_fd_send, error_ptr);
-        if (m_fd_recv >= 0)
+        }
+        else
         {
-            ConnectionStatus recv_status = Close (m_fd_recv, error_ptr);
-            if (status == eConnectionStatusSuccess)
-                status = recv_status;
+            // File descriptors are the different, close both if needed
+            if (m_fd_send >= 0)
+                status = Close (m_fd_send, error_ptr);
+            if (m_fd_recv >= 0)
+            {
+                ConnectionStatus recv_status = Close (m_fd_recv, error_ptr);
+                if (status == eConnectionStatusSuccess)
+                    status = recv_status;
+            }
         }
     }
     return status;
@@ -601,27 +606,33 @@ ConnectionFileDescriptor::BytesAvailable (uint32_t timeout_usec, Error *error_pt
 ConnectionStatus
 ConnectionFileDescriptor::Close (int& fd, Error *error_ptr)
 {
-    //Mutex::Locker locker (m_mutex);
     if (error_ptr)
         error_ptr->Clear();
     bool success = true;
+    // Avoid taking a lock if we can
     if (fd >= 0)
     {
-        LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_CONNECTION));
-        if (log)
-            log->Printf ("%p ConnectionFileDescriptor::Close (fd = %i)", this,fd);
-
-        success = ::close (fd) == 0;
-        if (!success && error_ptr)
+        Mutex::Locker locker (m_mutex);
+        // Check the FD after the lock is taken to ensure only one thread
+        // can get into the close scope below
+        if (fd >= 0)
         {
-            // Only set the error if we have been asked to since something else
-            // might have caused us to try and shut down the connection and may
-            // have already set the error.
-            error_ptr->SetErrorToErrno();
+            LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_CONNECTION));
+            if (log)
+                log->Printf ("%p ConnectionFileDescriptor::Close (fd = %i)", this,fd);
+
+            success = ::close (fd) == 0;
+            // A reference to a FD was passed in, set it to an invalid value
+            fd = -1;
+            if (!success && error_ptr)
+            {
+                // Only set the error if we have been asked to since something else
+                // might have caused us to try and shut down the connection and may
+                // have already set the error.
+                error_ptr->SetErrorToErrno();
+            }
         }
-        fd = -1;
     }
-    m_fd_send_type = m_fd_recv_type = eFDTypeFile;
     if (success)
         return eConnectionStatusSuccess;
     else
diff --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
index fed40e0ff1b..8f327d6220d 100644
--- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
+++ b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
@@ -469,8 +469,7 @@ ProcessKDP::DoDetach()
     // Sleep for one second to let the process get all detached...
     StopAsyncThread ();
     
-    m_comm.StopReadThread();
-    m_comm.Disconnect();    // Disconnect from the debug server.
+    m_comm.Clear();
     
     SetPrivateState (eStateDetached);
     ResumePrivateStateThread();
@@ -508,8 +507,7 @@ ProcessKDP::DoDestroy ()
         }
     }
     StopAsyncThread ();
-    m_comm.StopReadThread();
-    m_comm.Disconnect();    // Disconnect from the debug server.
+    m_comm.Clear();
     return error;
 }
 
diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index 5e47c9896c2..8466847b9e1 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -1347,7 +1347,13 @@ Driver::MainLoop ()
                         else if (event.BroadcasterMatchesRef (sb_interpreter.GetBroadcaster()))
                         {
                             if (event_type & SBCommandInterpreter::eBroadcastBitQuitCommandReceived)
+                            {
+                                editline_output_pty.CloseMasterFileDescriptor();
+                                master_out_comm.Disconnect();
+                                out_comm_2.Disconnect();
+                                fclose (stdin);
                                 done = true;
+                            }
                             else if (event_type & SBCommandInterpreter::eBroadcastBitAsynchronousErrorData)
                             {
                                 const char *data = SBEvent::GetCStringFromEvent (event);