From d48f416ec344d974f455fbd7190ba56b5572f231 Mon Sep 17 00:00:00 2001 From: Glenn Maynard Date: Sun, 21 Mar 2004 08:36:43 +0000 Subject: [PATCH] implement mutex order tracking --- stepmania/src/RageThreads.cpp | 123 +++++++++++++++++++++++++++++++++- stepmania/src/RageThreads.h | 9 +++ 2 files changed, 131 insertions(+), 1 deletion(-) diff --git a/stepmania/src/RageThreads.cpp b/stepmania/src/RageThreads.cpp index d1c6bd9103..065bdfaf35 100644 --- a/stepmania/src/RageThreads.cpp +++ b/stepmania/src/RageThreads.cpp @@ -21,6 +21,7 @@ #include #include +#include /* SDL threads aren't quite enough. We need to be able to suspend or * kill all threads, including the main one. SDL doesn't count the @@ -40,6 +41,7 @@ #endif #define MAX_THREADS 128 +static vector *g_MutexList = NULL; /* watch out for static initialization order problems */ static const unsigned int UnknownThreadID = 0xFFFFFFFF; struct ThreadSlot @@ -482,6 +484,7 @@ struct RageMutexImpl void Lock(); void Unlock(); + bool IsLockedByThisThread() const; }; RageMutexImpl::RageMutexImpl( RageMutex *parent ) @@ -563,6 +566,11 @@ void RageMutexImpl::Unlock() sm_crash(); } +bool RageMutexImpl::IsLockedByThisThread() const +{ + return LockedBy == GetCurrentThreadId(); +} + #elif defined(HAVE_LIBPTHREAD) #include struct RageMutexImpl @@ -578,6 +586,7 @@ struct RageMutexImpl void Lock(); void Unlock(); + bool IsLockedByThisThread() const; }; RageMutexImpl::RageMutexImpl( RageMutex *parent ) @@ -687,6 +696,11 @@ void RageMutexImpl::Unlock() pthread_mutex_unlock( &mutex ); } +bool RageMutexImpl::IsLockedByThisThread() const +{ + return LockedBy == SDL_ThreadID(); +} + #else /* SDL implementation. */ struct RageMutexImpl @@ -702,6 +716,7 @@ struct RageMutexImpl void Lock(); void Unlock(); + bool IsLockedByThisThread() const; }; RageMutexImpl::RageMutexImpl( RageMutex *parent ) @@ -741,24 +756,125 @@ void RageMutexImpl::Unlock() LockedBy = 0; SDL_UnlockMutex( mutex ); } + +bool RageMutexImpl::IsLockedByThisThread() const +{ + return LockedBy == SDL_ThreadID(); +} + #endif +static const int MAX_MUTEXES = 256; +/* g_MutexesBefore[n] is a list of mutex IDs which must be locked before n (if at all). + * The array g_MutexesBefore[n] is locked for writing by locking mutex n, so lock that + * mutex *before* calling MarkLockedMutex(). */ +bool g_MutexesBefore[MAX_MUTEXES][MAX_MUTEXES]; + +void RageMutex::MarkLockedMutex() +{ + /* This only makes locking take about 25% longer, and we generally don't lock in + * inner loops, so this is enabled by default for now. */ +// if( !g_bEnableMutexOrderChecking ) +// return; + + const int ID = this->m_UniqueID; + ASSERT( ID < MAX_MUTEXES ); + + /* This is a queue of all mutexes that must be locked before ID, if at all. */ + vector before; + + /* Iterate over all locked mutexes that are locked by this thread. */ + unsigned i; + for( i = 0; i < g_MutexList->size(); ++i ) + { + const RageMutex *mutex = (*g_MutexList)[i]; + + if( mutex->m_UniqueID == this->m_UniqueID ) + continue; + + if( !mutex->IsLockedByThisThread() ) + continue; + + /* mutex must be locked before this. If we've previously marked the opposite, + * then we have an inconsistent lock order. */ + if( g_MutexesBefore[mutex->m_UniqueID][this->m_UniqueID] ) + { + LOG->Warn( "Mutex lock inconsistency: mutex \"%s\" must be locked before \"%s\"", + this->GetName().c_str(), mutex->GetName().c_str() ); + + break; + } + + /* Optimization: don't add it to the queue if it's already been done. */ + if( !g_MutexesBefore[this->m_UniqueID][mutex->m_UniqueID] ) + before.push_back( mutex ); + } + + while( before.size() ) + { + const RageMutex *mutex = before.back(); + before.pop_back(); + + g_MutexesBefore[this->m_UniqueID][mutex->m_UniqueID] = 1; + + /* All IDs which must be locked before mutex must also be locked before + * this. That is, if A < mutex, because mutex < this, mark A < this. */ + for( i = 0; i < g_MutexList->size(); ++i ) + { + const RageMutex *mutex2 = (*g_MutexList)[i]; + if( g_MutexesBefore[mutex->m_UniqueID][mutex2->m_UniqueID] ) + before.push_back( mutex2 ); + } + } +} + +/* XXX: How can g_FreeMutexIDs and g_MutexList be threadsafed? */ +static set *g_FreeMutexIDs = NULL; RageMutex::RageMutex( const CString name ): m_sName( name ) { mut = new RageMutexImpl(this); + + if( g_FreeMutexIDs == NULL ) + { + g_FreeMutexIDs = new set; + for( int i = 0; i < MAX_MUTEXES; ++i ) + g_FreeMutexIDs->insert( i ); + } + + RAGE_ASSERT_M( !g_FreeMutexIDs->empty(), ssprintf("MAX_MUTEXES exceeded creating \"%s\"", name.c_str() ) ); + m_UniqueID = *g_FreeMutexIDs->begin(); + g_FreeMutexIDs->erase( g_FreeMutexIDs->begin() ); + + if( g_MutexList == NULL ) + g_MutexList = new vector; + + g_MutexList->push_back( this ); } RageMutex::~RageMutex() { + vector::iterator it = find( g_MutexList->begin(), g_MutexList->end(), this ); + ASSERT( it != g_MutexList->end() ); + g_MutexList->erase( it ); + if( g_MutexList->empty() ) + { + delete g_MutexList; + g_MutexList = NULL; + } + delete mut; + + g_FreeMutexIDs->insert( m_UniqueID ); + } void RageMutex::Lock() { mut->Lock(); + MarkLockedMutex(); } void RageMutex::Unlock() @@ -766,6 +882,11 @@ void RageMutex::Unlock() mut->Unlock(); } +bool RageMutex::IsLockedByThisThread() const +{ + return mut->IsLockedByThisThread(); +} + LockMutex::LockMutex(RageMutex &mut, const char *file_, int line_): mutex(mut), file(file_), @@ -802,7 +923,7 @@ void LockMutex::Unlock() ----------------------------------------------------------------------------- File: RageThreads - Copyright (c) 2001-2002 by the person(s) listed below. All rights reserved. + Copyright (c) 2001-2004 by the person(s) listed below. All rights reserved. Glenn Maynard ----------------------------------------------------------------------------- */ diff --git a/stepmania/src/RageThreads.h b/stepmania/src/RageThreads.h index 5a59bf0ec1..d9c71867e3 100644 --- a/stepmania/src/RageThreads.h +++ b/stepmania/src/RageThreads.h @@ -46,12 +46,21 @@ namespace Checkpoints struct RageMutexImpl; class RageMutex { + friend struct RageMutexImpl; + RageMutexImpl *mut; const CString m_sName; + int m_UniqueID; + + void MarkLockedMutex(); + public: + CString GetName() const { return m_sName; } void Lock(); void Unlock(); + bool IsLockedByThisThread() const; + RageMutex( CString name ); ~RageMutex(); };