From 6b0c9cfdfe6ec2ae07fd6f8668792b4f6d106ebc Mon Sep 17 00:00:00 2001 From: "Jake.Stine" Date: Sat, 17 Oct 2009 23:30:16 +0000 Subject: [PATCH 1/4] Added some missing semaphore/mutex resets, and bugfixed the emitter when using a certain type of complex indirect sddressing ( ie, ptr32[addr + (eax*4)] ) git-svn-id: http://pcsx2.googlecode.com/svn/trunk@2026 96395faa-99c1-11dd-bbfe-3dabce05a288 --- common/include/x86emitter/inlines.inl | 6 ++++-- common/src/Utilities/ThreadTools.cpp | 1 + pcsx2/MTGS.cpp | 3 +-- pcsx2/System/SysThreads.cpp | 14 +------------- 4 files changed, 7 insertions(+), 17 deletions(-) diff --git a/common/include/x86emitter/inlines.inl b/common/include/x86emitter/inlines.inl index 5ad9e5f97..8d6e9f185 100644 --- a/common/include/x86emitter/inlines.inl +++ b/common/include/x86emitter/inlines.inl @@ -232,10 +232,12 @@ namespace x86Emitter else if( Index.IsEmpty() ) { Index = src.Index; - Factor = 1; + Factor = src.Factor; } else if( Index == src.Index ) - Factor++; + { + Factor += src.Factor; + } else pxFailDev( L"x86Emitter: address modifiers cannot have more than two index registers." ); // oops, only 2 regs allowed per ModRm! diff --git a/common/src/Utilities/ThreadTools.cpp b/common/src/Utilities/ThreadTools.cpp index 29c2d3f98..c7ea45533 100644 --- a/common/src/Utilities/ThreadTools.cpp +++ b/common/src/Utilities/ThreadTools.cpp @@ -144,6 +144,7 @@ void Threading::PersistentThread::Start() Detach(); // clean up previous thread handle, if one exists. FrankenMutex( m_lock_InThread ); + m_sem_event.Reset(); OnStart(); diff --git a/pcsx2/MTGS.cpp b/pcsx2/MTGS.cpp index b09c1d2b8..c4ddce3fa 100644 --- a/pcsx2/MTGS.cpp +++ b/pcsx2/MTGS.cpp @@ -282,8 +282,7 @@ void mtgsThreadObject::ExecuteTaskInThread() AtomicExchange(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.Unlock(); + m_lock_RingRestart.Wait(); StateCheckInThread( false ); // disable cancel since the above locks are cancelable already continue; diff --git a/pcsx2/System/SysThreads.cpp b/pcsx2/System/SysThreads.cpp index 0ca5e9e3d..834f96fe6 100644 --- a/pcsx2/System/SysThreads.cpp +++ b/pcsx2/System/SysThreads.cpp @@ -55,7 +55,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(); @@ -228,16 +228,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 +256,6 @@ void SysThreadBase::StateCheckInThread( bool isCancelable ) // fallthrough... case ExecMode_Paused: - //locker.Unlock(); while( m_ExecMode == ExecMode_Paused ) m_ResumeEvent.WaitRaw(); @@ -284,7 +273,6 @@ void SysThreadBase::StateCheckInThread( bool isCancelable ) // fallthrough... case ExecMode_Closed: - //locker.Unlock(); while( m_ExecMode == ExecMode_Closed ) m_ResumeEvent.WaitRaw(); From 877f59a255f926b6d4872342fc751f585b63119f Mon Sep 17 00:00:00 2001 From: "Jake.Stine" Date: Sun, 18 Oct 2009 03:01:10 +0000 Subject: [PATCH 2/4] More thread sync fixes (these mostly only showed up on linux, because of threads starting a lot slower); fixed by including a startup signal to let the creating thread know when the worker has aquired its persistent locks. git-svn-id: http://pcsx2.googlecode.com/svn/trunk@2029 96395faa-99c1-11dd-bbfe-3dabce05a288 --- pcsx2/MTGS.cpp | 8 ++++---- pcsx2/System/SysThreads.cpp | 35 +++++++++++++++-------------------- pcsx2/System/SysThreads.h | 23 ++++++++++++++--------- pcsx2/gui/AppCoreThread.cpp | 7 ++++--- pcsx2/gui/MainFrame.cpp | 6 ++++-- pcsx2/gui/MainMenuClicks.cpp | 6 +++--- 6 files changed, 44 insertions(+), 41 deletions(-) diff --git a/pcsx2/MTGS.cpp b/pcsx2/MTGS.cpp index c4ddce3fa..48a42880c 100644 --- a/pcsx2/MTGS.cpp +++ b/pcsx2/MTGS.cpp @@ -237,8 +237,8 @@ void mtgsThreadObject::OpenPlugin() void mtgsThreadObject::ExecuteTaskInThread() { - // Required by the underlying SysThreadBase class (is unlocked on exit) m_RunningLock.Lock(); + m_StartupEvent.Post(); #ifdef RINGBUF_DEBUG_STACK PacketTagType prevCmd; @@ -247,7 +247,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; @@ -389,7 +389,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; @@ -397,7 +397,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); diff --git a/pcsx2/System/SysThreads.cpp b/pcsx2/System/SysThreads.cpp index 834f96fe6..36c4dc877 100644 --- a/pcsx2/System/SysThreads.cpp +++ b/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. @@ -47,6 +47,7 @@ void SysThreadBase::Start() _parent::Start(); m_ExecMode = ExecMode_Closing; m_sem_event.Post(); + m_StartupEvent.Wait(); } @@ -55,6 +56,7 @@ void SysThreadBase::OnStart() if( !pxAssertDev( m_ExecMode == ExecMode_NoThreadYet, "SysSustainableThread:Start(): Invalid execution mode" ) ) return; m_ResumeEvent.Reset(); + m_StartupEvent.Reset(); FrankenMutex( m_ExecModeMutex ); FrankenMutex( m_RunningLock ); @@ -83,7 +85,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 +154,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 +173,13 @@ void SysThreadBase::Resume() ScopedLock locker( m_ExecModeMutex ); - // Recursion guard is needed because of the non-blocking Wait if the state - // is Suspending/Closing. Processed events could recurse into Resume, and we'll - // want to silently ignore them. - - //RecursionGuard guard( m_resume_guard ); - //if( guard.IsReentrant() ) return; - switch( m_ExecMode ) { case ExecMode_Opened: return; case ExecMode_NoThreadYet: Start(); - m_ExecMode = ExecMode_Closing; + // fall through... case ExecMode_Closing: @@ -192,14 +187,12 @@ 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(); - + // 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; @@ -269,9 +262,9 @@ void SysThreadBase::StateCheckInThread( bool isCancelable ) OnSuspendInThread(); m_ExecMode = ExecMode_Closed; m_RunningLock.Unlock(); - } + } // fallthrough... - + case ExecMode_Closed: while( m_ExecMode == ExecMode_Closed ) m_ResumeEvent.WaitRaw(); @@ -279,7 +272,7 @@ void SysThreadBase::StateCheckInThread( bool isCancelable ) m_RunningLock.Lock(); OnResumeInThread( true ); break; - + jNO_DEFAULT; } } @@ -354,7 +347,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(EmuConfig) = src; if( resumeWhenDone ) Resume(); @@ -373,7 +366,7 @@ SysCoreThread& SysCoreThread::Get() void SysCoreThread::CpuInitializeMess() { if( m_hasValidState ) return; - + wxString elf_file; if( EmuConfig.SkipBiosSplash ) { @@ -434,7 +427,9 @@ void SysCoreThread::CpuExecute() void SysCoreThread::ExecuteTaskInThread() { m_RunningLock.Lock(); + tls_coreThread = this; + m_StartupEvent.Post(); m_sem_event.WaitRaw(); StateCheckInThread(); diff --git a/pcsx2/System/SysThreads.h b/pcsx2/System/SysThreads.h index d0f266ca7..748b8ddab 100644 --- a/pcsx2/System/SysThreads.h +++ b/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,12 @@ protected: // Used to wake up the thread from sleeping when it's in a suspended state. Semaphore m_ResumeEvent; - + + // Used to signal the creating thread that the worker has entered the running state. + // This is necessary because until the thread has established itself, locking against + // m_RunningLock isn't a reliable synchronization tool. + Semaphore m_StartupEvent; + // 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 +97,7 @@ public: { return m_ExecMode > ExecMode_Closed; } - + bool IsClosed() const { return !IsOpen(); } ExecutionMode GetExecutionMode() const { return m_ExecMode; } @@ -101,7 +106,7 @@ public: virtual bool Suspend( bool isBlocking = true ); virtual void Resume(); virtual bool Pause(); - + virtual void StateCheckInThread( bool isCancelable = true ); virtual void OnCleanupInThread(); @@ -123,7 +128,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 +168,7 @@ public: virtual void ApplySettings( const Pcsx2Config& src ); virtual void OnResumeReady(); virtual void Reset(); - + bool HasValidState() { return m_hasValidState; diff --git a/pcsx2/gui/AppCoreThread.cpp b/pcsx2/gui/AppCoreThread.cpp index 5a4f28d1d..a2d40b16c 100644 --- a/pcsx2/gui/AppCoreThread.cpp +++ b/pcsx2/gui/AppCoreThread.cpp @@ -41,7 +41,7 @@ bool AppCoreThread::Suspend( bool isBlocking ) m_kevt.m_shiftDown = false; m_kevt.m_controlDown = false; m_kevt.m_altDown = false; - + return retval; } @@ -66,7 +66,8 @@ void AppCoreThread::Resume() evt.SetInt( CoreStatus_Suspended ); wxGetApp().AddPendingEvent( evt ); - sApp.SysExecute(); + if( (m_ExecMode != ExecMode_Closing) || (m_ExecMode != ExecMode_Pausing) ) + sApp.SysExecute(); } } @@ -83,7 +84,7 @@ void AppCoreThread::OnResumeReady() void AppCoreThread::OnResumeInThread( bool isSuspended ) { _parent::OnResumeInThread( isSuspended ); - + wxCommandEvent evt( pxEVT_CoreThreadStatus ); evt.SetInt( CoreStatus_Resumed ); wxGetApp().AddPendingEvent( evt ); diff --git a/pcsx2/gui/MainFrame.cpp b/pcsx2/gui/MainFrame.cpp index b6891427d..9732e54a3 100644 --- a/pcsx2/gui/MainFrame.cpp +++ b/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(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( 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) ); diff --git a/pcsx2/gui/MainMenuClicks.cpp b/pcsx2/gui/MainMenuClicks.cpp index c38b298b3..eb9874030 100644 --- a/pcsx2/gui/MainMenuClicks.cpp +++ b/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() ) { From 6c631e009de7b2f534d01c6dd5ac387d0ff292cd Mon Sep 17 00:00:00 2001 From: "Jake.Stine" Date: Sun, 18 Oct 2009 12:30:00 +0000 Subject: [PATCH 3/4] Yet more thread sync fixes, mostly to do with rapid reset/resume/suspend/exit actions. git-svn-id: http://pcsx2.googlecode.com/svn/trunk@2030 96395faa-99c1-11dd-bbfe-3dabce05a288 --- common/include/Utilities/Threading.h | 38 +++++--- common/src/Utilities/Exceptions.cpp | 2 +- common/src/Utilities/Mutex.cpp | 5 +- common/src/Utilities/Semaphore.cpp | 7 +- common/src/Utilities/ThreadTools.cpp | 130 +++++++++++++++++-------- pcsx2/GS.h | 4 - pcsx2/MTGS.cpp | 72 ++++++++------ pcsx2/RecoverySystem.cpp | 3 +- pcsx2/System/SysThreads.cpp | 53 +++++++--- pcsx2/System/SysThreads.h | 14 +-- pcsx2/gui/AppCoreThread.cpp | 15 ++- pcsx2/gui/AppInit.cpp | 19 +++- pcsx2/gui/ConsoleLogger.h | 6 +- pcsx2/gui/Panels/ConfigurationPanels.h | 2 - pcsx2/gui/Plugins.cpp | 1 - pcsx2/windows/WinConsolePipe.cpp | 4 - 16 files changed, 243 insertions(+), 132 deletions(-) diff --git a/common/include/Utilities/Threading.h b/common/include/Utilities/Threading.h index d4b20d0fd..07a7d5717 100644 --- a/common/include/Utilities/Threading.h +++ b/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 OnCleanupInThread()=0; + virtual void OnStart(); + + 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 void AtomicExchangeAdd( volatile u32& Target, u32 value ); - extern void AtomicIncrement( volatile u32& Target ); - extern void AtomicDecrement( volatile u32& Target ); - extern void AtomicExchange( volatile s32& Target, s32 value ); - extern void AtomicExchangeAdd( volatile s32& Target, u32 value ); - extern void AtomicIncrement( volatile s32& Target ); - extern void AtomicDecrement( volatile s32& Target ); + extern u32 AtomicExchange( volatile u32& Target, u32 value ); + extern u32 AtomicExchangeAdd( volatile u32& Target, u32 value ); + extern u32 AtomicIncrement( volatile u32& Target ); + extern u32 AtomicDecrement( volatile u32& Target ); + extern s32 AtomicExchange( volatile s32& Target, s32 value ); + extern s32 AtomicExchangeAdd( volatile s32& Target, u32 value ); + extern s32 AtomicIncrement( volatile s32& Target ); + extern s32 AtomicDecrement( volatile s32& Target ); - extern void _AtomicExchangePointer( const void ** target, const void* value ); - extern void _AtomicCompareExchangePointer( const void ** target, const void* value, const void* comparand ); + extern void* _AtomicExchangePointer( void * volatile * const target, void* const value ); + 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 ) } diff --git a/common/src/Utilities/Exceptions.cpp b/common/src/Utilities/Exceptions.cpp index 359097f66..984713a6e 100644 --- a/common/src/Utilities/Exceptions.cpp +++ b/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 diff --git a/common/src/Utilities/Mutex.cpp b/common/src/Utilities/Mutex.cpp index 55c68055a..a90eadc34 100644 --- a/common/src/Utilities/Mutex.cpp +++ b/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 ); diff --git a/common/src/Utilities/Semaphore.cpp b/common/src/Utilities/Semaphore.cpp index e2619603f..7ac071a4a 100644 --- a/common/src/Utilities/Semaphore.cpp +++ b/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 ); diff --git a/common/src/Utilities/ThreadTools.cpp b/common/src/Utilities/ThreadTools.cpp index c7ea45533..4a5e2cafb 100644 --- a/common/src/Utilities/ThreadTools.cpp +++ b/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,10 +140,6 @@ void Threading::PersistentThread::Start() if( m_running ) return; Detach(); // clean up previous thread handle, if one exists. - - FrankenMutex( m_lock_InThread ); - m_sem_event.Reset(); - OnStart(); if( pthread_create( &m_thread, NULL, _internal_callback, this ) != 0 ) @@ -237,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 @@ -322,7 +367,6 @@ void Threading::PersistentThread::_ThreadCleanup() _try_virtual_invoke( &PersistentThread::OnCleanupInThread ); - m_running = false; m_lock_InThread.Unlock(); } @@ -331,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::OnCleanupInThread() {} +void Threading::PersistentThread::OnStart() +{ + 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 @@ -497,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 ); -} - -__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 ); + return _InterlockedExchangeAdd( (volatile long*)&Target, -1 ); } diff --git a/pcsx2/GS.h b/pcsx2/GS.h index 63ad972a4..d052bc0c1 100644 --- a/pcsx2/GS.h +++ b/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. diff --git a/pcsx2/MTGS.cpp b/pcsx2/MTGS.cpp index 48a42880c..1d826e6c2 100644 --- a/pcsx2/MTGS.cpp +++ b/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(); - m_QueuedFrames++; + AtomicIncrement( 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() { - m_RunningLock.Lock(); - m_StartupEvent.Post(); - #ifdef RINGBUF_DEBUG_STACK PacketTagType prevCmd; #endif @@ -279,7 +273,7 @@ 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.Wait(); @@ -322,11 +316,9 @@ void mtgsThreadObject::ExecuteTaskInThread() GSvsync(tag.data[0]); gsFrameSkip( !tag.data[1] ); - m_lock_FrameQueueCounter.Lock(); - AtomicDecrement( m_QueuedFrames ); - jASSUME( m_QueuedFrames >= 0 ); + int framecnt = AtomicDecrement( m_QueuedFrames ); + pxAssertDev( framecnt >= 0, "Frame queue sync count failure." ); //Console.Status( " << Frame Removed!" ); - m_lock_FrameQueueCounter.Unlock(); if( PADupdate != NULL ) { @@ -413,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; } @@ -474,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 ) @@ -490,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; @@ -605,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 ); - jASSUME( writepos < RingBufferSize ); + pxAssert( size < RingBufferSize ); + pxAssert( writepos < RingBufferSize ); m_packet_size = GIFPath_ParseTag(pathidx, srcdata, size); size = m_packet_size + 1; // takes into account our command qword. @@ -666,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; - AtomicExchange( m_WritePos, writepos ); + m_WritePos = writepos = 0; m_lock_RingRestart.Unlock(); SetEvent(); @@ -733,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; @@ -752,8 +743,8 @@ __forceinline uint mtgsThreadObject::_PrepForSimplePacket() __forceinline void mtgsThreadObject::_FinishSimplePacket( uint future_writepos ) { - assert( future_writepos != volatize(m_RingPos) ); - AtomicExchange( m_WritePos, future_writepos ); + pxAssert( future_writepos != volatize(m_RingPos) ); + m_WritePos = future_writepos; } void mtgsThreadObject::SendSimplePacket( GS_RINGTYPE type, int data0, int data1, int data2 ) @@ -794,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(); } @@ -802,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(); diff --git a/pcsx2/RecoverySystem.cpp b/pcsx2/RecoverySystem.cpp index c601e52fd..46b8f3575 100644 --- a/pcsx2/RecoverySystem.cpp +++ b/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(); } diff --git a/pcsx2/System/SysThreads.cpp b/pcsx2/System/SysThreads.cpp index 36c4dc877..6e3d0f807 100644 --- a/pcsx2/System/SysThreads.cpp +++ b/pcsx2/System/SysThreads.cpp @@ -46,8 +46,26 @@ 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(); - m_StartupEvent.Wait(); } @@ -56,7 +74,6 @@ void SysThreadBase::OnStart() if( !pxAssertDev( m_ExecMode == ExecMode_NoThreadYet, "SysSustainableThread:Start(): Invalid execution mode" ) ) return; m_ResumeEvent.Reset(); - m_StartupEvent.Reset(); FrankenMutex( m_ExecModeMutex ); FrankenMutex( m_RunningLock ); @@ -173,13 +190,27 @@ void SysThreadBase::Resume() ScopedLock locker( m_ExecModeMutex ); + // Implementation Note: + // The entire state coming out of a Wait is indeterminate because of user input + // 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 + // right, we should abort. + switch( m_ExecMode ) { case ExecMode_Opened: return; case ExecMode_NoThreadYet: - Start(); + { + static int __Guard = 0; + RecursionGuard guard( __Guard ); + if( guard.IsReentrant() ) return; + Start(); + if( !m_running || (m_ExecMode == ExecMode_NoThreadYet) ) + throw Exception::ThreadCreationError(); + if( m_ExecMode == ExecMode_Opened ) return; + } // fall through... case ExecMode_Closing: @@ -188,11 +219,7 @@ void SysThreadBase::Resume() // state before continuing... m_RunningLock.Wait(); - - // 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_running ) return; if( (m_ExecMode != ExecMode_Closed) && (m_ExecMode != ExecMode_Paused) ) return; if( g_plugins == NULL ) return; break; @@ -212,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; @@ -426,10 +460,7 @@ void SysCoreThread::CpuExecute() void SysCoreThread::ExecuteTaskInThread() { - m_RunningLock.Lock(); - tls_coreThread = this; - m_StartupEvent.Post(); m_sem_event.WaitRaw(); StateCheckInThread(); diff --git a/pcsx2/System/SysThreads.h b/pcsx2/System/SysThreads.h index 748b8ddab..687b4eada 100644 --- a/pcsx2/System/SysThreads.h +++ b/pcsx2/System/SysThreads.h @@ -74,11 +74,6 @@ protected: // Used to wake up the thread from sleeping when it's in a suspended state. Semaphore m_ResumeEvent; - // Used to signal the creating thread that the worker has entered the running state. - // This is necessary because until the thread has established itself, locking against - // m_RunningLock isn't a reliable synchronization tool. - Semaphore m_StartupEvent; - // 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. @@ -108,16 +103,17 @@ public: 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(); - -protected: + virtual void OnCleanupInThread(); + virtual void OnStartInThread(); // Used internally from Resume(), so let's make it private here. virtual void Start(); diff --git a/pcsx2/gui/AppCoreThread.cpp b/pcsx2/gui/AppCoreThread.cpp index a2d40b16c..a646a70f5 100644 --- a/pcsx2/gui/AppCoreThread.cpp +++ b/pcsx2/gui/AppCoreThread.cpp @@ -45,6 +45,9 @@ bool AppCoreThread::Suspend( bool isBlocking ) 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 ) @@ -67,8 +71,17 @@ void AppCoreThread::Resume() wxGetApp().AddPendingEvent( evt ); if( (m_ExecMode != ExecMode_Closing) || (m_ExecMode != ExecMode_Pausing) ) - sApp.SysExecute(); + { + if( ++resume_tries <= 2 ) + { + sApp.SysExecute(); + } + else + Console.Status( "SysResume: Multiple resume retries failed. Giving up..." ); + } } + + resume_tries = 0; } void AppCoreThread::OnResumeReady() diff --git a/pcsx2/gui/AppInit.cpp b/pcsx2/gui/AppInit.cpp index a54fad2f4..6032ff5bd 100644 --- a/pcsx2/gui/AppInit.cpp +++ b/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; - CoreThread.Cancel(); + try + { + sys_resume_lock += 10; + CoreThread.Cancel(); - if( m_CorePlugins ) - m_CorePlugins->Shutdown(); + if( m_CorePlugins ) + 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 diff --git a/pcsx2/gui/ConsoleLogger.h b/pcsx2/gui/ConsoleLogger.h index 68564e34a..a924fe9c7 100644 --- a/pcsx2/gui/ConsoleLogger.h +++ b/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() {} }; // -------------------------------------------------------------------------------------- diff --git a/pcsx2/gui/Panels/ConfigurationPanels.h b/pcsx2/gui/Panels/ConfigurationPanels.h index 7a18125f0..236e70dc8 100644 --- a/pcsx2/gui/Panels/ConfigurationPanels.h +++ b/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 diff --git a/pcsx2/gui/Plugins.cpp b/pcsx2/gui/Plugins.cpp index de2e8ec8c..c33f64a57 100644 --- a/pcsx2/gui/Plugins.cpp +++ b/pcsx2/gui/Plugins.cpp @@ -57,7 +57,6 @@ public: virtual ~LoadPluginsTask() throw(); protected: - void OnStart() {} void OnCleanupInThread(); void ExecuteTaskInThread(); }; diff --git a/pcsx2/windows/WinConsolePipe.cpp b/pcsx2/windows/WinConsolePipe.cpp index cf2fcf1e0..ed4e904a0 100644 --- a/pcsx2/windows/WinConsolePipe.cpp +++ b/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() { } }; // -------------------------------------------------------------------------------------- From 950c99f88646d3328725769768f8aa016b5e3005 Mon Sep 17 00:00:00 2001 From: "Jake.Stine" Date: Sun, 18 Oct 2009 12:30:54 +0000 Subject: [PATCH 4/4] wxWidgets/Win32: Bugfixed wxLog getting perma-suspended when calling YieldIfNeeded git-svn-id: http://pcsx2.googlecode.com/svn/trunk@2031 96395faa-99c1-11dd-bbfe-3dabce05a288 --- 3rdparty/wxWidgets/src/msw/app.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/3rdparty/wxWidgets/src/msw/app.cpp b/3rdparty/wxWidgets/src/msw/app.cpp index a00f5185d..b307a339a 100644 --- a/3rdparty/wxWidgets/src/msw/app.cpp +++ b/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