From: <axl...@us...> - 2010-01-15 18:11:50
|
Revision: 660 http://hgengine.svn.sourceforge.net/hgengine/?rev=660&view=rev Author: axlecrusher Date: 2010-01-15 18:11:44 +0000 (Fri, 15 Jan 2010) Log Message: ----------- Fix deadlock on windows. Mutexes can not have a default name. In windows, mutexes with identical names are shared. Modified Paths: -------------- Mercury2/src/MScopedArray.h Mercury2/src/MercuryThreads.cpp Mercury2/src/MercuryThreads.h Mercury2/src/ModuleManager.cpp Modified: Mercury2/src/MScopedArray.h =================================================================== --- Mercury2/src/MScopedArray.h 2010-01-14 21:30:42 UTC (rev 659) +++ Mercury2/src/MScopedArray.h 2010-01-15 18:11:44 UTC (rev 660) @@ -14,9 +14,9 @@ MScopedArray(T* ptr) :m_ptr(ptr), m_count(NULL) { - m_criticalSection.Wait(); + m_criticalSection.DangerousWait(); IncrementReference(); - m_criticalSection.UnLock(); + m_criticalSection.DangerousWait(); } MScopedArray() @@ -35,26 +35,26 @@ inline ~MScopedArray() { - m_criticalSection.Wait(); + m_criticalSection.DangerousWait(); DecrementReference(); - m_criticalSection.UnLock(); + m_criticalSection.DangerousUnLock(); } inline unsigned int Count() { unsigned int count = 0; - m_criticalSection.Wait(); + m_criticalSection.DangerousWait(); if( m_ptr && m_count ) count = *m_count; - m_criticalSection.UnLock(); + m_criticalSection.DangerousUnLock(); return count; } void Clear() { - m_criticalSection.Wait(); + m_criticalSection.DangerousWait(); DecrementReference(); m_ptr = NULL; - m_criticalSection.UnLock(); + m_criticalSection.DangerousUnLock(); } /* void Forget() @@ -85,11 +85,11 @@ //Assignment MScopedArray& operator=(const MScopedArray<T>& rhs) { - m_criticalSection.Wait(); + m_criticalSection.DangerousWait(); DecrementReference(); m_ptr = rhs.m_ptr; IncrementReference(); - m_criticalSection.UnLock(); + m_criticalSection.DangerousUnLock(); return *this; } Modified: Mercury2/src/MercuryThreads.cpp =================================================================== --- Mercury2/src/MercuryThreads.cpp 2010-01-14 21:30:42 UTC (rev 659) +++ Mercury2/src/MercuryThreads.cpp 2010-01-15 18:11:44 UTC (rev 660) @@ -4,6 +4,9 @@ #include <windows.h> #endif +//XXX WARNING in windows mutex of the same name are shared!!! +//we can not give mutexes a default name + #include <stdio.h> MercuryThread::MercuryThread() @@ -131,7 +134,7 @@ //Mutex functions MercuryMutex::MercuryMutex( ) -:m_name("(null)") +:m_heldBy(0) { iLockCount = 0; Open( ); @@ -139,7 +142,7 @@ } MercuryMutex::MercuryMutex( const MString &name ) -:m_name(name) +:m_name(name),m_heldBy(0) { iLockCount = 0; Open( ); @@ -151,11 +154,26 @@ Close( ); } -int MercuryMutex::Wait( long lMilliseconds ) +void MercuryMutex::SetName(const MString& name) { + m_name = name; +} + +bool MercuryMutex::Wait( long lMilliseconds ) +{ int r = 0; #if defined( WIN32 ) r = WaitForSingleObject( m_mutex, lMilliseconds ); + + switch( r ) + { + case WAIT_TIMEOUT: + fprintf(stderr, "Mutex held by thread ID 0x%x timed out (%d locks)\n", m_heldBy, iLockCount ); + return false; + case WAIT_FAILED: + fprintf(stderr, "Mutex held by thread ID 0x%x failed (%d locks)\n", m_heldBy, iLockCount ); + return false; + } #else /* timespec abstime; abstime.tv_sec = 0; @@ -172,25 +190,30 @@ // } #endif // printf("Locked %s\n", m_name.c_str()); + m_heldBy = MercuryThread::Current(); ++iLockCount; - return r; +// return r; + return true; } -int MercuryMutex::UnLock( ) +bool MercuryMutex::UnLock( ) { // printf("Unlocked %s\n", m_name.c_str()); --iLockCount; + if (iLockCount==0) m_heldBy = 0; #if defined( WIN32 ) - return ReleaseMutex( m_mutex ); + bool r = ReleaseMutex( m_mutex )!=0; + if (!r) fprintf(stderr, "Failed to release mutex %s, error %d!!\n", m_name.c_str(), GetLastError()); + return r; #else pthread_mutex_unlock( &m_mutex ); - return 0; + return true; #endif } int MercuryMutex::Open( ) { - iLockCount++; + ++iLockCount; #if defined( WIN32 ) SECURITY_ATTRIBUTES *p = ( SECURITY_ATTRIBUTES* ) malloc( sizeof( SECURITY_ATTRIBUTES ) ); p->nLength = sizeof( SECURITY_ATTRIBUTES ); Modified: Mercury2/src/MercuryThreads.h =================================================================== --- Mercury2/src/MercuryThreads.h 2010-01-14 21:30:42 UTC (rev 659) +++ Mercury2/src/MercuryThreads.h 2010-01-15 18:11:44 UTC (rev 660) @@ -22,6 +22,8 @@ class StartThreadData; #endif +#include <stdio.h> + ///Thread object class MercuryThread { @@ -74,13 +76,22 @@ MercuryMutex( const MString &name ); ~MercuryMutex(); + /** These functions are dangerous because you need to be VERY careful to cover all + unlock scenarios including errors and exceptions. + Recommend using AutoMutexLock on the stack to avoid problems **/ + ///Wait for a mutex to unlock (0xFFFFFF is infinate on windows) - int Wait( long lMilliseconds = 0xFFFFFF ); + bool DangerousWait( long lMilliseconds = 0xFFFFFF ) { return Wait(lMilliseconds); } //return false on error + /// Unlock a mutex for the next thing waiting in line. + bool DangerousUnLock( ) { return UnLock( ); } //return false on error - ///Unlock a mutex for the next thing waiting in line. - int UnLock( ); + inline unsigned long HeldBy() const { return m_heldBy; } + void SetName(const MString& name); private: + friend class AutoMutexLock; + bool Wait( long lMilliseconds = 0xFFFFFF ); //return false on error + bool UnLock( ); //return false on error ///Start up a mutex. You need to do this as well as UnLock() afterwards when in a constructor. int Open( ); @@ -91,6 +102,7 @@ MString m_name; int iLockCount; + unsigned long m_heldBy; #if defined( WIN32 ) void * m_mutex; #else @@ -109,8 +121,12 @@ AutoMutexLock( MercuryMutex& mutex) :m_mutex(&mutex) { - m_mutex->Wait(); + //loop until we get a lock but use a timeout so we are warned + //of a held mutex indicating a possible deadlock + bool l = m_mutex->Wait(1000); + while (!l) m_mutex->Wait(1000); } + ~AutoMutexLock() { m_mutex->UnLock(); Modified: Mercury2/src/ModuleManager.cpp =================================================================== --- Mercury2/src/ModuleManager.cpp 2010-01-14 21:30:42 UTC (rev 659) +++ Mercury2/src/ModuleManager.cpp 2010-01-15 18:11:44 UTC (rev 660) @@ -37,7 +37,8 @@ void ModuleManager::InitializeAllModules() { - m_mHandleMutex.Wait(); + AutoMutexLock lock(m_mHandleMutex); + XMLDocument* doc = XMLDocument::Load("modules.xml"); XMLNode r = doc->GetRootNode(); for (XMLNode child = r.Child(); child.IsValid(); child = child.NextNode()) @@ -60,7 +61,7 @@ m_hClassMFunction[child.Attribute( "class" )] = LoadFunct; LoadModule( ModuleName, LoadFunct ); } - m_mHandleMutex.UnLock(); + delete doc; } @@ -72,26 +73,27 @@ void * ModuleManager::LoadModule( const MString & ModuleName, const MString & LoadFunction ) { - m_mHandleMutex.Wait(); + { + //scope mutex lock + AutoMutexLock lock(m_mHandleMutex); - if( m_hAllHandles[ModuleName] ) UnloadModule( ModuleName ); + if( m_hAllHandles[ModuleName] ) UnloadModule( ModuleName ); - void * v = dlopen( ModuleName.c_str(), RTLD_NOW | RTLD_GLOBAL ); - m_hAllHandles[ModuleName] = v; + void * v = dlopen( ModuleName.c_str(), RTLD_NOW | RTLD_GLOBAL ); + m_hAllHandles[ModuleName] = v; - if( !m_hAllHandles[ModuleName] ) - { + if( !m_hAllHandles[ModuleName] ) + { #ifdef WIN32 - fprintf( stderr, "Error opening: %s\n", ModuleName.c_str() ); + fprintf( stderr, "Error opening: %s\n", ModuleName.c_str() ); #else - fprintf( stderr, "Error opening: %s (%s)\n", ModuleName.c_str(), dlerror() ); + fprintf( stderr, "Error opening: %s (%s)\n", ModuleName.c_str(), dlerror() ); #endif - return false; + return false; + } } - m_mHandleMutex.UnLock(); - //If no load funct, just exit early. if( LoadFunction == "" ) return 0; This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site. |