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.

llvm-svn: 154365

6 files changed, 21 insertions, 18 deletions

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index 5b589dce795..caa70258e30 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -201,10 +201,7 @@ GDBRemoteCommunication::SendPacketNoLock (const char *payload, size_t payload_le
             // 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 @@ GDBRemoteCommunication::GetAck ()
 }
 
 bool
-GDBRemoteCommunication::GetSequenceMutex (Mutex::Locker& locker)
+GDBRemoteCommunication::TryLockSequenceMutex (Mutex::Locker& locker)
 {
     return locker.TryLock (m_sequence_mutex.GetMutex());
 }
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
index 0e9ea0dcb1d..d7986d5b278 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -74,7 +74,7 @@ public:
                         size_t payload_length);
 
     bool
-    GetSequenceMutex(lldb_private::Mutex::Locker& locker);
+    TryLockSequenceMutex(lldb_private::Mutex::Locker& locker);
 
     bool
     CheckForPacket (const uint8_t *src, 
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index e308f915d1c..4fbfa982e0d 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -240,7 +240,7 @@ GDBRemoteCommunicationClient::SendPacketAndWaitForResponse
     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 @@ GDBRemoteCommunicationClient::SendAsyncSignal (int signo)
     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 @@ GDBRemoteCommunicationClient::SendInterrupt
     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 @@ GDBRemoteCommunicationClient::GetCurrentThreadIDs (std::vector<lldb::tid_t> &thr
     Mutex::Locker locker;
     thread_ids.clear();
     
-    if (GetSequenceMutex (locker))
+    if (TryLockSequenceMutex (locker))
     {
         sequence_mutex_unavailable = false;
         StringExtractorGDBRemote response;
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
index 77051fffc6e..120caf3d7a7 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -181,7 +181,7 @@ GDBRemoteRegisterContext::ReadRegisterBytes (const RegisterInfo *reg_info, DataE
     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 @@ GDBRemoteRegisterContext::WriteRegisterBytes (const lldb_private::RegisterInfo *
                                   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 @@ GDBRemoteRegisterContext::ReadAllRegisterValues (lldb::DataBufferSP &data_sp)
     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 @@ GDBRemoteRegisterContext::WriteAllRegisterValues (const lldb::DataBufferSP &data
 
     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());
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 8a1f120f308..8b667dffd8d 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1129,7 +1129,7 @@ ProcessGDBRemote::DoResume ()
     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 @@ ProcessGDBRemote::UpdateThreadList (ThreadList &old_thread_list, ThreadList &new
                 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;
 }
 
 
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
index 9f3b75874ea..a41957fdc0a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -265,7 +265,7 @@ protected:
         return m_flags;
     }
 
-    uint32_t
+    virtual bool
     UpdateThreadList (lldb_private::ThreadList &old_thread_list, 
                       lldb_private::ThreadList &new_thread_list);