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
This commit is contained in:
Greg Clayton 2012-04-10 00:18:59 +00:00
parent 1d9672bdce
commit 9fc13556b4
17 changed files with 43 additions and 39 deletions

View File

@ -64,7 +64,7 @@ public:
//------------------------------------------------------------------
// Plug-in Methods
//------------------------------------------------------------------
virtual uint32_t
virtual bool
UpdateThreadList (ThreadList &old_thread_list, ThreadList &new_thread_list) = 0;
virtual void

View File

@ -2937,7 +2937,7 @@ public:
//------------------------------------------------------------------
// Thread Queries
//------------------------------------------------------------------
virtual uint32_t
virtual bool
UpdateThreadList (ThreadList &old_thread_list, ThreadList &new_thread_list) = 0;
void

View File

@ -223,7 +223,7 @@ OperatingSystemDarwinKernel::GetPluginVersion()
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 @@ OperatingSystemDarwinKernel::UpdateThreadList (ThreadList &old_thread_list, Thre
}
next_valobj_sp.swap(valobj_sp);
}
return new_thread_list.GetSize(false);
return new_thread_list.GetSize(false) > 0;
}
void

View File

@ -61,7 +61,7 @@ public:
//------------------------------------------------------------------
// lldb_private::OperatingSystem Methods
//------------------------------------------------------------------
virtual uint32_t
virtual bool
UpdateThreadList (lldb_private::ThreadList &old_thread_list,
lldb_private::ThreadList &new_thread_list);

View File

@ -151,7 +151,7 @@ CommunicationKDP::SendRequestPacketNoLock (const PacketStreamType &request_packe
}
bool
CommunicationKDP::GetSequenceMutex (Mutex::Locker& locker)
CommunicationKDP::TryLockSequenceMutex (Mutex::Locker& locker)
{
return locker.TryLock (m_sequence_mutex.GetMutex());
}

View File

@ -102,7 +102,7 @@ public:
uint32_t usec);
bool
GetSequenceMutex(lldb_private::Mutex::Locker& locker);
TryLockSequenceMutex(lldb_private::Mutex::Locker& locker);
bool
CheckForPacket (const uint8_t *src,

View File

@ -301,7 +301,7 @@ ProcessKDP::DoResume ()
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 @@ ProcessKDP::UpdateThreadList (ThreadList &old_thread_list, ThreadList &new_threa
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;
}

View File

@ -231,7 +231,7 @@ protected:
void
Clear ( );
uint32_t
virtual bool
UpdateThreadList (lldb_private::ThreadList &old_thread_list,
lldb_private::ThreadList &new_thread_list);

View File

@ -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());
}

View File

@ -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,

View File

@ -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;

View File

@ -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());

View File

@ -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;
}

View File

@ -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);

View File

@ -337,7 +337,7 @@ ProcessMachCore::GetDynamicLoader ()
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 @@ ProcessMachCore::UpdateThreadList (ThreadList &old_thread_list, ThreadList &new_
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

View File

@ -116,7 +116,7 @@ protected:
void
Clear ( );
uint32_t
virtual bool
UpdateThreadList (lldb_private::ThreadList &old_thread_list,
lldb_private::ThreadList &new_thread_list);

View File

@ -1236,13 +1236,15 @@ Process::UpdateThreadListIfNeeded ()
// 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 @@ Process::RunThreadPlan (ExecutionContext &exe_ctx,
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;