aligned_stack: sync with trunk; some bugfixes to the stack prep on entry; and re-enabled the esp/ebp integrity assertions.

git-svn-id: http://pcsx2.googlecode.com/svn/branches/aligned_stack@2032 96395faa-99c1-11dd-bbfe-3dabce05a288

3rdparty/wxWidgets/src/msw/app.cpp

@@ -690,12 +690,6 @@ bool wxApp::Yield(bool onlyIfNeeded)
     // MT-FIXME
     static bool s_inYield = false;
 
-#if wxUSE_LOG
-    // disable log flushing from here because a call to wxYield() shouldn't
-    // normally result in message boxes popping up &c
-    wxLog::Suspend();
-#endif // wxUSE_LOG
-
     if ( s_inYield )
     {
         if ( !onlyIfNeeded )
@@ -706,6 +700,12 @@ bool wxApp::Yield(bool onlyIfNeeded)
         return false;
     }
 
+#if wxUSE_LOG
+	// disable log flushing from here because a call to wxYield() shouldn't
+	// normally result in message boxes popping up &c
+	wxLog::Suspend();
+#endif // wxUSE_LOG
+
     s_inYield = true;
 
     // we don't want to process WM_QUIT from here - it should be processed in

common/include/Utilities/Threading.h

@@ -268,6 +268,9 @@ namespace Threading
 		virtual void Block();
 		virtual void RethrowException() const;
 
+		void WaitOnSelf( Semaphore& mutex );
+		void WaitOnSelf( MutexLock& mutex );
+
 		bool IsRunning() const;
 		bool IsSelf() const;
 		wxString GetName() const;
@@ -276,10 +279,15 @@ namespace Threading
 		// Extending classes should always implement your own OnStart(), which is called by
 		// Start() once necessary locks have been obtained.  Do not override Start() directly
 		// unless you're really sure that's what you need to do. ;)
-		virtual void OnStart()=0;
+		virtual void OnStart();
-		virtual void OnCleanupInThread()=0;
+		
+		virtual void OnStartInThread();
 
-		// Implemented by derived class to handle threading actions!
+		// This is called when the thread has been canceled or exits normally.  The PersistentThread
+		// automatically binds it to the pthread cleanup routines as soon as the thread starts.
+		virtual void OnCleanupInThread();
+
+		// Implemented by derived class to perform actual threaded task!
 		virtual void ExecuteTaskInThread()=0;
 
 		void TestCancel();
@@ -444,23 +452,23 @@ namespace Threading
 	// Our fundamental interlocking functions.  All other useful interlocks can be derived
 	// from these little beasties!
 
-	extern void AtomicExchange( volatile u32& Target, u32 value );
+	extern u32 AtomicExchange( volatile u32& Target, u32 value );
-	extern void AtomicExchangeAdd( volatile u32& Target, u32 value );
+	extern u32 AtomicExchangeAdd( volatile u32& Target, u32 value );
-	extern void AtomicIncrement( volatile u32& Target );
+	extern u32 AtomicIncrement( volatile u32& Target );
-	extern void AtomicDecrement( volatile u32& Target );
+	extern u32 AtomicDecrement( volatile u32& Target );
-	extern void AtomicExchange( volatile s32& Target, s32 value );
+	extern s32 AtomicExchange( volatile s32& Target, s32 value );
-	extern void AtomicExchangeAdd( volatile s32& Target, u32 value );
+	extern s32 AtomicExchangeAdd( volatile s32& Target, u32 value );
-	extern void AtomicIncrement( volatile s32& Target );
+	extern s32 AtomicIncrement( volatile s32& Target );
-	extern void AtomicDecrement( volatile s32& Target );
+	extern s32 AtomicDecrement( volatile s32& Target );
 
-	extern void _AtomicExchangePointer( const void ** target, const void* value );
+	extern void* _AtomicExchangePointer( void * volatile * const target, void* const value );
-	extern void _AtomicCompareExchangePointer( const void ** target, const void* value, const void* comparand );
+	extern void* _AtomicCompareExchangePointer( void * volatile * const target, void* const value, void* const comparand );
 
 	#define AtomicExchangePointer( target, value ) \
-		_AtomicExchangePointer( (const void**)(&target), (const void*)(value) )
+		_InterlockedExchangePointer( &target, value )
 
 	#define AtomicCompareExchangePointer( target, value, comparand ) \
-		_AtomicCompareExchangePointer( (const void**)(&target), (const void*)(value), (const void*)(comparand) )
+		_InterlockedCompareExchangePointer( &target, value, comparand )
 
 }
 

common/src/Utilities/Exceptions.cpp

@@ -85,7 +85,7 @@ Exception::BaseException::~BaseException() throw() {}
 void Exception::BaseException::InitBaseEx( const wxString& msg_eng, const wxString& msg_xlt )
 {
 	m_message_diag = msg_eng;
-	m_message_user = msg_xlt;
+	m_message_user = msg_xlt.IsEmpty() ? msg_eng : msg_xlt;
 
 	// Linux/GCC exception handling is still suspect (this is likely to do with GCC more
 	// than linux), and fails to propagate exceptions up the stack from EErec code.  This

common/src/Utilities/Mutex.cpp

@@ -150,9 +150,8 @@ void Threading::MutexLock::Lock()
 	}
 	else
 	{
-		do {
+		while( !LockRaw(def_yieldgui_interval) )
 			wxTheApp->Yield( true );
-		} while( !LockRaw(def_yieldgui_interval) );
 	}
 #else
 	LockRaw();
@@ -183,8 +182,8 @@ bool Threading::MutexLock::Lock( const wxTimeSpan& timeout )
 		wxTimeSpan countdown( (timeout) );
 
 		do {
-			wxTheApp->Yield(true);
 			if( LockRaw( def_yieldgui_interval ) ) break;
+			wxTheApp->Yield(true);
 			countdown -= def_yieldgui_interval;
 		} while( countdown.GetMilliseconds() > 0 );
 

common/src/Utilities/Semaphore.cpp

@@ -68,7 +68,7 @@ bool Threading::Semaphore::WaitRaw( const wxTimeSpan& timeout )
 {
 	wxDateTime megafail( wxDateTime::UNow() + timeout );
 	const timespec fail = { megafail.GetTicks(), megafail.GetMillisecond() * 1000000 };
-	return sem_timedwait( &m_sema, &fail ) != -1;
+	return sem_timedwait( &m_sema, &fail ) == 0;
 }
 
 
@@ -94,9 +94,8 @@ void Threading::Semaphore::Wait()
 	}
 	else
 	{
-		do {
+		while( !WaitRaw( def_yieldgui_interval ) )
 			wxTheApp->Yield( true );
-		} while( !WaitRaw( def_yieldgui_interval ) );
 	}
 #else
 	WaitRaw();
@@ -136,8 +135,8 @@ bool Threading::Semaphore::Wait( const wxTimeSpan& timeout )
 		wxTimeSpan countdown( (timeout) );
 
 		do {
-			wxTheApp->Yield(true);
 			if( WaitRaw( def_yieldgui_interval ) ) break;
+			wxTheApp->Yield(true);
 			countdown -= def_yieldgui_interval;
 		} while( countdown.GetMilliseconds() > 0 );
 

common/src/Utilities/ThreadTools.cpp

@@ -43,13 +43,11 @@ bool Threading::_WaitGui_RecursionGuard( const char* guardname )
 	// In order to avoid deadlock we need to make sure we cut some time to handle messages.
 	// But this can result in recursive yield calls, which would crash the app.  Protect
 	// against them here and, if recursion is detected, perform a standard blocking wait.
-	//   (also, wx ignores message pumping on recursive Yields, so no point in allowing
-	//    more then one recursion)
 
 	static int __Guard = 0;
 	RecursionGuard guard( __Guard );
 
-	if( guard.Counter >= 2 )
+	if( guard.IsReentrant() )
 	{
 		Console.WriteLn( "(Thread Log) Possible yield recursion detected in %s; performing blocking wait.", guardname );
 		return true;
@@ -130,7 +128,7 @@ void Threading::PersistentThread::FrankenMutex( MutexLock& mutex )
 }
 
 // Main entry point for starting or e-starting a persistent thread.  This function performs necessary
-// locks and checks for avoiding race conditions, and then calls OnStart() immeediately before
+// locks and checks for avoiding race conditions, and then calls OnStart() immediately before
 // the actual thread creation.  Extending classes should generally not override Start(), and should
 // instead override DoPrepStart instead.
 //
@@ -142,9 +140,6 @@ void Threading::PersistentThread::Start()
 	if( m_running ) return;
 
 	Detach();		// clean up previous thread handle, if one exists.
-
-	FrankenMutex( m_lock_InThread );
-
 	OnStart();
 
 	if( pthread_create( &m_thread, NULL, _internal_callback, this ) != 0 )
@@ -236,6 +231,57 @@ void Threading::PersistentThread::RethrowException() const
 	m_except->Rethrow();
 }
 
+// This helper function is a deadlock-safe method of waiting on a semaphore in a PersistentThread.  If the
+// thread is terminated or canceled by another thread or a nested action prior to the semaphore being
+// posted, this function will detect that and throw a ThreadTimedOut exception.
+//
+// Note: Use of this function only applies to semaphores which are posted by the worker thread.  Calling
+// this function from the context of the thread itself is an error, and a dev assertion will be generated.
+//
+// Exceptions:
+//   ThreadTimedOut
+//
+void Threading::PersistentThread::WaitOnSelf( Semaphore& sem )
+{
+	if( !pxAssertDev( !IsSelf(), "WaitOnSelf called from inside the thread (invalid operation!)" ) ) return;
+
+	while( true )
+	{
+		if( sem.Wait( wxTimeSpan(0, 0, 0, 250) ) ) return;
+		if( !m_running )
+		{
+			wxString msg( m_name + L": thread was terminated while another thread was waiting on a semaphore." );
+			throw Exception::ThreadTimedOut( msg, msg );
+		}
+	}
+}
+
+// This helper function is a deadlock-safe method of waiting on a mutex in a PersistentThread.  If the
+// thread is terminated or canceled by another thread or a nested action prior to the mutex being
+// unlocked, this function will detect that and throw a ThreadTimedOut exception.
+//
+// Note: Use of this function only applies to semaphores which are posted by the worker thread.  Calling
+// this function from the context of the thread itself is an error, and a dev assertion will be generated.
+//
+// Exceptions:
+//   ThreadTimedOut
+//
+void Threading::PersistentThread::WaitOnSelf( MutexLock& mutex )
+{
+	if( !pxAssertDev( !IsSelf(), "WaitOnSelf called from inside the thread (invalid operation!)" ) ) return;
+
+	while( true )
+	{
+		if( mutex.Wait( wxTimeSpan(0, 0, 0, 250) ) ) return;
+		if( !m_running )
+		{
+			wxString msg( m_name + L": thread was terminated while another thread was waiting on a mutex." );
+			throw Exception::ThreadTimedOut( msg, msg );
+		}
+	}
+}
+
+
 // Inserts a thread cancellation point.  If the thread has received a cancel request, this
 // function will throw an SEH exception designed to exit the thread (so make sure to use C++
 // object encapsulation for anything that could leak resources, to ensure object unwinding
@@ -321,7 +367,6 @@ void Threading::PersistentThread::_ThreadCleanup()
 
 	_try_virtual_invoke( &PersistentThread::OnCleanupInThread );
 
-	m_running = false;
 	m_lock_InThread.Unlock();
 }
 
@@ -330,16 +375,36 @@ wxString Threading::PersistentThread::GetName() const
 	return m_name;
 }
 
+// This override is called by PeristentThread when the thread is first created, prior to
+// calling ExecuteTaskInThread.  This is useful primarily for "base" classes that extend
+// from PersistentThread, giving them the ability to bind startup code to all threads that
+// derive from them.  (the alternative would have been to make ExecuteTaskInThread a
+// private member, and provide a new Task executor by a different name).
+void Threading::PersistentThread::OnStartInThread()
+{
+	m_running = true;
+}
+
 void Threading::PersistentThread::_internal_execute()
 {
 	m_lock_InThread.Lock();
-	m_running = true;
 	_DoSetThreadName( m_name );
+
+	OnStartInThread();
+
 	_try_virtual_invoke( &PersistentThread::ExecuteTaskInThread );
 }
 
-void Threading::PersistentThread::OnStart() {}
+void Threading::PersistentThread::OnStart()
-void Threading::PersistentThread::OnCleanupInThread() {}
+{
+	FrankenMutex( m_lock_InThread );
+	m_sem_event.Reset();
+}
+
+void Threading::PersistentThread::OnCleanupInThread()
+{
+	m_running = false;
+}
 
 // passed into pthread_create, and is used to dispatch the thread's object oriented
 // callback function
@@ -496,52 +561,42 @@ void Threading::WaitEvent::Wait()
 // --------------------------------------------------------------------------------------
 // define some overloads for InterlockedExchanges for commonly used types, like u32 and s32.
 
-__forceinline void Threading::AtomicExchange( volatile u32& Target, u32 value )
+__forceinline u32 Threading::AtomicExchange( volatile u32& Target, u32 value )
 {
-	_InterlockedExchange( (volatile long*)&Target, value );
+	return _InterlockedExchange( (volatile long*)&Target, value );
 }
 
-__forceinline void Threading::AtomicExchangeAdd( volatile u32& Target, u32 value )
+__forceinline u32 Threading::AtomicExchangeAdd( volatile u32& Target, u32 value )
 {
-	_InterlockedExchangeAdd( (volatile long*)&Target, value );
+	return _InterlockedExchangeAdd( (volatile long*)&Target, value );
 }
 
-__forceinline void Threading::AtomicIncrement( volatile u32& Target )
+__forceinline u32 Threading::AtomicIncrement( volatile u32& Target )
 {
-	_InterlockedExchangeAdd( (volatile long*)&Target, 1 );
+	return _InterlockedExchangeAdd( (volatile long*)&Target, 1 );
 }
 
-__forceinline void Threading::AtomicDecrement( volatile u32& Target )
+__forceinline u32 Threading::AtomicDecrement( volatile u32& Target )
 {
-	_InterlockedExchangeAdd( (volatile long*)&Target, -1 );
+	return _InterlockedExchangeAdd( (volatile long*)&Target, -1 );
 }
 
-__forceinline void Threading::AtomicExchange( volatile s32& Target, s32 value )
+__forceinline s32 Threading::AtomicExchange( volatile s32& Target, s32 value )
 {
-	_InterlockedExchange( (volatile long*)&Target, value );
+	return _InterlockedExchange( (volatile long*)&Target, value );
 }
 
-__forceinline void Threading::AtomicExchangeAdd( volatile s32& Target, u32 value )
+__forceinline s32 Threading::AtomicExchangeAdd( volatile s32& Target, u32 value )
 {
-	_InterlockedExchangeAdd( (volatile long*)&Target, value );
+	return _InterlockedExchangeAdd( (volatile long*)&Target, value );
 }
 
-__forceinline void Threading::AtomicIncrement( volatile s32& Target )
+__forceinline s32 Threading::AtomicIncrement( volatile s32& Target )
 {
-	_InterlockedExchangeAdd( (volatile long*)&Target, 1 );
+	return _InterlockedExchangeAdd( (volatile long*)&Target, 1 );
 }
 
-__forceinline void Threading::AtomicDecrement( volatile s32& Target )
+__forceinline s32 Threading::AtomicDecrement( volatile s32& Target )
 {
-	_InterlockedExchangeAdd( (volatile long*)&Target, -1 );
+	return _InterlockedExchangeAdd( (volatile long*)&Target, -1 );
-}
-
-__forceinline void Threading::_AtomicExchangePointer( const void ** target, const void* value )
-{
-	_InterlockedExchange( (volatile long*)target, (long)value );
-}
-
-__forceinline void Threading::_AtomicCompareExchangePointer( const void ** target, const void* value, const void* comparand )
-{
-	_InterlockedCompareExchange( (volatile long*)target, (long)value, (long)comparand );
 }

pcsx2/GS.h

@@ -108,10 +108,6 @@ protected:
 	// run very fast and have little or no ringbuffer overhead (typically opening menus)
 	volatile s32 m_QueuedFrames;
 
-	// Protection lock for the frame queue counter -- needed because we can't safely
-	// AtomicExchange from two threads.
-	MutexLock m_lock_FrameQueueCounter;
-
 	// These vars maintain instance data for sending Data Packets.
 	// Only one data packet can be constructed and uploaded at a time.
 

pcsx2/MTGS.cpp

@@ -72,7 +72,7 @@ struct MTGS_BufferedData
 
 	u128& operator[]( uint idx )
 	{
-		jASSUME( idx < RingBufferSize );
+		pxAssert( idx < RingBufferSize );
 		return m_Ring[idx];
 	}
 };
@@ -99,7 +99,6 @@ mtgsThreadObject::mtgsThreadObject() :
 ,	m_CopyDataTally( 0 )
 ,	m_RingBufferIsBusy( false )
 ,	m_QueuedFrames( 0 )
-,	m_lock_FrameQueueCounter()
 ,	m_packet_size( 0 )
 ,	m_packet_ringpos( 0 )
 
@@ -146,7 +145,7 @@ void mtgsThreadObject::ResetGS()
 	//  * Signal a reset.
 	//  * clear the path and byRegs structs (used by GIFtagDummy)
 
-	AtomicExchange( m_RingPos, m_WritePos );
+	m_RingPos = m_WritePos;
 
 	MTGS_LOG( "MTGS: Sending Reset..." );
 	SendSimplePacket( GS_RINGTYPE_RESET, 0, 0, 0 );
@@ -171,10 +170,8 @@ void mtgsThreadObject::PostVsyncEnd( bool updategs )
 		SpinWait();
 	}
 
-	m_lock_FrameQueueCounter.Lock();
+	AtomicIncrement( m_QueuedFrames );
-	m_QueuedFrames++;
 	//Console.Status( " >> Frame Added!" );
-	m_lock_FrameQueueCounter.Unlock();
 
 	SendSimplePacket( GS_RINGTYPE_VSYNC,
 		(*(u32*)(PS2MEM_GS+0x1000)&0x2000), updategs, 0);
@@ -237,9 +234,6 @@ void mtgsThreadObject::OpenPlugin()
 
 void mtgsThreadObject::ExecuteTaskInThread()
 {
-	// Required by the underlying SysThreadBase class (is unlocked on exit)
-	m_RunningLock.Lock();
-
 #ifdef RINGBUF_DEBUG_STACK
 	PacketTagType prevCmd;
 #endif
@@ -247,7 +241,7 @@ void mtgsThreadObject::ExecuteTaskInThread()
 	pthread_cleanup_push( _clean_close_gs, this );
 	while( true )
 	{
-		m_sem_event.WaitRaw();		// ... because this does a cancel test itself..
+		m_sem_event.WaitRaw();			// ... because this does a cancel test itself..
 		StateCheckInThread( false );	// false disables cancel test here!
 
 		m_RingBufferIsBusy = true;
@@ -279,11 +273,10 @@ void mtgsThreadObject::ExecuteTaskInThread()
 			switch( tag.command )
 			{
 				case GS_RINGTYPE_RESTART:
-					AtomicExchange(m_RingPos, 0);
+					m_RingPos = 0;
 
 					// stall for a bit to let the MainThread have time to update the g_pGSWritePos.
-					m_lock_RingRestart.Lock();
+					m_lock_RingRestart.Wait();
-					m_lock_RingRestart.Unlock();
 
 					StateCheckInThread( false );		// disable cancel since the above locks are cancelable already
 				continue;
@@ -323,11 +316,9 @@ void mtgsThreadObject::ExecuteTaskInThread()
 					GSvsync(tag.data[0]);
 					gsFrameSkip( !tag.data[1] );
 
-					m_lock_FrameQueueCounter.Lock();
+					int framecnt = AtomicDecrement( m_QueuedFrames );
-					AtomicDecrement( m_QueuedFrames );
+					pxAssertDev( framecnt >= 0, "Frame queue sync count failure." );
-					jASSUME( m_QueuedFrames >= 0 );
 					//Console.Status( " << Frame Removed!" );
-					m_lock_FrameQueueCounter.Unlock();
 
 					if( PADupdate != NULL )
 					{
@@ -390,7 +381,7 @@ void mtgsThreadObject::ExecuteTaskInThread()
 				case GS_RINGTYPE_MODECHANGE:
 					_gs_ChangeTimings( tag.data[0], tag.data[1] );
 				break;
-				
+
 				case GS_RINGTYPE_CRC:
 					GSsetGameCRC( tag.data[0], 0 );
 				break;
@@ -398,7 +389,7 @@ void mtgsThreadObject::ExecuteTaskInThread()
 				case GS_RINGTYPE_STARTTIME:
 					m_iSlowStart += tag.data[0];
 				break;
-				
+
 #ifdef PCSX2_DEVBUILD
 				default:
 					Console.Error("GSThreadProc, bad packet (%x) at m_RingPos: %x, m_WritePos: %x", tag.command, m_RingPos, m_WritePos);
@@ -414,7 +405,7 @@ void mtgsThreadObject::ExecuteTaskInThread()
 			uint newringpos = m_RingPos + ringposinc;
 			pxAssert( newringpos <= RingBufferSize );
 			newringpos &= RingBufferMask;
-			AtomicExchange( m_RingPos, newringpos );
+			m_RingPos = newringpos;
 		}
 		m_RingBufferIsBusy = false;
 	}
@@ -475,10 +466,10 @@ u8* mtgsThreadObject::GetDataPacketPtr() const
 void mtgsThreadObject::SendDataPacket()
 {
 	// make sure a previous copy block has been started somewhere.
-	jASSUME( m_packet_size != 0 );
+	pxAssert( m_packet_size != 0 );
 
 	uint temp = m_packet_ringpos + m_packet_size;
-	jASSUME( temp <= RingBufferSize );
+	pxAssert( temp <= RingBufferSize );
 	temp &= RingBufferMask;
 
 	if( IsDebugBuild )
@@ -491,17 +482,17 @@ void mtgsThreadObject::SendDataPacket()
 				// The writepos should never leapfrog the readpos
 				// since that indicates a bad write.
 				if( m_packet_ringpos < readpos )
-					assert( temp < readpos );
+					pxAssert( temp < readpos );
 			}
 
 			// Updating the writepos should never make it equal the readpos, since
 			// that would stop the buffer prematurely (and indicates bad code in the
 			// ringbuffer manager)
-			assert( readpos != temp );
+			pxAssert( readpos != temp );
 		}
 	}
 
-	AtomicExchange( m_WritePos, temp );
+	m_WritePos = temp;
 
 	m_packet_size = 0;
 
@@ -606,11 +597,11 @@ int mtgsThreadObject::PrepDataPacket( GIF_PATH pathidx, const u8* srcdata, u32 s
 	uint writepos = m_WritePos;
 
 	// Checks if a previous copy was started without an accompanying call to GSRINGBUF_DONECOPY
-	jASSUME( m_packet_size == 0 );
+	pxAssert( m_packet_size == 0 );
 
 	// Sanity checks! (within the confines of our ringbuffer please!)
-	jASSUME( size < RingBufferSize );
+	pxAssert( size < RingBufferSize );
-	jASSUME( writepos < RingBufferSize );
+	pxAssert( writepos < RingBufferSize );
 
 	m_packet_size = GIFPath_ParseTag(pathidx, srcdata, size);
 	size		  = m_packet_size + 1; // takes into account our command qword.
@@ -667,8 +658,7 @@ int mtgsThreadObject::PrepDataPacket( GIF_PATH pathidx, const u8* srcdata, u32 s
 
 		m_lock_RingRestart.Lock();
 		SendSimplePacket( GS_RINGTYPE_RESTART, 0, 0, 0 );
-		writepos = 0;
+		m_WritePos = writepos = 0;
-		AtomicExchange( m_WritePos, writepos );
 		m_lock_RingRestart.Unlock();
 		SetEvent();
 
@@ -734,7 +724,7 @@ __forceinline uint mtgsThreadObject::_PrepForSimplePacket()
 #endif
 
 	uint future_writepos = m_WritePos+1;
-	jASSUME( future_writepos <= RingBufferSize );
+	pxAssert( future_writepos <= RingBufferSize );
 
     future_writepos &= RingBufferMask;
 
@@ -753,8 +743,8 @@ __forceinline uint mtgsThreadObject::_PrepForSimplePacket()
 
 __forceinline void mtgsThreadObject::_FinishSimplePacket( uint future_writepos )
 {
-	assert( future_writepos != volatize(m_RingPos) );
+	pxAssert( future_writepos != volatize(m_RingPos) );
-	AtomicExchange( m_WritePos, future_writepos );
+	m_WritePos = future_writepos;
 }
 
 void mtgsThreadObject::SendSimplePacket( GS_RINGTYPE type, int data0, int data1, int data2 )
@@ -795,7 +785,28 @@ void mtgsThreadObject::WaitForOpen()
 {
 	if( gsIsOpened ) return;
 	Resume();
-	m_sem_OpenDone.Wait();
+
+	// Two-phase timeout on MTGS opening, so that possible errors are handled
+	// in a timely fashion.  We check for errors after 2 seconds, and then give it
+	// another 4 seconds if no errors occurred (this might seem long, but sometimes a
+	// GS plugin can be very stubborned, especially in debug mode builds).
+
+	if( !m_sem_OpenDone.Wait( wxTimeSpan(0, 0, 2, 0) ) )
+	{
+		RethrowException();
+
+		if( !m_sem_OpenDone.Wait( wxTimeSpan(0, 0, 4, 0) ) )
+		{
+			RethrowException();
+			
+			// Not opened yet, and no exceptions.  Weird?  You decide!
+			// TODO : implement a user confirmation to cancel the action and exit the
+			//   emulator forcefully, or to continue waiting on the GS.
+			
+			throw Exception::PluginOpenError( PluginId_GS, "The MTGS thread has become unresponsive while waiting for the GS plugin to open." );
+		}
+	}
+
 	mtgsThread.RethrowException();
 }
 
@@ -803,7 +814,7 @@ void mtgsThreadObject::Freeze( int mode, MTGS_FreezeData& data )
 {
 	if( mode == FREEZE_LOAD )
 	{
-		AtomicExchange( m_RingPos, m_WritePos );
+		WaitGS();
 		SendPointerPacket( GS_RINGTYPE_FREEZE, mode, &data );
 		SetEvent();
 		Resume();

pcsx2/RecoverySystem.cpp

@@ -74,6 +74,8 @@ protected:
 	{
 		if( !state_buffer_lock.TryLock() )
 			throw Exception::CancelEvent( m_name + L"request ignored: state copy buffer is already locked!" );
+			
+		_parent::OnStart();
 	}
 
 	void SendFinishEvent( int type )
@@ -104,7 +106,6 @@ protected:
 	void OnStart()
 	{
 		_parent::OnStart();
-
 		++sys_resume_lock;
 		CoreThread.Pause();
 	}

pcsx2/System/SysThreads.cpp

@@ -1,6 +1,6 @@
 /*  PCSX2 - PS2 Emulator for PCs
  *  Copyright (C) 2002-2009  PCSX2 Dev Team
- * 
+ *
  *  PCSX2 is free software: you can redistribute it and/or modify it under the terms
  *  of the GNU Lesser General Public License as published by the Free Software Found-
  *  ation, either version 3 of the License, or (at your option) any later version.
@@ -46,6 +46,25 @@ void SysThreadBase::Start()
 {
 	_parent::Start();
 	m_ExecMode = ExecMode_Closing;
+
+	Sleep( 1 );
+
+	if( !m_ResumeEvent.WaitRaw( wxTimeSpan(0, 0, 1, 500) ) )
+	{
+		RethrowException();
+		if( pxAssertDev( m_ExecMode == ExecMode_Closing, "Unexpected thread status during SysThread startup." ) )
+		{
+			throw Exception::ThreadCreationError( 
+				wxsFormat( L"Timeout occurred while attempting to start the %s thread.", m_name.c_str() ),
+				wxEmptyString
+			);
+		}
+	}
+
+	pxAssertDev( (m_ExecMode == ExecMode_Closing) || (m_ExecMode == ExecMode_Closed),
+		"Unexpected thread status during SysThread startup."
+	);
+
 	m_sem_event.Post();
 }
 
@@ -55,7 +74,7 @@ void SysThreadBase::OnStart()
 	if( !pxAssertDev( m_ExecMode == ExecMode_NoThreadYet, "SysSustainableThread:Start(): Invalid execution mode" ) ) return;
 
 	m_ResumeEvent.Reset();
-	//m_SuspendEvent.Reset();
+	FrankenMutex( m_ExecModeMutex );
 	FrankenMutex( m_RunningLock );
 
 	_parent::OnStart();
@@ -83,7 +102,7 @@ void SysThreadBase::OnStart()
 bool SysThreadBase::Suspend( bool isBlocking )
 {
 	if( IsSelf() || !IsRunning() ) return false;
-	
+
 	// shortcut ExecMode check to avoid deadlocking on redundant calls to Suspend issued
 	// from Resume or OnResumeReady code.
 	if( m_ExecMode == ExecMode_Closed ) return false;
@@ -152,7 +171,7 @@ bool SysThreadBase::Pause()
 // Resumes the core execution state, or does nothing is the core is already running.  If
 // settings were changed, resets will be performed as needed and emulation state resumed from
 // memory savestates.
-// 
+//
 // Note that this is considered a non-blocking action.  Most times the state is safely resumed
 // on return, but in the case of re-entrant or nested message handling the function may return
 // before the thread has resumed.  If you need explicit behavior tied to the completion of the
@@ -171,20 +190,27 @@ void SysThreadBase::Resume()
 
 	ScopedLock locker( m_ExecModeMutex );
 
-	// Recursion guard is needed because of the non-blocking Wait if the state
+	// Implementation Note:
-	// is Suspending/Closing.  Processed events could recurse into Resume, and we'll
+	// The entire state coming out of a Wait is indeterminate because of user input
-	// want to silently ignore them.
+	// and pending messages being handled.  So after each call we do some seemingly redundant
-	
+	// sanity checks against m_ExecMode/m_Running status, and if something doesn't feel
-	//RecursionGuard guard( m_resume_guard );
+	// right, we should abort.
-	//if( guard.IsReentrant() ) return;
 
 	switch( m_ExecMode )
 	{
 		case ExecMode_Opened: return;
 
 		case ExecMode_NoThreadYet:
+		{
+			static int __Guard = 0;
+			RecursionGuard guard( __Guard );
+			if( guard.IsReentrant() ) return;
+
 			Start();
-			m_ExecMode = ExecMode_Closing;
+			if( !m_running || (m_ExecMode == ExecMode_NoThreadYet) )
+				throw Exception::ThreadCreationError();
+			if( m_ExecMode == ExecMode_Opened ) return;
+		}
 		// fall through...
 
 		case ExecMode_Closing:
@@ -192,14 +218,8 @@ void SysThreadBase::Resume()
 			// we need to make sure and wait for the emuThread to enter a fully suspended
 			// state before continuing...
 
-			//locker.Unlock();		// no deadlocks please, thanks. :)
 			m_RunningLock.Wait();
-			//locker.Lock();
+			if( !m_running ) return;
-			
-			// The entire state coming out of a Wait is indeterminate because of user input
-			// and pending messages being handled.  If something doesn't feel right, we should
-			// abort.
-			
 			if( (m_ExecMode != ExecMode_Closed) && (m_ExecMode != ExecMode_Paused) ) return;
 			if( g_plugins == NULL ) return;
 		break;
@@ -219,6 +239,13 @@ void SysThreadBase::Resume()
 //    (Called from the context of this thread only)
 // --------------------------------------------------------------------------------------
 
+void SysThreadBase::OnStartInThread()
+{
+	m_RunningLock.Lock();
+	_parent::OnStartInThread();
+	m_ResumeEvent.Post();
+}
+
 void SysThreadBase::OnCleanupInThread()
 {
 	m_ExecMode = ExecMode_NoThreadYet;
@@ -228,16 +255,6 @@ void SysThreadBase::OnCleanupInThread()
 
 void SysThreadBase::StateCheckInThread( bool isCancelable )
 {
-	// Shortcut for the common case, to avoid unnecessary Mutex locks:
-	/*if( m_ExecMode == ExecMode_Opened )
-	{
-		if( isCancelable ) TestCancel();
-		return;
-	}
-
-	// Oh, seems we need a full lock, because something special is happening!
-	ScopedLock locker( m_ExecModeMutex );*/
-
 	switch( m_ExecMode )
 	{
 
@@ -266,7 +283,6 @@ void SysThreadBase::StateCheckInThread( bool isCancelable )
 		// fallthrough...
 
 		case ExecMode_Paused:
-			//locker.Unlock();
 			while( m_ExecMode == ExecMode_Paused )
 				m_ResumeEvent.WaitRaw();
 
@@ -280,18 +296,17 @@ void SysThreadBase::StateCheckInThread( bool isCancelable )
 			OnSuspendInThread();
 			m_ExecMode = ExecMode_Closed;
 			m_RunningLock.Unlock();
-		} 
+		}
 		// fallthrough...
-		
+
 		case ExecMode_Closed:
-			//locker.Unlock();
 			while( m_ExecMode == ExecMode_Closed )
 				m_ResumeEvent.WaitRaw();
 
 			m_RunningLock.Lock();
 			OnResumeInThread( true );
 		break;
-		
+
 		jNO_DEFAULT;
 	}
 }
@@ -366,7 +381,7 @@ void SysCoreThread::ApplySettings( const Pcsx2Config& src )
 
 	m_resetRecompilers		= ( src.Cpu != EmuConfig.Cpu ) || ( src.Gamefixes != EmuConfig.Gamefixes ) || ( src.Speedhacks != EmuConfig.Speedhacks );
 	m_resetProfilers		= (src.Profiler != EmuConfig.Profiler );
- 
+
 	const_cast<Pcsx2Config&>(EmuConfig) = src;
 
 	if( resumeWhenDone ) Resume();
@@ -385,7 +400,7 @@ SysCoreThread& SysCoreThread::Get()
 void SysCoreThread::CpuInitializeMess()
 {
 	if( m_hasValidState ) return;
-	
+
 	wxString elf_file;
 	if( EmuConfig.SkipBiosSplash )
 	{
@@ -445,7 +460,6 @@ void SysCoreThread::CpuExecute()
 
 void SysCoreThread::ExecuteTaskInThread()
 {
-	m_RunningLock.Lock();
 	tls_coreThread = this;
 
 	m_sem_event.WaitRaw();

pcsx2/System/SysThreads.h

@@ -1,6 +1,6 @@
 /*  PCSX2 - PS2 Emulator for PCs
  *  Copyright (C) 2002-2009  PCSX2 Dev Team
- * 
+ *
  *  PCSX2 is free software: you can redistribute it and/or modify it under the terms
  *  of the GNU Lesser General Public License as published by the Free Software Found-
  *  ation, either version 3 of the License, or (at your option) any later version.
@@ -44,7 +44,7 @@ protected:
 		// Thread has not been created yet.  Typically this is the same as IsRunning()
 		// returning FALSE.
 		ExecMode_NoThreadYet,
-		
+
 		// Close signal has been sent to the thread, but the thread's response is still
 		// pending (thread is busy/running).
 		ExecMode_Closing,
@@ -55,11 +55,11 @@ protected:
 
 		// Thread is active and running, with pluigns in an "open" state.
 		ExecMode_Opened,
-		
+
 		// Pause signal has been sent to the thread, but the thread's response is still
 		// pending (thread is busy/running).
 		ExecMode_Pausing,
-		
+
 		// Thread is safely paused, with plugins in an "open" state, and waiting for a
 		// resume/open signal.
 		ExecMode_Paused,
@@ -73,7 +73,7 @@ protected:
 
 	// Used to wake up the thread from sleeping when it's in a suspended state.
 	Semaphore				m_ResumeEvent;
-	
+
 	// Locked whenever the thread is not in a suspended state (either closed or paused).
 	// Issue a Wait against this mutex for performing actions that require the thread
 	// to be suspended.
@@ -92,7 +92,7 @@ public:
 	{
 		return m_ExecMode > ExecMode_Closed;
 	}
-	
+
 	bool IsClosed() const { return !IsOpen(); }
 
 	ExecutionMode GetExecutionMode() const { return m_ExecMode; }
@@ -101,18 +101,19 @@ public:
 	virtual bool Suspend( bool isBlocking = true );
 	virtual void Resume();
 	virtual bool Pause();
-	
+
 	virtual void StateCheckInThread( bool isCancelable = true );
-	virtual void OnCleanupInThread();
+
+protected:
+	virtual void OnStart();
 
 	// This function is called by Resume immediately prior to releasing the suspension of
 	// the core emulation thread.  You should overload this rather than Resume(), since
 	// Resume() has a lot of checks and balances to prevent re-entrance and race conditions.
 	virtual void OnResumeReady() {}
 
-	virtual void OnStart();
+	virtual void OnCleanupInThread();
-
+	virtual void OnStartInThread();
-protected:
 
 	// Used internally from Resume(), so let's make it private here.
 	virtual void Start();
@@ -123,7 +124,7 @@ protected:
 	// thread, requesting this thread suspend itself temporarily).  After this is called,
 	// the thread enters a waiting state on the m_ResumeEvent semaphore.
 	virtual void OnSuspendInThread()=0;
-	
+
 	// Extending classes should implement this, but should not call it.  The parent class
 	// handles invocation by the following guidelines: Called *in thread* from StateCheckInThread()
 	// prior to pausing the thread (ie, when Pause() has been called on a separate thread,
@@ -163,7 +164,7 @@ public:
 	virtual void ApplySettings( const Pcsx2Config& src );
 	virtual void OnResumeReady();
 	virtual void Reset();
-	
+
 	bool HasValidState()
 	{
 		return m_hasValidState;

pcsx2/gui/AppCoreThread.cpp

@@ -41,10 +41,13 @@ bool AppCoreThread::Suspend( bool isBlocking )
 	m_kevt.m_shiftDown		= false;
 	m_kevt.m_controlDown	= false;
 	m_kevt.m_altDown		= false;
-	
+
 	return retval;
 }
 
+
+static int resume_tries = 0;
+
 void AppCoreThread::Resume()
 {
 	// Thread control (suspend / resume) should only be performed from the main/gui thread.
@@ -55,6 +58,7 @@ void AppCoreThread::Resume()
 		Console.WriteLn( "SysResume: State is locked, ignoring Resume request!" );
 		return;
 	}
+	
 	_parent::Resume();
 
 	if( m_ExecMode != ExecMode_Opened )
@@ -66,8 +70,18 @@ void AppCoreThread::Resume()
 		evt.SetInt( CoreStatus_Suspended );
 		wxGetApp().AddPendingEvent( evt );
 
-		sApp.SysExecute();
+		if( (m_ExecMode != ExecMode_Closing) || (m_ExecMode != ExecMode_Pausing) )
+		{
+			if( ++resume_tries <= 2 )
+			{
+				sApp.SysExecute();
+			}
+			else
+				Console.Status( "SysResume: Multiple resume retries failed.  Giving up..." );
+		}
 	}
+	
+	resume_tries = 0;
 }
 
 void AppCoreThread::OnResumeReady()
@@ -83,7 +97,7 @@ void AppCoreThread::OnResumeReady()
 void AppCoreThread::OnResumeInThread( bool isSuspended )
 {
 	_parent::OnResumeInThread( isSuspended );
-	
+
 	wxCommandEvent evt( pxEVT_CoreThreadStatus );
 	evt.SetInt( CoreStatus_Resumed );
 	wxGetApp().AddPendingEvent( evt );

pcsx2/gui/AppInit.cpp

@@ -343,12 +343,23 @@ void Pcsx2App::CleanupMess()
 	// app is shutting down, so don't let the system resume for anything.  (sometimes there
 	// are pending Resume messages in the queue from previous user actions)
 
-	sys_resume_lock += 10;
+	try
-	CoreThread.Cancel();
+	{
+		sys_resume_lock += 10;
+		CoreThread.Cancel();
 
-	if( m_CorePlugins )
+		if( m_CorePlugins )
-		m_CorePlugins->Shutdown();
+			m_CorePlugins->Shutdown();
+	}
+	catch( Exception::RuntimeError& ex )
+	{
+		// Handle runtime errors gracefully during shutdown.  Mostly these are things
+		// that we just don't care about by now, and just want to "get 'er done!" so
+		// we can exit the app. ;)
 
+		Console.Error( ex.FormatDiagnosticMessage() );
+	}
+	
 	// Notice: deleting the plugin manager (unloading plugins) here causes Lilypad to crash,
 	// likely due to some pending message in the queue that references lilypad procs.
 	// We don't need to unload plugins anyway tho -- shutdown is plenty safe enough for

pcsx2/gui/ConsoleLogger.h

@@ -68,6 +68,8 @@ protected:
 
 class ConsoleTestThread : public Threading::PersistentThread
 {
+	typedef PersistentThread _parent;
+
 protected:
 	volatile bool m_done;
 	void ExecuteTaskInThread();
@@ -82,10 +84,6 @@ public:
 	{
 		m_done = true;
 	}
-
-	protected:
-		void OnStart() {}
-		void OnCleanupInThread() {}
 };
 
 // --------------------------------------------------------------------------------------

pcsx2/gui/MainFrame.cpp

@@ -280,7 +280,7 @@ void MainEmuFrame::OnSettingsLoadSave( void* obj, const IniInterface& evt )
 {
 	if( obj == NULL ) return;
 	MainEmuFrame* mframe = (MainEmuFrame*)obj;
-	
+
 	// FIXME: Evil const cast hack!
 	mframe->LoadSaveRecentIsoList( const_cast<IniInterface&>(evt) );
 }
@@ -312,7 +312,7 @@ MainEmuFrame::MainEmuFrame(wxWindow* parent, const wxString& title):
 	m_SaveStatesSubmenu( *MakeStatesSubMenu( MenuId_State_Save01 ) ),
 
 	m_MenuItem_Console( *new wxMenuItem( &m_menuMisc, MenuId_Console, L"Show Console", wxEmptyString, wxITEM_CHECK ) ),
-	
+
 	m_Listener_CoreThreadStatus( wxGetApp().Source_CoreThreadStatus(), CmdEvt_Listener( this, OnCoreThreadStatusChanged ) ),
 	m_Listener_CorePluginStatus( wxGetApp().Source_CorePluginStatus(), CmdEvt_Listener( this, OnCorePluginStatusChanged ) ),
 	m_Listener_SettingsApplied( wxGetApp().Source_SettingsApplied(), EventListener<int>( this, OnSettingsApplied ) ),
@@ -518,6 +518,8 @@ void MainEmuFrame::ReloadRecentLists()
 
 void MainEmuFrame::ApplyCoreStatus()
 {
+	bool valstate = SysHasValidState();
+
 	GetMenuBar()->Enable( MenuId_Sys_SuspendResume, SysHasValidState() );
 	GetMenuBar()->Enable( MenuId_Sys_Reset, SysHasValidState() || (g_plugins!=NULL) );
 

pcsx2/gui/MainMenuClicks.cpp

@@ -1,6 +1,6 @@
 /*  PCSX2 - PS2 Emulator for PCs
  *  Copyright (C) 2002-2009  PCSX2 Dev Team
- * 
+ *
  *  PCSX2 is free software: you can redistribute it and/or modify it under the terms
  *  of the GNU Lesser General Public License as published by the Free Software Found-
  *  ation, either version 3 of the License, or (at your option) any later version.
@@ -70,7 +70,7 @@ bool MainEmuFrame::_DoSelectIsoBrowser()
 		UpdateIsoSrcFile();
 		return true;
 	}
-	
+
 	return false;
 }
 
@@ -78,7 +78,7 @@ void MainEmuFrame::Menu_BootCdvd_Click( wxCommandEvent &event )
 {
 	CoreThread.Suspend();
 
-	if( !wxFileExists( g_Conf->CurrentIso ) )
+	if( (g_Conf->CdvdSource == CDVDsrc_Iso) && !wxFileExists(g_Conf->CurrentIso) )
 	{
 		if( !_DoSelectIsoBrowser() )
 		{

pcsx2/gui/Panels/ConfigurationPanels.h

@@ -416,9 +416,7 @@ namespace Panels
 			void DoNextPlugin( int evtidx );
 
 		protected:
-			void OnStart() {}
 			void ExecuteTaskInThread();
-			void OnCleanupInThread() {}
 		};
 
 		// This panel contains all of the plugin combo boxes.  We stick them

pcsx2/gui/Plugins.cpp

@@ -57,7 +57,6 @@ public:
 	virtual ~LoadPluginsTask() throw();
 
 protected:
-	void OnStart() {}
 	void OnCleanupInThread();
 	void ExecuteTaskInThread();
 };

pcsx2/windows/WinConsolePipe.cpp

@@ -97,8 +97,6 @@ public:
 	}
 	
 protected:
-	void OnStart() {}
-
 	void ExecuteTaskInThread()
 	{
 		SetThreadPriority( GetCurrentThread(), THREAD_PRIORITY_BELOW_NORMAL );
@@ -165,8 +163,6 @@ protected:
 			Console.Error( ex.FormatDiagnosticMessage() );
 		}
 	}
-
-	void OnCleanupInThread() { }
 };
 
 // --------------------------------------------------------------------------------------

pcsx2/x86/ix86-32/iR5900-32.cpp

@@ -335,7 +335,7 @@ static DynGenFunc* EnterRecompiledCode	= NULL;
 //      stackframe setup code in this function)
 static void __fastcall StackFrameCheckFailed( int espORebp, int regval )
 {
-	pxFailDev( wxsFormat( L"(Stackframe) Sanitycheck Failed on %s\n\tCurrent=%d; Saved=%d",
+	pxFailDev( wxsFormat( L"(Stackframe) Sanity check failed on %s\n\tCurrent=%d; Saved=%d",
 		(espORebp==0) ? L"ESP" : L"EBP", regval, (espORebp==0) ? s_store_esp : s_store_ebp )
 	);
 
@@ -346,14 +346,14 @@ static void __fastcall StackFrameCheckFailed( int espORebp, int regval )
 
 static void _DynGen_StackFrameCheck()
 {
-	if( true ) return;
+	if( !IsDevBuild ) return;
 
 	// --------- EBP Here -----------
 
 	xCMP( ebp, &s_store_ebp );
 	xForwardJE8 skipassert_ebp;
 
-	xMOV( ecx, 1 );
+	xMOV( ecx, 1 );					// 1 specifies EBP
 	xMOV( edx, ebp );
 	xCALL( StackFrameCheckFailed );
 	xMOV( ebp, &s_store_ebp );		// half-hearted frame recovery attempt!
@@ -365,7 +365,7 @@ static void _DynGen_StackFrameCheck()
 	xCMP( esp, &s_store_esp );
 	xForwardJE8 skipassert_esp;
 
-	xMOV( ecx, 1 );
+	xXOR( ecx, ecx );				// 0 specifies ESI
 	xMOV( edx, esp );
 	xCALL( StackFrameCheckFailed );
 	xMOV( esp, &s_store_esp );		// half-hearted frame recovery attempt!
@@ -423,21 +423,21 @@ static DynGenFunc* _DynGen_EnterRecompiledCode()
 	//   for the duration of our function, and is used to restore the original
 	//   esp before returning from the function
 
-	// Optimization: We "allocate" 0x20 bytes of stack ahead of time here.  The first
+	// Optimization: We "allocate" 0x10 bytes of stack ahead of time here, which we can
-	// 16 bytes are used for saving esi, edi, and ebx.  The second 16 bytes are used
+	// use for supplying parameters to cdecl functions.
-	// for passing parameters to stdcall/cdecl functions.
 
 	xPUSH( ebp );
+	xPUSH( edi );
+	xPUSH( esi );
+	xPUSH( ebx );
+	
 	xMOV( ebp, esp );
 	xAND( esp, -0x10 );
-	xSUB( esp, 0x20 );
+	xSUB( esp, 0x10 );
 
 	xMOV( &s_store_ebp, ebp );
 	xMOV( &s_store_esp, esp );
-
+	xSUB( ptr32[&s_store_esp], 4 );		// account for the address pushed when we xCALL
-	xMOV( ptr[esp+0x18], edi );
-	xMOV( ptr[esp+0x14], esi );
-	xMOV( ptr[esp+0x10], ebx );
 
 	//xPUSH( edi );
 	//xPUSH( esi );
@@ -453,12 +453,14 @@ static DynGenFunc* _DynGen_EnterRecompiledCode()
 	//xPOP( ebp );
 	//xRET();
 
+	xADD( ptr32[&s_store_esp], 4 );		// account for the address pushed when we xCALL
 	_DynGen_StackFrameCheck();
 
-	xMOV( edi, ptr[esp+0x18] );
-	xMOV( esi, ptr[esp+0x14] );
-	xMOV( ebx, ptr[esp+0x10] );
 	xMOV( esp, ebp );
+	
+	xPOP( ebx );
+	xPOP( esi );
+	xPOP( edi );
 	xPOP( ebp );
 	xRET();
 

plugins/spu2-x/src/Timestretcher.cpp

@@ -241,7 +241,7 @@ static void CvtPacketToFloat( StereoOut32* srcdest )
 // Parameter note: Size should always be a multiple of 128, thanks!
 static void CvtPacketToInt( StereoOut32* srcdest, uint size )
 {
-	jASSUME( (size & 127) == 0 );
+	//jASSUME( (size & 127) == 0 );
 	
 	const StereoOutFloat* src = (StereoOutFloat*)srcdest;
 	StereoOut32* dest = srcdest;