[Lldb-commits] [lldb] r149355 - in /lldb/trunk: include/lldb/Core/ConnectionFileDescriptor.h source/Core/Communication.cpp source/Core/ConnectionFileDescriptor.cpp source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp tools/driver/Driver.cpp

Greg Clayton gclayton at apple.com
Mon Jan 30 20:56:17 PST 2012


Author: gclayton
Date: Mon Jan 30 22:56:17 2012
New Revision: 149355

URL: http://llvm.org/viewvc/llvm-project?rev=149355&view=rev
Log:
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.


Modified:
    lldb/trunk/include/lldb/Core/ConnectionFileDescriptor.h
    lldb/trunk/source/Core/Communication.cpp
    lldb/trunk/source/Core/ConnectionFileDescriptor.cpp
    lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
    lldb/trunk/tools/driver/Driver.cpp

Modified: lldb/trunk/include/lldb/Core/ConnectionFileDescriptor.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ConnectionFileDescriptor.h?rev=149355&r1=149354&r2=149355&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Core/ConnectionFileDescriptor.h (original)
+++ lldb/trunk/include/lldb/Core/ConnectionFileDescriptor.h Mon Jan 30 22:56:17 2012
@@ -105,7 +105,7 @@
     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);

Modified: lldb/trunk/source/Core/Communication.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Communication.cpp?rev=149355&r1=149354&r2=149355&view=diff
==============================================================================
--- lldb/trunk/source/Core/Communication.cpp (original)
+++ lldb/trunk/source/Core/Communication.cpp Mon Jan 30 22:56:17 2012
@@ -65,8 +65,8 @@
 Communication::Clear()
 {
     SetReadThreadBytesReceivedCallback (NULL, NULL);
-    StopReadThread (NULL);
     Disconnect (NULL);
+    StopReadThread (NULL);
 }
 
 ConnectionStatus
@@ -253,7 +253,7 @@
 
     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 @@
     // 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 @@
 void
 Communication::SetConnection (Connection *connection)
 {
-    StopReadThread(NULL);
     Disconnect (NULL);
+    StopReadThread(NULL);
     m_connection_sp.reset(connection);
 }
 

Modified: lldb/trunk/source/Core/ConnectionFileDescriptor.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ConnectionFileDescriptor.cpp?rev=149355&r1=149354&r2=149355&view=diff
==============================================================================
--- lldb/trunk/source/Core/ConnectionFileDescriptor.cpp (original)
+++ lldb/trunk/source/Core/ConnectionFileDescriptor.cpp Mon Jan 30 22:56:17 2012
@@ -73,8 +73,8 @@
     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 @@
     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 @@
 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 @@
 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 @@
         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 @@
 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)
-        {
-            // 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();
+        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)
+        {
+            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

Modified: lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp?rev=149355&r1=149354&r2=149355&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp (original)
+++ lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp Mon Jan 30 22:56:17 2012
@@ -469,8 +469,7 @@
     // 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 @@
         }
     }
     StopAsyncThread ();
-    m_comm.StopReadThread();
-    m_comm.Disconnect();    // Disconnect from the debug server.
+    m_comm.Clear();
     return error;
 }
 

Modified: lldb/trunk/tools/driver/Driver.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/driver/Driver.cpp?rev=149355&r1=149354&r2=149355&view=diff
==============================================================================
--- lldb/trunk/tools/driver/Driver.cpp (original)
+++ lldb/trunk/tools/driver/Driver.cpp Mon Jan 30 22:56:17 2012
@@ -1347,7 +1347,13 @@
                         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);





More information about the lldb-commits mailing list