* Documented some of the event/threading proxy class and its underlying event queue.

* Simplified and improved (slightly) the savestate memory cleanup on error/cancellation.

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

pcsx2/ZipTools/ThreadedZipTools.h

@@ -83,6 +83,8 @@ protected:
 	ScopedPtr<ArchiveDataBuffer>	m_data;
 
 public:
+	virtual ~ArchiveEntryList() throw() {}
+
 	ArchiveEntryList() {}
 
 	ArchiveEntryList( ArchiveDataBuffer* data )
@@ -90,13 +92,11 @@ public:
 		m_data = data;
 	}
 
-	ArchiveEntryList( ScopedPtr<ArchiveDataBuffer>& data )
+	ArchiveEntryList( ArchiveDataBuffer& data )
 	{
-		m_data = data.DetachPtr();
+		m_data = &data;
 	}
 	
-	virtual ~ArchiveEntryList() throw() {}
-	
 	const VmStateBuffer* GetBuffer() const
 	{
 		return m_data;
@@ -148,8 +148,8 @@ class BaseCompressThread
 	typedef pxThread _parent;
 
 protected:
-	ScopedPtr< ArchiveEntryList >	m_src_list;
+	pxOutputStream*					m_gzfp;
-	ScopedPtr< pxOutputStream >		m_gzfp;
+	ArchiveEntryList*				m_src_list;
 	bool							m_PendingSaveFlag;
 	
 	wxString						m_final_filename;
@@ -163,9 +163,9 @@ public:
 		return *this;
 	}
 
-	BaseCompressThread& SetSource( ScopedPtr< ArchiveEntryList >& srcdata )
+	BaseCompressThread& SetSource( ArchiveEntryList& srcdata )
 	{
-		m_src_list = srcdata.DetachPtr();
+		m_src_list = &srcdata;
 		return *this;
 	}
 
@@ -175,9 +175,9 @@ public:
 		return *this;
 	}
 
-	BaseCompressThread& SetOutStream( ScopedPtr< pxOutputStream >& out )
+	BaseCompressThread& SetOutStream( pxOutputStream& out )
 	{
-		m_gzfp = out.DetachPtr();
+		m_gzfp = &out;
 		return *this;
 	}
 

pcsx2/ZipTools/thread_gzip.cpp

@@ -48,7 +48,6 @@ void BaseCompressThread::ExecuteTaskInThread()
 	//    result over the original.
 
 	if( !m_src_list ) return;
-
 	SetPendingSave();
 	
 	Yield( 3 );
@@ -89,5 +88,8 @@ void BaseCompressThread::OnCleanupInThread()
 {
 	_parent::OnCleanupInThread();
 	wxGetApp().DeleteThread( this );
+
+	safe_delete(m_gzfp);
+	safe_delete(m_src_list);
 }
 

pcsx2/gui/AppInit.cpp

@@ -781,7 +781,7 @@ void Pcsx2App::OnDestroyWindow( wxWindowDestroyEvent& evt )
 // --------------------------------------------------------------------------------------
 //  SysEventHandler
 // --------------------------------------------------------------------------------------
-class SysEvtHandler : public pxEvtHandler
+class SysEvtHandler : public pxEvtQueue
 {
 public:
 	wxString GetEvtHandlerName() const { return L"SysExecutor"; }

pcsx2/gui/ExecutorThread.cpp

@@ -22,13 +22,13 @@ using namespace pxSizerFlags;
 //  ConsoleLogSource_Event  (implementations)
 // --------------------------------------------------------------------------------------
 
-bool ConsoleLogSource_Event::Write( const pxEvtHandler* evtHandler, const SysExecEvent* evt, const wxChar* msg ) {
+bool ConsoleLogSource_Event::Write( const pxEvtQueue* evtHandler, const SysExecEvent* evt, const wxChar* msg ) {
 	return _parent::Write( pxsFmt(L"(%s:%s) ", evtHandler->GetEventHandlerName().c_str(), evt->GetEventName().c_str()) + msg );
 }
-bool ConsoleLogSource_Event::Warn( const pxEvtHandler* evtHandler, const SysExecEvent* evt, const wxChar* msg )	{
+bool ConsoleLogSource_Event::Warn( const pxEvtQueue* evtHandler, const SysExecEvent* evt, const wxChar* msg )	{
 	return _parent::Write( pxsFmt(L"(%s:%s) ", evtHandler->GetEventHandlerName().c_str(), evt->GetEventName().c_str()) + msg );
 }
-bool ConsoleLogSource_Event::Error( const pxEvtHandler* evtHandler, const SysExecEvent* evt, const wxChar* msg ) {
+bool ConsoleLogSource_Event::Error( const pxEvtQueue* evtHandler, const SysExecEvent* evt, const wxChar* msg ) {
 	return _parent::Write( pxsFmt(L"(%s:%s) ", evtHandler->GetEventHandlerName().c_str(), evt->GetEventName().c_str()) + msg );
 }
 
@@ -84,7 +84,7 @@ void SysExecEvent::SetException( BaseException* ex )
 {
 	if( !ex ) return;
 
-	ex->DiagMsg() += wxsFormat(L"(%s) ", GetEventName().c_str());
+	ex->DiagMsg() += pxsFmt(L"(%s) ", GetEventName().c_str());
 	//ex->UserMsg() = prefix + ex->UserMsg();
 
 	if( m_sync )
@@ -153,9 +153,9 @@ void SysExecEvent::PostResult() const
 }
 
 // --------------------------------------------------------------------------------------
-//  pxEvtHandler Implementations
+//  pxEvtQueue Implementations
 // --------------------------------------------------------------------------------------
-pxEvtHandler::pxEvtHandler()
+pxEvtQueue::pxEvtQueue()
 {
 	AtomicExchange( m_Quitting, false );
 	m_qpc_Start = 0;
@@ -167,7 +167,7 @@ pxEvtHandler::pxEvtHandler()
 // (typically these are shutdown events critical to closing the app cleanly). Once
 // all such events have been processed, the thread is stopped.
 //
-void pxEvtHandler::ShutdownQueue()
+void pxEvtQueue::ShutdownQueue()
 {
 	if( m_Quitting ) return;
 	AtomicExchange( m_Quitting, true );
@@ -190,7 +190,7 @@ struct ScopedThreadCancelDisable
 };
 
 // isIdle  - parameter is useful for logging only (currently)
-void pxEvtHandler::ProcessEvents( pxEvtList& list, bool isIdle )
+void pxEvtQueue::ProcessEvents( pxEvtList& list, bool isIdle )
 {
 	ScopedLock synclock( m_mtx_pending );
     
@@ -238,19 +238,19 @@ void pxEvtHandler::ProcessEvents( pxEvtList& list, bool isIdle )
 	}
 }
 
-void pxEvtHandler::ProcessIdleEvents()
+void pxEvtQueue::ProcessIdleEvents()
 {
 	ProcessEvents( m_idleEvents, true );
 }
 
-void pxEvtHandler::ProcessPendingEvents()
+void pxEvtQueue::ProcessPendingEvents()
 {
 	ProcessEvents( m_pendingEvents );
 }
 
 // This method is provided for wxWidgets API conformance.  I like to use PostEvent instead
 // since it's reminiscent of PostMessage in Windows (and behaves rather similarly).
-void pxEvtHandler::AddPendingEvent( SysExecEvent& evt )
+void pxEvtQueue::AddPendingEvent( SysExecEvent& evt )
 {
 	PostEvent( evt );
 }
@@ -261,7 +261,7 @@ void pxEvtHandler::AddPendingEvent( SysExecEvent& evt )
 // to it automatically when the event has been executed.  If you are using a scoped event
 // you should use the Reference/Handle overload instead!
 //
-void pxEvtHandler::PostEvent( SysExecEvent* evt )
+void pxEvtQueue::PostEvent( SysExecEvent* evt )
 {
 	ScopedPtr<SysExecEvent> sevt( evt );
 	if( !sevt ) return;
@@ -274,19 +274,19 @@ void pxEvtHandler::PostEvent( SysExecEvent* evt )
 
 	ScopedLock synclock( m_mtx_pending );
 	
-	pxEvtLog.Write( this, evt, wxsFormat(L"Posting event! (pending=%d, idle=%d)", m_pendingEvents.size(), m_idleEvents.size()) );
+	pxEvtLog.Write( this, evt, pxsFmt(L"Posting event! (pending=%d, idle=%d)", m_pendingEvents.size(), m_idleEvents.size()) );
 
 	m_pendingEvents.push_back( sevt.DetachPtr() );
 	if( m_pendingEvents.size() == 1)
 		m_wakeup.Post();
 }
 
-void pxEvtHandler::PostEvent( const SysExecEvent& evt )
+void pxEvtQueue::PostEvent( const SysExecEvent& evt )
 {
 	PostEvent( evt.Clone() );
 }
 
-void pxEvtHandler::PostIdleEvent( SysExecEvent* evt )
+void pxEvtQueue::PostIdleEvent( SysExecEvent* evt )
 {
 	if( !evt ) return;
 
@@ -298,7 +298,7 @@ void pxEvtHandler::PostIdleEvent( SysExecEvent* evt )
 
 	ScopedLock synclock( m_mtx_pending );
 
-	pxEvtLog.Write( this, evt, wxsFormat(L"Posting event! (pending=%d, idle=%d) [idle]", m_pendingEvents.size(), m_idleEvents.size()) );
+	pxEvtLog.Write( this, evt, pxsFmt(L"Posting event! (pending=%d, idle=%d) [idle]", m_pendingEvents.size(), m_idleEvents.size()) );
 
 	if( m_pendingEvents.size() == 0)
 	{
@@ -309,12 +309,12 @@ void pxEvtHandler::PostIdleEvent( SysExecEvent* evt )
 		m_idleEvents.push_back( evt );
 }
 
-void pxEvtHandler::PostIdleEvent( const SysExecEvent& evt )
+void pxEvtQueue::PostIdleEvent( const SysExecEvent& evt )
 {
 	PostIdleEvent( evt.Clone() );
 }
 
-void pxEvtHandler::ProcessEvent( SysExecEvent& evt )
+void pxEvtQueue::ProcessEvent( SysExecEvent& evt )
 {
 	if( wxThread::GetCurrentId() != m_OwnerThreadId )
 	{
@@ -327,7 +327,7 @@ void pxEvtHandler::ProcessEvent( SysExecEvent& evt )
 		evt._DoInvokeEvent();
 }
 
-void pxEvtHandler::ProcessEvent( SysExecEvent* evt )
+void pxEvtQueue::ProcessEvent( SysExecEvent* evt )
 {
 	if( !evt ) return;
 
@@ -345,7 +345,7 @@ void pxEvtHandler::ProcessEvent( SysExecEvent* evt )
 	}
 }
 
-bool pxEvtHandler::Rpc_TryInvokeAsync( FnType_Void* method, const wxChar* traceName )
+bool pxEvtQueue::Rpc_TryInvokeAsync( FnType_Void* method, const wxChar* traceName )
 {
 	if( wxThread::GetCurrentId() != m_OwnerThreadId )
 	{
@@ -356,7 +356,7 @@ bool pxEvtHandler::Rpc_TryInvokeAsync( FnType_Void* method, const wxChar* traceN
 	return false;
 }
 
-bool pxEvtHandler::Rpc_TryInvoke( FnType_Void* method, const wxChar* traceName )
+bool pxEvtQueue::Rpc_TryInvoke( FnType_Void* method, const wxChar* traceName )
 {
 	if( wxThread::GetCurrentId() != m_OwnerThreadId )
 	{
@@ -376,19 +376,17 @@ bool pxEvtHandler::Rpc_TryInvoke( FnType_Void* method, const wxChar* traceName )
 // This method invokes the derived class Idle implementations (if any) and then enters
 // the sleep state until such time that new messages are received.
 //
-// FUTURE: Processes idle messages from the idle message queue (not implemented yet).
-//
 // Extending: Derived classes should override _DoIdle instead, unless it is necessary
 // to implement post-wakeup behavior.
 //
-void pxEvtHandler::Idle()
+void pxEvtQueue::Idle()
 {
 	ProcessIdleEvents();
 	_DoIdle();
 	m_wakeup.WaitWithoutYield();
 }
 
-void pxEvtHandler::SetActiveThread()
+void pxEvtQueue::SetActiveThread()
 {
 	m_OwnerThreadId = wxThread::GetCurrentId();
 }
@@ -454,7 +452,7 @@ void WaitingForThreadedTaskDialog::OnTerminateApp_Clicked( wxCommandEvent& evt )
 // --------------------------------------------------------------------------------------
 //  ExecutorThread Implementations
 // --------------------------------------------------------------------------------------
-ExecutorThread::ExecutorThread( pxEvtHandler* evthandler )
+ExecutorThread::ExecutorThread( pxEvtQueue* evthandler )
 {
 	m_EvtHandler = evthandler;
 }
@@ -465,7 +463,7 @@ bool ExecutorThread::IsRunning() const
 	return( !m_EvtHandler->IsShuttingDown() );
 }
 
-// Exposes the internal pxEvtHandler::ShutdownQueue API.  See pxEvtHandler for details.
+// Exposes the internal pxEvtQueue::ShutdownQueue API.  See pxEvtQueue for details.
 void ExecutorThread::ShutdownQueue()
 {
 	if( !m_EvtHandler ) return;
@@ -476,7 +474,7 @@ void ExecutorThread::ShutdownQueue()
 	Block();
 }
 
-// Exposes the internal pxEvtHandler::PostEvent API.  See pxEvtHandler for details.
+// Exposes the internal pxEvtQueue::PostEvent API.  See pxEvtQueue for details.
 void ExecutorThread::PostEvent( SysExecEvent* evt )
 {
 	if( !pxAssert( m_EvtHandler ) ) { delete evt; return; }
@@ -489,7 +487,7 @@ void ExecutorThread::PostEvent( const SysExecEvent& evt )
 	m_EvtHandler->PostEvent( evt );
 }
 
-// Exposes the internal pxEvtHandler::PostIdleEvent API.  See pxEvtHandler for details.
+// Exposes the internal pxEvtQueue::PostIdleEvent API.  See pxEvtQueue for details.
 void ExecutorThread::PostIdleEvent( SysExecEvent* evt )
 {
 	if( !pxAssert( m_EvtHandler ) ) return;
@@ -502,7 +500,7 @@ void ExecutorThread::PostIdleEvent( const SysExecEvent& evt )
 	m_EvtHandler->PostIdleEvent( evt );
 }
 
-// Exposes the internal pxEvtHandler::ProcessEvent API.  See pxEvtHandler for details.
+// Exposes the internal pxEvtQueue::ProcessEvent API.  See pxEvtQueue for details.
 void ExecutorThread::ProcessEvent( SysExecEvent* evt )
 {
 	if( m_EvtHandler )

pcsx2/gui/SysState.cpp

@@ -403,10 +403,10 @@ public:
 
 	SysExecEvent_ZipToDisk* Clone() const { return new SysExecEvent_ZipToDisk( *this ); }
 
-	SysExecEvent_ZipToDisk( ScopedPtr<ArchiveEntryList>& srclist, const wxString& filename )
+	SysExecEvent_ZipToDisk( ArchiveEntryList& srclist, const wxString& filename )
 		: m_filename( filename )
 	{
-		m_src_list = srclist.DetachPtr();
+		m_src_list = &srclist;
 	}
 
 	SysExecEvent_ZipToDisk( ArchiveEntryList* srclist, const wxString& filename )
@@ -454,13 +454,14 @@ protected:
 		}
 
 		(*new VmStateCompressThread())
-			.SetSource(m_src_list)
+			.SetSource(elist)
 			.SetOutStream(out)
 			.SetFinishedPath(m_filename)
 			.Start();
 
 		// No errors?  Release cleanup handlers:			
 		elist.DetachPtr();
+		out.DetachPtr();
 	}
 	
 	void CleanupEvent()
@@ -630,6 +631,8 @@ void StateCopy_SaveToFile( const wxString& file )
 	
 	GetSysExecutorThread().PostEvent(new SysExecEvent_DownloadState	( ziplist ));
 	GetSysExecutorThread().PostEvent(new SysExecEvent_ZipToDisk		( ziplist, file ));
+	
+	ziplist.DetachPtr();
 }
 
 void StateCopy_LoadFromFile( const wxString& file )

pcsx2/gui/pxEventThread.h

@@ -22,7 +22,7 @@
 // TODO!!  Make the system defined in this header system a bit more generic, and then move
 // it to the Utilities library.
 
-class pxEvtHandler;
+class pxEvtQueue;
 class SysExecEvent;
 
 // --------------------------------------------------------------------------------------
@@ -38,9 +38,9 @@ public:
 
 	ConsoleLogSource_Event();
 
-	bool Write( const pxEvtHandler* evtHandler, const SysExecEvent* evt, const wxChar* msg );
+	bool Write( const pxEvtQueue* evtHandler, const SysExecEvent* evt, const wxChar* msg );
-	bool Warn( const pxEvtHandler* evtHandler, const SysExecEvent* evt, const wxChar* msg );
+	bool Warn( const pxEvtQueue* evtHandler, const SysExecEvent* evt, const wxChar* msg );
-	bool Error( const pxEvtHandler* evtHandler, const SysExecEvent* evt, const wxChar* msg );
+	bool Error( const pxEvtQueue* evtHandler, const SysExecEvent* evt, const wxChar* msg );
 };
 
 extern ConsoleLogSource_Event pxConLog_Event;
@@ -51,7 +51,7 @@ extern ConsoleLogSource_Event pxConLog_Event;
 // --------------------------------------------------------------------------------------
 //  SysExecEvent
 // --------------------------------------------------------------------------------------
-// Base class for all pxEvtHandler processable events.
+// Base class for all pxEvtQueue processable events.
 //
 // Rules for deriving:
 //  * Override InvokeEvent(), *NOT* _DoInvokeEvent().  _DoInvokeEvent() performs setup and
@@ -67,7 +67,11 @@ extern ConsoleLogSource_Event pxConLog_Event;
 //    awaking the invoking thread as soon as the queue has caught up to and processed
 //    the event.
 //
-//  * Avoid using virtual class inheritence.  It's unreliable at best.
+//  * Avoid using virtual class inheritence to 'cleverly' bind a SysExecEvent to another
+//    existing class.  Multiple inheritence is unreliable at best, and virtual inheritence
+//    is even worse.  Create a SysExecEvent wrapper class instead, and embed an instance of
+//    the other class you want to turn into an event within it.  It might feel like more work
+//    but it *will* be less work in the long run.
 //
 class SysExecEvent : public ICloneable
 {
@@ -174,13 +178,15 @@ protected:
 #endif
 
 // --------------------------------------------------------------------------------------
-//  pxEvtHandler
+//  pxEvtQueue
 // --------------------------------------------------------------------------------------
-// Purpose: To provide a safe environment for queuing tasks that must be executed in
+// Thread-safe platform-independent message queue, intended to act as a proxy between
-// sequential order (in blocking fashion).  Unlike the wxWidgets event handlers, instances
+// control threads (such as the Main/UI thread) and worker threads.
-// of this handler can be stalled for extended periods of time without affecting the
+//
-// responsiveness of the GUI or frame updates of the DirectX output windows.  This class
+// Proxy message queues provide a safe environment for queuing tasks that must be executed
-// is mostly intended to be used from the context of an ExecutorThread.
+// in sequential order (in blocking fashion) on one or more worker threads.  The queue is
+// deadlock-free, which means the Main/UI thread can queue events into this EventQueue without
+// fear of causing unresponsiveness within the user interface.
 //
 // Rationales:
 //  * Using the main event handler of wxWidgets is dangerous because it must call itself
@@ -189,12 +195,11 @@ protected:
 //    running events that expect the suspend to be complete while the suspend was still
 //    pending.
 //
-//  * wxWidgets Event Queue (wxEvtHandler) isn't thread-safe and isn't even
+//  * wxWidgets Event Queue (wxEvtHandler) isn't thread-safe and isn't even intended for
-//    intended for use for anything other than wxWindow events (it uses static vars
+//    use for anything other than wxWindow events (it uses static vars and checks/modifies
-//    and checks/modifies wxApp globals while processing), so it's useless to us.
+//    wxApp globals while processing), so it's useless to us.  Have to roll our own. -_-
-//    Have to roll our own. -_-
 //
-class pxEvtHandler
+class pxEvtQueue
 {
 protected:
 	pxEvtList					m_pendingEvents;
@@ -210,10 +215,10 @@ protected:
 	volatile u64				m_qpc_Start;
 
 public:
-	pxEvtHandler();
+	pxEvtQueue();
-	virtual ~pxEvtHandler() throw() {}
+	virtual ~pxEvtQueue() throw() {}
 
-	virtual wxString GetEventHandlerName() const { return L"pxEvtHandler"; }
+	virtual wxString GetEventHandlerName() const { return L"pxEvtQueue"; }
 
 	virtual void ShutdownQueue();
 	bool IsShuttingDown() const { return !!m_Quitting; }
@@ -243,8 +248,22 @@ protected:
 // --------------------------------------------------------------------------------------
 //  ExecutorThread
 // --------------------------------------------------------------------------------------
-// Threaded wrapper class for implementing pxEvtHandler.  Simply create the desired
+// Threaded wrapper class for implementing a pxEvtQueue on its own thread, to serve as a
-// EvtHandler, start the thread, and enjoy queued event execution in fully blocking fashion.
+// proxy between worker threads and response threads (which cannot be allowed to stall or
+// deadlock).  Simply create the desired EvtHandler, start the thread, and enjoy queued
+// event execution in fully blocking fashion.
+//
+// Deadlock Protection Notes:
+//  The ExecutorThread utilizes an underlying pxEventQueue, which only locks the queue for
+//  adding and removing items only.  Events are processed during an unlocked queue state,
+//  which allows other threads to add events with a guarantee that the add operation will
+//  take very little time.
+//
+// Implementation Notes:
+//  We use object encapsulation instead of multiple inheritance in order to avoid the many
+//  unavoidable catastrophes and pitfalls involved in multi-inheritance that would ruin our
+//  will to live.  The alternative requires manually exposing the interface of the underlying
+//  instance of pxEventQueue; and that's ok really when you consider the alternative. --air
 //
 class ExecutorThread : public Threading::pxThread
 {
@@ -252,10 +271,10 @@ class ExecutorThread : public Threading::pxThread
 
 protected:
 	ScopedPtr<wxTimer>		m_ExecutorTimer;
-	ScopedPtr<pxEvtHandler>	m_EvtHandler;
+	ScopedPtr<pxEvtQueue>	m_EvtHandler;
 
 public:
-	ExecutorThread( pxEvtHandler* evtandler = NULL );
+	ExecutorThread( pxEvtQueue* evtandler = NULL );
 	virtual ~ExecutorThread() throw() { }
 
 	virtual void ShutdownQueue();