|
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.
|