implement mutex order tracking

stepmania/src/RageThreads.cpp

@@ -21,6 +21,7 @@
 
 #include <signal.h>
 #include <errno.h>
+#include <set>
 
 /* 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<RageMutex*> *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 <sys/time.h>
 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<const RageMutex *> 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<int> *g_FreeMutexIDs = NULL;
 
 RageMutex::RageMutex( const CString name ):
 	m_sName( name )
 {
 	mut = new RageMutexImpl(this);
+
+	if( g_FreeMutexIDs == NULL )
+	{
+		g_FreeMutexIDs = new set<int>;
+		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<RageMutex*>;
+
+	g_MutexList->push_back( this );
 }
 
 RageMutex::~RageMutex()
 {
+	vector<RageMutex*>::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
 -----------------------------------------------------------------------------
 */

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