From 877f59a255f926b6d4872342fc751f585b63119f Mon Sep 17 00:00:00 2001 From: "Jake.Stine" Date: Sun, 18 Oct 2009 03:01:10 +0000 Subject: [PATCH] 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 c4ddce3faf..48a42880cd 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 834f96fe67..36c4dc877f 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 d0f266ca71..748b8ddabf 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 5a4f28d1dc..a2d40b16ca 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 b6891427d3..9732e54a31 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 c38b298b36..eb9874030b 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() ) {