[Lldb-commits] [lldb] r154365 - in /lldb/trunk: include/lldb/Target/ source/Plugins/OperatingSystem/Darwin-Kernel/ source/Plugins/Process/MacOSX-Kernel/ source/Plugins/Process/gdb-remote/ source/Plugins/Process/mach-core/ source/Target/

Greg Clayton gclayton at apple.com
Mon Apr 9 17:19:00 PDT 2012


Author: gclayton
Date: Mon Apr  9 19:18:59 2012
New Revision: 154365

URL: http://llvm.org/viewvc/llvm-project?rev=154365&view=rev
Log:
Trying to solve our disappearing thread issues by making thread list updates safer.

The current ProcessGDBRemote function that updates the threads could end up with an empty list if any other thread had the sequence mutex. We now don't clear the thread list when we can't access it, and we also have changed how lldb_private::Process handles the return code from the:

virtual bool
Process::UpdateThreadList (lldb_private::ThreadList &old_thread_list, 
                       	   lldb_private::ThreadList &new_thread_list) = 0;

A bool is now returned to indicate if the list was actually updated or not and the lldb_private::Process class will only update the stop ID of the validity of the thread list if "true" is returned.

The ProcessGDBRemote also got an extra assertion that will hopefully assert when running debug builds so we can find the source of this issue.


Modified:
    lldb/trunk/include/lldb/Target/OperatingSystem.h
    lldb/trunk/include/lldb/Target/Process.h
    lldb/trunk/source/Plugins/OperatingSystem/Darwin-Kernel/OperatingSystemDarwinKernel.cpp
    lldb/trunk/source/Plugins/OperatingSystem/Darwin-Kernel/OperatingSystemDarwinKernel.h
    lldb/trunk/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp
    lldb/trunk/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.h
    lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
    lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
    lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
    lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
    lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
    lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
    lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
    lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
    lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.cpp
    lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.h
    lldb/trunk/source/Target/Process.cpp

Modified: lldb/trunk/include/lldb/Target/OperatingSystem.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/OperatingSystem.h?rev=154365&r1=154364&r2=154365&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/OperatingSystem.h (original)
+++ lldb/trunk/include/lldb/Target/OperatingSystem.h Mon Apr  9 19:18:59 2012
@@ -64,7 +64,7 @@
     //------------------------------------------------------------------
     // Plug-in Methods
     //------------------------------------------------------------------
-    virtual uint32_t
+    virtual bool
     UpdateThreadList (ThreadList &old_thread_list, ThreadList &new_thread_list) = 0;
     
     virtual void

Modified: lldb/trunk/include/lldb/Target/Process.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=154365&r1=154364&r2=154365&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Process.h (original)
+++ lldb/trunk/include/lldb/Target/Process.h Mon Apr  9 19:18:59 2012
@@ -2937,7 +2937,7 @@
     //------------------------------------------------------------------
     // Thread Queries
     //------------------------------------------------------------------
-    virtual uint32_t
+    virtual bool
     UpdateThreadList (ThreadList &old_thread_list, ThreadList &new_thread_list) = 0;
 
     void

Modified: lldb/trunk/source/Plugins/OperatingSystem/Darwin-Kernel/OperatingSystemDarwinKernel.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/OperatingSystem/Darwin-Kernel/OperatingSystemDarwinKernel.cpp?rev=154365&r1=154364&r2=154365&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/OperatingSystem/Darwin-Kernel/OperatingSystemDarwinKernel.cpp (original)
+++ lldb/trunk/source/Plugins/OperatingSystem/Darwin-Kernel/OperatingSystemDarwinKernel.cpp Mon Apr  9 19:18:59 2012
@@ -223,7 +223,7 @@
     return 1;
 }
 
-uint32_t
+bool
 OperatingSystemDarwinKernel::UpdateThreadList (ThreadList &old_thread_list, ThreadList &new_thread_list)
 {
     // Make any constant strings once and cache the uniqued C string values
@@ -263,7 +263,7 @@
         }
         next_valobj_sp.swap(valobj_sp);
     }
-    return new_thread_list.GetSize(false);
+    return new_thread_list.GetSize(false) > 0;
 }
 
 void

Modified: lldb/trunk/source/Plugins/OperatingSystem/Darwin-Kernel/OperatingSystemDarwinKernel.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/OperatingSystem/Darwin-Kernel/OperatingSystemDarwinKernel.h?rev=154365&r1=154364&r2=154365&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/OperatingSystem/Darwin-Kernel/OperatingSystemDarwinKernel.h (original)
+++ lldb/trunk/source/Plugins/OperatingSystem/Darwin-Kernel/OperatingSystemDarwinKernel.h Mon Apr  9 19:18:59 2012
@@ -61,7 +61,7 @@
     //------------------------------------------------------------------
     // lldb_private::OperatingSystem Methods
     //------------------------------------------------------------------
-    virtual uint32_t
+    virtual bool
     UpdateThreadList (lldb_private::ThreadList &old_thread_list, 
                       lldb_private::ThreadList &new_thread_list);
     

Modified: lldb/trunk/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp?rev=154365&r1=154364&r2=154365&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp (original)
+++ lldb/trunk/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.cpp Mon Apr  9 19:18:59 2012
@@ -151,7 +151,7 @@
 }
 
 bool
-CommunicationKDP::GetSequenceMutex (Mutex::Locker& locker)
+CommunicationKDP::TryLockSequenceMutex (Mutex::Locker& locker)
 {
     return locker.TryLock (m_sequence_mutex.GetMutex());
 }

Modified: lldb/trunk/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.h?rev=154365&r1=154364&r2=154365&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.h (original)
+++ lldb/trunk/source/Plugins/Process/MacOSX-Kernel/CommunicationKDP.h Mon Apr  9 19:18:59 2012
@@ -102,7 +102,7 @@
                                           uint32_t usec);
 
     bool
-    GetSequenceMutex(lldb_private::Mutex::Locker& locker);
+    TryLockSequenceMutex(lldb_private::Mutex::Locker& locker);
 
     bool
     CheckForPacket (const uint8_t *src, 

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=154365&r1=154364&r2=154365&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp (original)
+++ lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp Mon Apr  9 19:18:59 2012
@@ -301,7 +301,7 @@
     return error;
 }
 
-uint32_t
+bool
 ProcessKDP::UpdateThreadList (ThreadList &old_thread_list, ThreadList &new_thread_list)
 {
     // locker will keep a mutex locked until it goes out of scope
@@ -323,7 +323,7 @@
             thread_sp.reset(new ThreadKDP (shared_from_this(), tid));
         new_thread_list.AddThread(thread_sp);
     }
-    return new_thread_list.GetSize(false);
+    return new_thread_list.GetSize(false) > 0;
 }
 
 

Modified: lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h?rev=154365&r1=154364&r2=154365&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h (original)
+++ lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h Mon Apr  9 19:18:59 2012
@@ -231,7 +231,7 @@
     void
     Clear ( );
     
-    uint32_t
+    virtual bool
     UpdateThreadList (lldb_private::ThreadList &old_thread_list, 
                       lldb_private::ThreadList &new_thread_list);
     

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp?rev=154365&r1=154364&r2=154365&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp Mon Apr  9 19:18:59 2012
@@ -201,10 +201,7 @@
             // logs all of the packet will set a boolean so that we don't dump this more
             // than once
             if (!m_history.DidDumpToLog ())
-            {
-                DumpHistory("/tmp/foo.txt");
                 m_history.Dump (log.get());
-            }
 
             log->Printf ("<%4zu> send packet: %.*s", bytes_written, (int)packet.GetSize(), packet.GetData());
         }
@@ -243,7 +240,7 @@
 }
 
 bool
-GDBRemoteCommunication::GetSequenceMutex (Mutex::Locker& locker)
+GDBRemoteCommunication::TryLockSequenceMutex (Mutex::Locker& locker)
 {
     return locker.TryLock (m_sequence_mutex.GetMutex());
 }

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h?rev=154365&r1=154364&r2=154365&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h Mon Apr  9 19:18:59 2012
@@ -74,7 +74,7 @@
                         size_t payload_length);
 
     bool
-    GetSequenceMutex(lldb_private::Mutex::Locker& locker);
+    TryLockSequenceMutex(lldb_private::Mutex::Locker& locker);
 
     bool
     CheckForPacket (const uint8_t *src, 

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp?rev=154365&r1=154364&r2=154365&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp Mon Apr  9 19:18:59 2012
@@ -240,7 +240,7 @@
     Mutex::Locker locker;
     LogSP log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_PROCESS));
     size_t response_len = 0;
-    if (GetSequenceMutex (locker))
+    if (TryLockSequenceMutex (locker))
     {
         if (SendPacketNoLock (payload, payload_length))
            response_len = WaitForPacketWithTimeoutMicroSecondsNoLock (response, GetPacketTimeoutInMicroSeconds ());
@@ -628,7 +628,7 @@
     return false;
 }
 
-// This function takes a mutex locker as a parameter in case the GetSequenceMutex
+// This function takes a mutex locker as a parameter in case the TryLockSequenceMutex
 // actually succeeds. If it doesn't succeed in acquiring the sequence mutex 
 // (the expected result), then it will send the halt packet. If it does succeed
 // then the caller that requested the interrupt will want to keep the sequence
@@ -653,7 +653,7 @@
     if (IsRunning())
     {
         // Only send an interrupt if our debugserver is running...
-        if (GetSequenceMutex (locker) == false)
+        if (TryLockSequenceMutex (locker) == false)
         {
             // Someone has the mutex locked waiting for a response or for the
             // inferior to stop, so send the interrupt on the down low...
@@ -1842,7 +1842,7 @@
     Mutex::Locker locker;
     thread_ids.clear();
     
-    if (GetSequenceMutex (locker))
+    if (TryLockSequenceMutex (locker))
     {
         sequence_mutex_unavailable = false;
         StringExtractorGDBRemote response;

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp?rev=154365&r1=154364&r2=154365&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp Mon Apr  9 19:18:59 2012
@@ -181,7 +181,7 @@
     if (!m_reg_valid[reg])
     {
         Mutex::Locker locker;
-        if (gdb_comm.GetSequenceMutex (locker))
+        if (gdb_comm.TryLockSequenceMutex (locker))
         {
             const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
             ProcessSP process_sp (m_thread.GetProcess());
@@ -331,7 +331,7 @@
                                   m_reg_data.GetByteOrder()))   // dst byte order
     {
         Mutex::Locker locker;
-        if (gdb_comm.GetSequenceMutex (locker))
+        if (gdb_comm.TryLockSequenceMutex (locker))
         {
             const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
             ProcessSP process_sp (m_thread.GetProcess());
@@ -440,7 +440,7 @@
     StringExtractorGDBRemote response;
 
     Mutex::Locker locker;
-    if (gdb_comm.GetSequenceMutex (locker))
+    if (gdb_comm.TryLockSequenceMutex (locker))
     {
         char packet[32];
         const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
@@ -496,7 +496,7 @@
 
     StringExtractorGDBRemote response;
     Mutex::Locker locker;
-    if (gdb_comm.GetSequenceMutex (locker))
+    if (gdb_comm.TryLockSequenceMutex (locker))
     {
         const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
         ProcessSP process_sp (m_thread.GetProcess());

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp?rev=154365&r1=154364&r2=154365&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Mon Apr  9 19:18:59 2012
@@ -1129,7 +1129,7 @@
     return error;
 }
 
-uint32_t
+bool
 ProcessGDBRemote::UpdateThreadList (ThreadList &old_thread_list, ThreadList &new_thread_list)
 {
     // locker will keep a mutex locked until it goes out of scope
@@ -1151,11 +1151,17 @@
                 thread_sp.reset (new ThreadGDBRemote (shared_from_this(), tid));
             new_thread_list.AddThread(thread_sp);
         }
+        SetThreadStopInfo (m_last_stop_packet);
+    }
+    else if (sequence_mutex_unavailable)
+    {
+#if defined (LLDB_CONFIGURATION_DEBUG)
+        assert(!"ProcessGDBRemote::UpdateThreadList() failed due to not getting the sequence mutex");
+#endif
+        return false; // We just didn't get the list
     }
 
-    if (sequence_mutex_unavailable == false)
-        SetThreadStopInfo (m_last_stop_packet);
-    return new_thread_list.GetSize(false);
+    return true;
 }
 
 

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h?rev=154365&r1=154364&r2=154365&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h Mon Apr  9 19:18:59 2012
@@ -265,7 +265,7 @@
         return m_flags;
     }
 
-    uint32_t
+    virtual bool
     UpdateThreadList (lldb_private::ThreadList &old_thread_list, 
                       lldb_private::ThreadList &new_thread_list);
 

Modified: lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.cpp?rev=154365&r1=154364&r2=154365&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.cpp (original)
+++ lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.cpp Mon Apr  9 19:18:59 2012
@@ -337,7 +337,7 @@
     return m_dyld_ap.get();
 }
 
-uint32_t
+bool
 ProcessMachCore::UpdateThreadList (ThreadList &old_thread_list, ThreadList &new_thread_list)
 {
     if (old_thread_list.GetSize(false) == 0)
@@ -362,7 +362,7 @@
         for (uint32_t i=0; i<num_threads; ++i)
             new_thread_list.AddThread (old_thread_list.GetThreadAtIndex (i));
     }
-    return new_thread_list.GetSize(false);
+    return new_thread_list.GetSize(false) > 0;
 }
 
 void

Modified: lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.h?rev=154365&r1=154364&r2=154365&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.h (original)
+++ lldb/trunk/source/Plugins/Process/mach-core/ProcessMachCore.h Mon Apr  9 19:18:59 2012
@@ -116,7 +116,7 @@
     void
     Clear ( );
     
-    uint32_t
+    virtual bool
     UpdateThreadList (lldb_private::ThreadList &old_thread_list, 
                       lldb_private::ThreadList &new_thread_list);
     

Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=154365&r1=154364&r2=154365&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Mon Apr  9 19:18:59 2012
@@ -1236,13 +1236,15 @@
             // and the os->UpdateThreadList(...) so it doesn't change on us
             ThreadList new_thread_list(this);
             // Always update the thread list with the protocol specific
-            // thread list
-            UpdateThreadList (m_thread_list, new_thread_list);
-            OperatingSystem *os = GetOperatingSystem ();
-            if (os)
-                os->UpdateThreadList (m_thread_list, new_thread_list);
-            m_thread_list.Update (new_thread_list);
-            m_thread_list.SetStopID (stop_id);
+            // thread list, but only update if "true" is returned
+            if (UpdateThreadList (m_thread_list, new_thread_list))
+            {
+                OperatingSystem *os = GetOperatingSystem ();
+                if (os)
+                    os->UpdateThreadList (m_thread_list, new_thread_list);
+                m_thread_list.Update (new_thread_list);
+                m_thread_list.SetStopID (stop_id);
+            }
         }
     }
 }
@@ -4281,7 +4283,6 @@
     if (IS_VALID_LLDB_HOST_THREAD(backup_private_state_thread))
     {
         StopPrivateStateThread();
-        lldb::thread_result_t thread_result;
         Error error;
         // Host::ThreadJoin(m_private_state_thread, &thread_result, &error);
         m_private_state_thread = backup_private_state_thread;





More information about the lldb-commits mailing list