maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

Change the Mutex::Locker class so that it takes the Mutex object and locks it, rather 

than being given the pthread_mutex_t from the Mutex and locks that.  That allows us to
track ownership of the Mutex better.  

Used this to switch the LLDB_CONFIGURATION_DEBUG enabled assert when we can't get the
gdb-remote sequence mutex to assert when the thread that had the mutex releases it.  This
is generally more useful information than saying just who failed to get it (since the
code that had it locked often had released it by the time the assert fired.)

llvm-svn: 158240
This commit is contained in:
Jim Ingham 2012-06-08 22:50:40 +00:00
parent d8d5669435
commit 4ceb928f02
7 changed files with 134 additions and 126 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
lldb/include/lldb/Host/Mutex.h
View File

@ -14,6 +14,10 @@
#include <pthread.h>
#include <assert.h>
#ifdef LLDB_CONFIGURATION_DEBUG
#include <string>
#endif
namespace lldb_private {
//----------------------------------------------------------------------
@ -121,13 +125,13 @@ public:
/// returns \b false otherwise.
//--------------------------------------------------------------
bool
TryLock (Mutex &mutex);
TryLock (Mutex &mutex, const char *failure_message = NULL);
bool
TryLock (Mutex *mutex)
TryLock (Mutex *mutex, const char *failure_message = NULL)
{
if (mutex)
return TryLock(*mutex);
return TryLock(*mutex, failure_message);
else
return false;
}
@ -139,9 +143,7 @@ public:
//--------------------------------------------------------------
/// Member variables
//--------------------------------------------------------------
pthread_mutex_t *m_mutex_ptr; ///< A pthread mutex that is locked when
///< acquired and unlocked when destroyed
///< or reset.
Mutex *m_mutex_ptr;
private:
Locker(const Locker&);
@ -176,6 +178,9 @@ public:
///
/// Destroys the mutex owned by this object.
//------------------------------------------------------------------
#ifdef LLDB_CONFIGURATION_DEBUG
virtual
#endif
~Mutex();
//------------------------------------------------------------------
@ -201,8 +206,11 @@ public:
/// @return
/// The error code from \c pthread_mutex_trylock().
//------------------------------------------------------------------
#ifdef LLDB_CONFIGURATION_DEBUG
virtual
#endif
int
TryLock();
TryLock(const char *failure_message = NULL);
//------------------------------------------------------------------
/// Unlock the mutex.
@ -215,22 +223,18 @@ public:
/// @return
/// The error code from \c pthread_mutex_unlock().
//------------------------------------------------------------------
#ifdef LLDB_CONFIGURATION_DEBUG
virtual
#endif
int
Unlock();
static
int Lock (pthread_mutex_t *mutex_ptr);
static
int TryLock (pthread_mutex_t *mutex_ptr);
static
int Unlock (pthread_mutex_t *mutex_ptr);
protected:
//------------------------------------------------------------------
// Member variables
//------------------------------------------------------------------
// TODO: Hide the mutex in the implementation file in case we ever need to port to an
// architecture that doesn't have pthread mutexes.
pthread_mutex_t m_mutex; ///< The pthread mutex object.
private:
@ -247,6 +251,37 @@ private:
const Mutex& operator=(const Mutex&);
};
#ifdef LLDB_CONFIGURATION_DEBUG
class TrackingMutex : public Mutex
{
public:
TrackingMutex() : Mutex() {}
TrackingMutex(Mutex::Type type) : Mutex (type) {}
virtual
~TrackingMutex() {}
virtual int
Unlock ();
virtual int
TryLock (const char *failure_message = NULL)
{
int return_value = Mutex::TryLock();
if (return_value != 0 && failure_message != NULL)
{
m_failure_message.assign(failure_message);
m_thread_that_tried = pthread_self();
}
return return_value;
}
protected:
pthread_t m_thread_that_tried;
std::string m_failure_message;
};
#endif
} // namespace lldb_private
#endif // #if defined(__cplusplus)

145
lldb/source/Host/common/Mutex.cpp
View File

@ -141,19 +141,14 @@ Mutex::Locker::~Locker ()
void
Mutex::Locker::Lock (Mutex &mutex)
{
pthread_mutex_t *mutex_ptr = mutex.GetMutex();
// We already have this mutex locked or both are NULL...
if (m_mutex_ptr == mutex_ptr)
if (m_mutex_ptr == &mutex)
return;
Unlock ();
if (mutex_ptr)
{
m_mutex_ptr = mutex_ptr;
Mutex::Lock (m_mutex_ptr);
}
m_mutex_ptr = &mutex;
m_mutex_ptr->Lock();
}
void
@ -161,27 +156,23 @@ Mutex::Locker::Unlock ()
{
if (m_mutex_ptr)
{
Mutex::Unlock (m_mutex_ptr);
m_mutex_ptr->Unlock ();
m_mutex_ptr = NULL;
}
}
bool
Mutex::Locker::TryLock (Mutex &mutex)
Mutex::Locker::TryLock (Mutex &mutex, const char *failure_message)
{
pthread_mutex_t *mutex_ptr = mutex.GetMutex();
// We already have this mutex locked!
if (m_mutex_ptr == mutex_ptr)
return m_mutex_ptr != NULL;
if (m_mutex_ptr == &mutex)
return true;
Unlock ();
if (mutex_ptr)
{
if (Mutex::TryLock (mutex_ptr) == 0)
m_mutex_ptr = mutex_ptr;
}
if (mutex.TryLock(failure_message) == 0)
m_mutex_ptr = &mutex;
return m_mutex_ptr != NULL;
}
@ -273,61 +264,6 @@ Mutex::GetMutex()
return &m_mutex;
}
int
Mutex::Lock (pthread_mutex_t *mutex_ptr)
{
DEBUG_LOG ("[%4.4x/%4.4x] pthread_mutex_lock (%p)...\n", Host::GetCurrentProcessID(), Host::GetCurrentThreadID(), mutex_ptr);
#if ENABLE_MUTEX_ERROR_CHECKING
error_check_mutex (mutex_ptr, eMutexActionAssertInitialized);
#endif
int err = ::pthread_mutex_lock (mutex_ptr);
#if ENABLE_MUTEX_ERROR_CHECKING
if (err)
{
Host::SetCrashDescriptionWithFormat ("%s error: pthread_mutex_lock(%p) => err = %i (%s)", __PRETTY_FUNCTION__, mutex_ptr, err, strerror(err));
assert(err == 0);
}
#endif
DEBUG_LOG ("[%4.4x/%4.4x] pthread_mutex_lock (%p) => %i\n", Host::GetCurrentProcessID(), Host::GetCurrentThreadID(), mutex_ptr, err);
return err;
}
int
Mutex::TryLock (pthread_mutex_t *mutex_ptr)
{
#if ENABLE_MUTEX_ERROR_CHECKING
error_check_mutex (mutex_ptr, eMutexActionAssertInitialized);
#endif
int err = ::pthread_mutex_trylock (mutex_ptr);
DEBUG_LOG ("[%4.4x/%4.4x] pthread_mutex_trylock (%p) => %i\n", Host::GetCurrentProcessID(), Host::GetCurrentThreadID(), mutex_ptr, err);
return err;
}
int
Mutex::Unlock (pthread_mutex_t *mutex_ptr)
{
#if ENABLE_MUTEX_ERROR_CHECKING
error_check_mutex (mutex_ptr, eMutexActionAssertInitialized);
#endif
int err = ::pthread_mutex_unlock (mutex_ptr);
#if ENABLE_MUTEX_ERROR_CHECKING
if (err)
{
Host::SetCrashDescriptionWithFormat ("%s error: pthread_mutex_unlock(%p) => err = %i (%s)", __PRETTY_FUNCTION__, mutex_ptr, err, strerror(err));
assert(err == 0);
}
#endif
DEBUG_LOG ("[%4.4x/%4.4x] pthread_mutex_unlock (%p) => %i\n", Host::GetCurrentProcessID(), Host::GetCurrentThreadID(), mutex_ptr, err);
return err;
}
//----------------------------------------------------------------------
// Locks the mutex owned by this object, if the mutex is already
// locked, the calling thread will block until the mutex becomes
@ -339,7 +275,24 @@ Mutex::Unlock (pthread_mutex_t *mutex_ptr)
int
Mutex::Lock()
{
return Mutex::Lock (&m_mutex);
DEBUG_LOG ("[%4.4x/%4.4x] pthread_mutex_lock (%p)...\n", Host::GetCurrentProcessID(), Host::GetCurrentThreadID(), mutex_ptr);
#if ENABLE_MUTEX_ERROR_CHECKING
error_check_mutex (&m_mutex, eMutexActionAssertInitialized);
#endif
int err = ::pthread_mutex_lock (&m_mutex);
#if ENABLE_MUTEX_ERROR_CHECKING
if (err)
{
Host::SetCrashDescriptionWithFormat ("%s error: pthread_mutex_lock(%p) => err = %i (%s)", __PRETTY_FUNCTION__, &m_mutex, err, strerror(err));
assert(err == 0);
}
#endif
DEBUG_LOG ("[%4.4x/%4.4x] pthread_mutex_lock (%p) => %i\n", Host::GetCurrentProcessID(), Host::GetCurrentThreadID(), &m_mutex, err);
return err;
}
//----------------------------------------------------------------------
@ -351,9 +304,15 @@ Mutex::Lock()
// The error code from the pthread_mutex_trylock() function call.
//----------------------------------------------------------------------
int
Mutex::TryLock()
Mutex::TryLock(const char *failure_message)
{
return Mutex::TryLock (&m_mutex);
#if ENABLE_MUTEX_ERROR_CHECKING
error_check_mutex (&m_mutex, eMutexActionAssertInitialized);
#endif
int err = ::pthread_mutex_trylock (&m_mutex);
DEBUG_LOG ("[%4.4x/%4.4x] pthread_mutex_trylock (%p) => %i\n", Host::GetCurrentProcessID(), Host::GetCurrentThreadID(), &m_mutex, err);
return err;
}
//----------------------------------------------------------------------
@ -368,5 +327,35 @@ Mutex::TryLock()
int
Mutex::Unlock()
{
return Mutex::Unlock (&m_mutex);
#if ENABLE_MUTEX_ERROR_CHECKING
error_check_mutex (&m_mutex, eMutexActionAssertInitialized);
#endif
int err = ::pthread_mutex_unlock (&m_mutex);
#if ENABLE_MUTEX_ERROR_CHECKING
if (err)
{
Host::SetCrashDescriptionWithFormat ("%s error: pthread_mutex_unlock(%p) => err = %i (%s)", __PRETTY_FUNCTION__, &m_mutex, err, strerror(err));
assert(err == 0);
}
#endif
DEBUG_LOG ("[%4.4x/%4.4x] pthread_mutex_unlock (%p) => %i\n", Host::GetCurrentProcessID(), Host::GetCurrentThreadID(), &m_mutex, err);
return err;
}
#ifdef LLDB_CONFIGURATION_DEBUG
int
TrackingMutex::Unlock ()
{
if (!m_failure_message.empty())
Host::SetCrashDescriptionWithFormat ("Unlocking lock (on thread %p) that thread: %p failed to get: %s",
pthread_self(),
m_thread_that_tried,
m_failure_message.c_str());
assert (m_failure_message.empty());
return Mutex::Unlock();
}
#endif

4
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
View File

@ -263,10 +263,10 @@ GDBRemoteCommunication::GetAck ()
}
bool
GDBRemoteCommunication::GetSequenceMutex (Mutex::Locker& locker)
GDBRemoteCommunication::GetSequenceMutex (Mutex::Locker& locker, const char *failure_message)
{
if (IsRunning())
return locker.TryLock (m_sequence_mutex);
return locker.TryLock (m_sequence_mutex, failure_message);
locker.Lock (m_sequence_mutex);
return true;

6
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
View File

@ -59,7 +59,7 @@ public:
size_t payload_length);
bool
GetSequenceMutex (lldb_private::Mutex::Locker& locker);
GetSequenceMutex (lldb_private::Mutex::Locker& locker, const char *failure_message = NULL);
bool
CheckForPacket (const uint8_t *src,
@ -242,7 +242,11 @@ protected:
// Classes that inherit from GDBRemoteCommunication can see and modify these
//------------------------------------------------------------------
uint32_t m_packet_timeout;
#ifdef LLDB_CONFIGURATION_DEBUG
lldb_private::TrackingMutex m_sequence_mutex;
#else
lldb_private::Mutex m_sequence_mutex; // Restrict access to sending/receiving packets to a single thread at a time
#endif
lldb_private::Predicate<bool> m_public_is_running;
lldb_private::Predicate<bool> m_private_is_running;
History m_history;

6
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
View File

@ -1939,7 +1939,7 @@ GDBRemoteCommunicationClient::GetCurrentThreadIDs (std::vector<lldb::tid_t> &thr
Mutex::Locker locker;
thread_ids.clear();
if (GetSequenceMutex (locker))
if (GetSequenceMutex (locker, "ProcessGDBRemote::UpdateThreadList() failed due to not getting the sequence mutex"))
{
sequence_mutex_unavailable = false;
StringExtractorGDBRemote response;
@ -1968,9 +1968,13 @@ GDBRemoteCommunicationClient::GetCurrentThreadIDs (std::vector<lldb::tid_t> &thr
}
else
{
#if defined (LLDB_CONFIGURATION_DEBUG)
// assert(!"ProcessGDBRemote::UpdateThreadList() failed due to not getting the sequence mutex");
#else
LogSP log (ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet (GDBR_LOG_PROCESS | GDBR_LOG_PACKETS));
if (log)
log->Printf("error: failed to get packet sequence mutex, not sending packet 'qfThreadInfo'");
#endif
sequence_mutex_unavailable = true;
}
return thread_ids.size();

29
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
View File

@ -183,7 +183,7 @@ GDBRemoteRegisterContext::ReadRegisterBytes (const RegisterInfo *reg_info, DataE
if (!m_reg_valid[reg])
{
Mutex::Locker locker;
if (gdb_comm.GetSequenceMutex (locker))
if (gdb_comm.GetSequenceMutex (locker, "Didn't get sequence mutex for read register."))
{
const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
ProcessSP process_sp (m_thread.GetProcess());
@ -357,7 +357,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.GetSequenceMutex (locker, "Didn't get sequence mutex for write register."))
{
const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
ProcessSP process_sp (m_thread.GetProcess());
@ -445,12 +445,6 @@ GDBRemoteRegisterContext::WriteRegisterBytes (const lldb_private::RegisterInfo *
else
{
LogSP log (ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet (GDBR_LOG_THREAD | GDBR_LOG_PACKETS));
#if LLDB_CONFIGURATION_DEBUG
StreamString strm;
gdb_comm.DumpHistory(strm);
Host::SetCrashDescription (strm.GetData());
assert (!"Didn't get sequence mutex for write register.");
#else
if (log)
{
if (log->GetVerbose())
@ -462,7 +456,6 @@ GDBRemoteRegisterContext::WriteRegisterBytes (const lldb_private::RegisterInfo *
else
log->Printf("error: failed to get packet sequence mutex, not sending write register for \"%s\"", reg_info->name);
}
#endif
}
}
return false;
@ -484,7 +477,7 @@ GDBRemoteRegisterContext::ReadAllRegisterValues (lldb::DataBufferSP &data_sp)
StringExtractorGDBRemote response;
Mutex::Locker locker;
if (gdb_comm.GetSequenceMutex (locker))
if (gdb_comm.GetSequenceMutex (locker, "Didn't get sequence mutex for read all registers."))
{
char packet[32];
const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
@ -522,12 +515,6 @@ GDBRemoteRegisterContext::ReadAllRegisterValues (lldb::DataBufferSP &data_sp)
else
{
LogSP log (ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet (GDBR_LOG_THREAD | GDBR_LOG_PACKETS));
#if LLDB_CONFIGURATION_DEBUG
StreamString strm;
gdb_comm.DumpHistory(strm);
Host::SetCrashDescription (strm.GetData());
assert (!"Didn't get sequence mutex for read all registers.");
#else
if (log)
{
if (log->GetVerbose())
@ -539,7 +526,6 @@ GDBRemoteRegisterContext::ReadAllRegisterValues (lldb::DataBufferSP &data_sp)
else
log->Printf("error: failed to get packet sequence mutex, not sending read all registers");
}
#endif
}
data_sp.reset();
@ -563,7 +549,7 @@ GDBRemoteRegisterContext::WriteAllRegisterValues (const lldb::DataBufferSP &data
StringExtractorGDBRemote response;
Mutex::Locker locker;
if (gdb_comm.GetSequenceMutex (locker))
if (gdb_comm.GetSequenceMutex (locker, "Didn't get sequence mutex for write all registers."))
{
const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
ProcessSP process_sp (m_thread.GetProcess());
@ -662,12 +648,6 @@ GDBRemoteRegisterContext::WriteAllRegisterValues (const lldb::DataBufferSP &data
else
{
LogSP log (ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet (GDBR_LOG_THREAD | GDBR_LOG_PACKETS));
#if LLDB_CONFIGURATION_DEBUG
StreamString strm;
gdb_comm.DumpHistory(strm);
Host::SetCrashDescription (strm.GetData());
assert (!"Didn't get sequence mutex for write all registers.");
#else
if (log)
{
if (log->GetVerbose())
@ -679,7 +659,6 @@ GDBRemoteRegisterContext::WriteAllRegisterValues (const lldb::DataBufferSP &data
else
log->Printf("error: failed to get packet sequence mutex, not sending write all registers");
}
#endif
}
return false;
}

3
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
View File

@ -1190,9 +1190,6 @@ ProcessGDBRemote::UpdateThreadIDList ()
m_gdb_comm.GetCurrentThreadIDs (m_thread_ids, sequence_mutex_unavailable);
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
}
return true;