diff options
7 files changed, 127 insertions, 119 deletions
diff --git a/lldb/include/lldb/Host/Mutex.h b/lldb/include/lldb/Host/Mutex.h index b848c151285..38f4f315507 100644 --- a/lldb/include/lldb/Host/Mutex.h +++ b/lldb/include/lldb/Host/Mutex.h @@ -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) diff --git a/lldb/source/Host/common/Mutex.cpp b/lldb/source/Host/common/Mutex.cpp index d519baf45b6..e8091bd3e0c 100644 --- a/lldb/source/Host/common/Mutex.cpp +++ b/lldb/source/Host/common/Mutex.cpp @@ -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,100 +264,98 @@ Mutex::GetMutex() return &m_mutex; } +//---------------------------------------------------------------------- +// Locks the mutex owned by this object, if the mutex is already +// locked, the calling thread will block until the mutex becomes +// available. +// +// RETURNS +// The error code from the pthread_mutex_lock() function call. +//---------------------------------------------------------------------- int -Mutex::Lock (pthread_mutex_t *mutex_ptr) +Mutex::Lock() { 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); + error_check_mutex (&m_mutex, eMutexActionAssertInitialized); #endif - int err = ::pthread_mutex_lock (mutex_ptr); + 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__, mutex_ptr, err, strerror(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(), mutex_ptr, err); + DEBUG_LOG ("[%4.4x/%4.4x] pthread_mutex_lock (%p) => %i\n", Host::GetCurrentProcessID(), Host::GetCurrentThreadID(), &m_mutex, err); return err; } +//---------------------------------------------------------------------- +// Attempts to lock the mutex owned by this object without blocking. +// If the mutex is already locked, TryLock() will not block waiting +// for the mutex, but will return an error condition. +// +// RETURNS +// The error code from the pthread_mutex_trylock() function call. +//---------------------------------------------------------------------- int -Mutex::TryLock (pthread_mutex_t *mutex_ptr) +Mutex::TryLock(const char *failure_message) { #if ENABLE_MUTEX_ERROR_CHECKING - error_check_mutex (mutex_ptr, eMutexActionAssertInitialized); + error_check_mutex (&m_mutex, 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); + 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; } +//---------------------------------------------------------------------- +// If the current thread holds the lock on the owned mutex, then +// Unlock() will unlock the mutex. Calling Unlock() on this object +// that the calling thread does not hold will result in undefined +// behavior. +// +// RETURNS +// The error code from the pthread_mutex_unlock() function call. +//---------------------------------------------------------------------- int -Mutex::Unlock (pthread_mutex_t *mutex_ptr) +Mutex::Unlock() { #if ENABLE_MUTEX_ERROR_CHECKING - error_check_mutex (mutex_ptr, eMutexActionAssertInitialized); + error_check_mutex (&m_mutex, eMutexActionAssertInitialized); #endif - int err = ::pthread_mutex_unlock (mutex_ptr); + 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__, mutex_ptr, err, strerror(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(), mutex_ptr, err); + DEBUG_LOG ("[%4.4x/%4.4x] pthread_mutex_unlock (%p) => %i\n", Host::GetCurrentProcessID(), Host::GetCurrentThreadID(), &m_mutex, 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 -// available. -// -// RETURNS -// The error code from the pthread_mutex_lock() function call. -//---------------------------------------------------------------------- -int -Mutex::Lock() -{ - return Mutex::Lock (&m_mutex); -} - -//---------------------------------------------------------------------- -// Attempts to lock the mutex owned by this object without blocking. -// If the mutex is already locked, TryLock() will not block waiting -// for the mutex, but will return an error condition. -// -// RETURNS -// The error code from the pthread_mutex_trylock() function call. -//---------------------------------------------------------------------- +#ifdef LLDB_CONFIGURATION_DEBUG int -Mutex::TryLock() +TrackingMutex::Unlock () { - return Mutex::TryLock (&m_mutex); + 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 + -//---------------------------------------------------------------------- -// If the current thread holds the lock on the owned mutex, then -// Unlock() will unlock the mutex. Calling Unlock() on this object -// that the calling thread does not hold will result in undefined -// behavior. -// -// RETURNS -// The error code from the pthread_mutex_unlock() function call. -//---------------------------------------------------------------------- -int -Mutex::Unlock() -{ - return Mutex::Unlock (&m_mutex); -} diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index fc83724fb25..0712ed4e1f7 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -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; diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h index b3bad7ede5c..a1077957c6a 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h @@ -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; diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index 90fb47eb7a3..b08350a5ec8 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -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(); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp index 9eec09d0b5c..4247add8554 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp @@ -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; } diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index be5ccab7b90..ed826bac7c2 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -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; |
