MTGS: Purge pxThread

pcsx2/CDVD/CDVD.cpp

@@ -25,6 +25,7 @@
 
 #include "common/FileSystem.h"
 #include "common/StringUtil.h"
+#include "common/Threading.h"
 
 #include "Ps1CD.h"
 #include "CDVD.h"
@@ -372,7 +373,7 @@ s32 cdvdWriteConfig(const u8* config)
 	return 0;
 }
 
-static MutexRecursive Mutex_NewDiskCB;
+static Threading::MutexRecursive Mutex_NewDiskCB;
 
 // Sets ElfCRC to the CRC of the game bound to the CDVD source.
 static __fi ElfObject* loadElf(std::string filename, bool isPSXElf)
@@ -414,7 +415,7 @@ static __fi ElfObject* loadElf(std::string filename, bool isPSXElf)
 static __fi void _reloadElfInfo(std::string elfpath)
 {
 	// Now's a good time to reload the ELF info...
-	ScopedLock locker(Mutex_NewDiskCB);
+	Threading::ScopedLock locker(Mutex_NewDiskCB);
 
 	if (elfpath == LastELF)
 		return;
@@ -911,7 +912,7 @@ void SaveStateBase::cdvdFreeze()
 
 void cdvdNewDiskCB()
 {
-	ScopedTryLock lock(Mutex_NewDiskCB);
+	Threading::ScopedTryLock lock(Mutex_NewDiskCB);
 	if (lock.Failed())
 	{
 		DevCon.WriteLn(Color_Red, L"NewDiskCB Lock Failed");

pcsx2/DebugTools/DebugInterface.cpp

@@ -27,6 +27,10 @@
 #include "IopMem.h"
 #include "SymbolMap.h"
 
+#ifndef PCSX2_CORE
+#include "System/SysThreads.h"
+#endif
+
 R5900DebugInterface r5900Debug;
 R3000DebugInterface r3000Debug;
 

pcsx2/GS.h

@@ -16,10 +16,11 @@
 #pragma once
 
 #include "Common.h"
-#include "System/SysThreads.h"
 #include "Gif.h"
 #include "GS/GS.h"
+#include <atomic>
 #include <functional>
+#include <thread>
 
 extern double GetVerticalFrequency();
 alignas(16) extern u8 g_RealGSMem[Ps2MemSize::GSregs];
@@ -316,10 +317,8 @@ struct MTGS_MemoryScreenshotData
 // --------------------------------------------------------------------------------------
 //  SysMtgsThread
 // --------------------------------------------------------------------------------------
-class SysMtgsThread : public SysThreadBase
+class SysMtgsThread
 {
-	typedef SysThreadBase _parent;
-
 public:
 	using AsyncCallType = std::function<void()>;
 
@@ -334,10 +333,11 @@ public:
 	std::atomic<int>	m_QueuedFrameCount;
 	std::atomic<bool>	m_VsyncSignalListener;
 
-	Mutex			m_mtx_RingBufferBusy2; // Gets released on semaXGkick waiting...
+	Threading::Mutex m_mtx_RingBufferBusy2; // Gets released on semaXGkick waiting...
-	Mutex			m_mtx_WaitGS;
+	Threading::Mutex m_mtx_WaitGS;
-	Semaphore		m_sem_OnRingReset;
+	Threading::WorkSema m_sem_event;
-	Semaphore		m_sem_Vsync;
+	Threading::Semaphore m_sem_OnRingReset;
+	Threading::Semaphore m_sem_Vsync;
 
 	// used to keep multiple threads from sending packets to the ringbuffer concurrently.
 	// (currently not used or implemented -- is a planned feature for a future threaded VU1)
@@ -347,9 +347,6 @@ public:
 	// has more than one command in it when the thread is kicked.
 	int				m_CopyDataTally;
 
-	Semaphore			m_sem_OpenDone;
-	std::atomic<bool>	m_Opened;
-
 	// These vars maintain instance data for sending Data Packets.
 	// Only one data packet can be constructed and uploaded at a time.
 
@@ -361,10 +358,25 @@ public:
 	Threading::Mutex m_lock_Stack;
 #endif
 
+	std::thread m_thread;
+	Threading::ThreadHandle m_thread_handle;
+	std::atomic_bool m_open_flag{false};
+	std::atomic_bool m_shutdown_flag{false};
+	Threading::KernelSemaphore m_open_or_close_done;
+
 public:
 	SysMtgsThread();
 	virtual ~SysMtgsThread();
 
+	__fi const Threading::ThreadHandle& GetThreadHandle() const { return m_thread_handle; }
+	__fi bool IsOpen() const { return m_open_flag.load(std::memory_order_acquire); }
+
+	/// Starts the thread, if it hasn't already been started.
+	void StartThread();
+
+	/// Fully stops the thread, closing in the process if needed.
+	void ShutdownThread();
+
 	// Waits for the GS to empty out the entire ring buffer contents.
 	void WaitGS(bool syncRegs=true, bool weakWait=false, bool isMTVU=false);
 	void ResetGS();
@@ -374,6 +386,7 @@ public:
 	void SendDataPacket();
 	void SendGameCRC( u32 crc );
 	bool WaitForOpen();
+	void WaitForClose();
 	void Freeze( FreezeAction mode, MTGS_FreezeData& data );
 
 	void SendSimpleGSPacket( MTGS_RingCommand type, u32 offset, u32 size, GIF_PATH path );
@@ -385,8 +398,6 @@ public:
 	void PostVsyncStart(bool registers_written);
 	void InitAndReadFIFO(u8* mem, u32 qwc);
 
-	bool IsGSOpened() const { return m_Opened; }
-
 	void RunOnGSThread(AsyncCallType func);
 	void ApplySettings();
 	void ResizeDisplayWindow(int width, int height, float scale);
@@ -398,22 +409,16 @@ public:
 	bool SaveMemorySnapshot(u32 width, u32 height, std::vector<u32>* pixels);
 
 protected:
-	void OpenGS();
+	bool TryOpenGS();
 	void CloseGS();
 
-	void OnStart() override;
+	void ThreadEntryPoint();
-	void OnResumeReady() override;
+	void MainLoop();
-
-	void OnSuspendInThread() override;
-	void OnPauseInThread(SystemsMask systemsToTearDown) override {}
-	void OnResumeInThread(SystemsMask systemsToReinstate) override;
-	void OnCleanupInThread() override;
 
 	void GenericStall( uint size );
 
 	// Used internally by SendSimplePacket type functions
 	void _FinishSimplePacket();
-	void ExecuteTaskInThread() override;
 };
 
 // GetMTGS() is a required external implementation. This function is *NOT* provided

pcsx2/MTGS.cpp

@@ -61,20 +61,10 @@ std::list<uint> ringposStack;
 #endif
 
 SysMtgsThread::SysMtgsThread()
-	: SysThreadBase()
 #ifdef RINGBUF_DEBUG_STACK
-	, m_lock_Stack()
+	: m_lock_Stack()
 #endif
 {
-	m_name = L"MTGS";
-
-	// All other state vars are initialized by OnStart().
-}
-
-void SysMtgsThread::OnStart()
-{
-	m_Opened = false;
-
 	m_ReadPos = 0;
 	m_WritePos = 0;
 	m_packet_size = 0;
@@ -86,22 +76,85 @@ void SysMtgsThread::OnStart()
 	m_SignalRingPosition = 0;
 
 	m_CopyDataTally = 0;
-
-	_parent::OnStart();
 }
 
 SysMtgsThread::~SysMtgsThread()
 {
-	try
+	ShutdownThread();
-	{
-		_parent::Cancel();
-	}
-	DESTRUCTOR_CATCHALL
 }
 
-void SysMtgsThread::OnResumeReady()
+void SysMtgsThread::StartThread()
 {
-	m_sem_OpenDone.Reset();
+	if (m_thread.joinable())
+		return;
+
+	pxAssertRel(!m_open_flag.load(), "GS thread should not be opened when starting");
+	m_sem_event.Reset();
+	m_shutdown_flag.store(false, std::memory_order_release);
+	m_thread = std::thread(&SysMtgsThread::ThreadEntryPoint, this);
+}
+
+void SysMtgsThread::ShutdownThread()
+{
+	if (!m_thread.joinable())
+		return;
+
+	// just go straight to shutdown, don't wait-for-open again
+	m_shutdown_flag.store(true, std::memory_order_release);
+	if (IsOpen())
+		WaitForClose();
+
+	// make sure the thread actually exits
+	m_sem_event.NotifyOfWork();
+	m_thread.join();
+}
+
+void SysMtgsThread::ThreadEntryPoint()
+{
+	m_thread_handle = Threading::ThreadHandle::GetForCallingThread();
+	Threading::SetNameOfCurrentThread("GS");
+
+	for (;;)
+	{
+		// wait until we're actually asked to initialize (and config has been loaded, etc)
+		while (!m_open_flag.load(std::memory_order_acquire))
+		{
+			if (m_shutdown_flag.load(std::memory_order_acquire))
+			{
+				m_sem_event.Kill();
+				m_thread_handle = {};
+				return;
+			}
+
+			m_sem_event.WaitForWork();
+		}
+		
+		// try initializing.. this could fail
+		const bool opened = TryOpenGS();
+		m_open_flag.store(opened, std::memory_order_release);
+
+		// notify emu thread that we finished opening (or failed)
+		m_open_or_close_done.Post();
+
+		// are we open?
+		if (!opened)
+		{
+			// wait until we're asked to try again...
+			continue;
+		}
+
+		// we're ready to go
+		MainLoop();
+
+		// when we come back here, it's because we closed (or shutdown)
+		// that means the emu thread should be blocked, waiting for us to be done
+		pxAssertRel(!m_open_flag.load(std::memory_order_relaxed), "Open flag is clear on close");
+		CloseGS();
+		m_open_or_close_done.Post();
+
+		// we need to reset sem_event here, because MainLoop() kills it.
+		m_sem_event.Reset();
+	}
 }
 
 void SysMtgsThread::ResetGS()
@@ -203,26 +256,18 @@ union PacketTagType
 	};
 };
 
-void SysMtgsThread::OpenGS()
+bool SysMtgsThread::TryOpenGS()
 {
-	if (m_Opened)
+	std::memcpy(RingBuffer.Regs, PS2MEM_GS, sizeof(PS2MEM_GS));
-		return;
 
-	memcpy(RingBuffer.Regs, PS2MEM_GS, sizeof(PS2MEM_GS));
+	if (!GSopen(EmuConfig.GS, EmuConfig.GS.Renderer, RingBuffer.Regs))
-
+		return false;
-	m_Opened = GSopen(EmuConfig.GS, EmuConfig.GS.Renderer, RingBuffer.Regs);
-	m_sem_OpenDone.Post();
-
-	if (!m_Opened)
-	{
-		Console.Error("GS failed to open");
-		return;
-	}
 
 	GSsetGameCRC(ElfCRC, 0);
+	return true;
 }
 
-void SysMtgsThread::ExecuteTaskInThread()
+void SysMtgsThread::MainLoop()
 {
 	// Threading info: run in MTGS thread
 	// m_ReadPos is only update by the MTGS thread so it is safe to load it with a relaxed atomic
@@ -231,7 +276,6 @@ void SysMtgsThread::ExecuteTaskInThread()
 	PacketTagType prevCmd;
 #endif
 
-	ScopedGuard kill_on_exception([this]{ m_sem_event.Kill(); });
 	ScopedLock mtvu_lock(m_mtx_RingBufferBusy2);
 
 	while (true)
@@ -244,7 +288,8 @@ void SysMtgsThread::ExecuteTaskInThread()
 		m_mtx_RingBufferBusy2.Release();
 
 		m_sem_event.WaitForWork();
-		StateCheckInThread();
+		if (!m_open_flag.load(std::memory_order_acquire))
+			break;
 
 		m_mtx_RingBufferBusy2.Acquire();
 
@@ -516,52 +561,28 @@ void SysMtgsThread::ExecuteTaskInThread()
 
 		//Console.Warning( "(MTGS Thread) Nothing to do!  ringpos=0x%06x", m_ReadPos );
 	}
+
+	// Unblock any threads in WaitGS in case MTGS gets cancelled while still processing work
+	m_ReadPos.store(m_WritePos.load(std::memory_order_acquire), std::memory_order_relaxed);
+	m_sem_event.Kill();
 }
 
 void SysMtgsThread::CloseGS()
 {
-	if (!m_Opened)
-		return;
 #ifndef PCSX2_CORE
 	if (GSDump::isRunning)
 		return;
 #endif
-	m_Opened = false;
 	GSclose();
 }
 
-void SysMtgsThread::OnSuspendInThread()
-{
-	CloseGS();
-	_parent::OnSuspendInThread();
-}
-
-void SysMtgsThread::OnResumeInThread(SystemsMask systemsToReinstate)
-{
-	if (systemsToReinstate & System_GS)
-		OpenGS();
-
-	_parent::OnResumeInThread(systemsToReinstate);
-}
-
-void SysMtgsThread::OnCleanupInThread()
-{
-	CloseGS();
-	// Unblock any threads in WaitGS in case MTGS gets cancelled while still processing work
-	m_ReadPos.store(m_WritePos.load(std::memory_order_acquire), std::memory_order_relaxed);
-	_parent::OnCleanupInThread();
-}
-
 // Waits for the GS to empty out the entire ring buffer contents.
 // If syncRegs, then writes pcsx2's gs regs to MTGS's internal copy
 // If weakWait, then this function is allowed to exit after MTGS finished a path1 packet
 // If isMTVU, then this implies this function is being called from the MTVU thread...
 void SysMtgsThread::WaitGS(bool syncRegs, bool weakWait, bool isMTVU)
 {
-	pxAssertDev(!IsSelf(), "This method is only allowed from threads *not* named MTGS.");
+	pxAssertDev(std::this_thread::get_id() != m_thread.get_id(), "This method is only allowed from threads *not* named MTGS.");
-
-	if (m_ExecMode == ExecMode_NoThreadYet || !IsRunning())
-		return;
 	if (!pxAssertDev(IsOpen(), "MTGS Warning!  WaitGS issued on a closed thread."))
 		return;
 
@@ -585,7 +606,6 @@ void SysMtgsThread::WaitGS(bool syncRegs, bool weakWait, bool isMTVU)
 			while (true)
 			{
 				m_mtx_RingBufferBusy2.Wait();
-				RethrowException();
 				if (path.GetPendingGSPackets() != startP1Packs)
 					break;
 			}
@@ -594,16 +614,7 @@ void SysMtgsThread::WaitGS(bool syncRegs, bool weakWait, bool isMTVU)
 	else
 	{
 		if (!m_sem_event.WaitForEmpty())
-		{
+			pxFailRel("MTGS Thread Died");
-			// There's a small race here as the semaphore is killed before the exception is set
-			// Try a few times to recover the actual exception before throwing something more generic
-			for (int i = 0; i < 5; i++)
-			{
-				std::this_thread::yield();
-				RethrowException();
-			}
-			throw Exception::RuntimeError(std::runtime_error("MTGS Thread Died"));
-		}
 	}
 
 	if (syncRegs)
@@ -836,54 +847,51 @@ void SysMtgsThread::SendGameCRC(u32 crc)
 
 bool SysMtgsThread::WaitForOpen()
 {
-	if (m_Opened)
+	if (IsOpen())
 		return true;
-	Resume();
 
-	// Two-phase timeout on MTGS opening, so that possible errors are handled
+	StartThread();
-	// in a timely fashion.  We check for errors after 2 seconds, and then give it
+
-	// another 12 seconds if no errors occurred (this might seem long, but sometimes our
+	// request open, and kick the thread.
-	// GS can be very stubborned, especially in debug mode builds).
+	m_open_flag.store(true, std::memory_order_release);
+	m_sem_event.NotifyOfWork();
+
+	// wait for it to finish its stuff
+	m_open_or_close_done.Wait();
+
+	// did we succeed?
+	const bool result = m_open_flag.load(std::memory_order_acquire);
+	if (!result)
+		Console.Error("GS failed to open.");
 
 #ifndef PCSX2_CORE
-	if (!m_sem_OpenDone.Wait(wxTimeSpan(0, 0, 2, 0)))
+	if (!result) // EE thread will continue running and explode everything if we don't throw an exception
-	{
-		RethrowException();
-
-		if (!m_sem_OpenDone.Wait(wxTimeSpan(0, 0, 12, 0)))
-		{
-			RethrowException();
-
-			pxAssert(_("The MTGS thread has become unresponsive while waiting for GS to open."));
-		}
-	}
-
-	RethrowException();
-	if (!m_Opened) // EE thread will continue running and explode everything if we don't throw an exception
 		throw Exception::RuntimeError(std::runtime_error("GS failed to open."));
-	return m_Opened;
-#else
-	if (!m_sem_OpenDone.Wait(wxTimeSpan(0, 0, 12, 0)) || !m_Opened)
-	{
-		Suspend(false);
-		return false;
-	}
-
-	return true;
 #endif
+
+	return result;
+}
+
+void SysMtgsThread::WaitForClose()
+{
+	if (!IsOpen())
+		return;
+
+	// ask the thread to stop processing work, by clearing the open flag
+	m_open_flag.store(false, std::memory_order_release);
+
+	// and kick the thread if it's sleeping
+	m_sem_event.NotifyOfWork();
+
+	// and wait for it to finish up..
+	m_open_or_close_done.Wait();
 }
 
 void SysMtgsThread::Freeze(FreezeAction mode, MTGS_FreezeData& data)
 {
-	pxAssertDev(!IsSelf(), "This method is only allowed from threads *not* named MTGS.");
+	pxAssertRel(IsOpen(), "GS thread is open");
+	pxAssertDev(std::this_thread::get_id() != m_thread.get_id(), "This method is only allowed from threads *not* named MTGS.");
 	SendPointerPacket(GS_RINGTYPE_FREEZE, (int)mode, &data);
-	// make sure MTGS is processing the packet we send it
-	Resume();
-	// we are forced to wait for the semaphore to be released, otherwise
-	// we'll end up in a state where the main thread is stuck on WaitGS
-	// and MTGS stuck on sApp.OpenGSPanel, which post an event to the main
-	// thread. Obviously this ends up in a deadlock. -- govanify
-	WaitForOpen();
 	WaitGS();
 }
 

pcsx2/PerformanceMetrics.cpp

@@ -118,7 +118,7 @@ void PerformanceMetrics::Reset()
 	s_last_frame_time.Reset();
 
 	s_last_cpu_time = s_cpu_thread_handle.GetCPUTime();
-	s_last_gs_time = GetMTGS().GetCpuTime();
+	s_last_gs_time = GetMTGS().GetThreadHandle().GetCPUTime();
 	s_last_vu_time = THREAD_VU1 ? vu1Thread.GetThreadHandle().GetCPUTime() : 0;
 	s_last_ticks = GetCPUTicks();
 
@@ -183,7 +183,7 @@ void PerformanceMetrics::Update(bool gs_register_write, bool fb_blit)
 								(1.0 / static_cast<double>(s_frames_since_last_update));
 
 	const u64 cpu_time = s_cpu_thread_handle.GetCPUTime();
-	const u64 gs_time = GetMTGS().GetCpuTime();
+	const u64 gs_time = GetMTGS().GetThreadHandle().GetCPUTime();
 	const u64 vu_time = THREAD_VU1 ? vu1Thread.GetThreadHandle().GetCPUTime() : 0;
 
 	const u64 cpu_delta = cpu_time - s_last_cpu_time;

pcsx2/System/SysCoreThread.cpp

@@ -212,7 +212,7 @@ void SysCoreThread::ApplySettings(const Pcsx2Config& src)
 // --------------------------------------------------------------------------------------
 bool SysCoreThread::HasPendingStateChangeRequest() const
 {
-	return !m_hasActiveMachine || GetMTGS().HasPendingException() || _parent::HasPendingStateChangeRequest();
+	return !m_hasActiveMachine || _parent::HasPendingStateChangeRequest();
 }
 
 void SysCoreThread::_reset_stuff_as_needed()
@@ -299,7 +299,6 @@ void SysCoreThread::GameStartingInThread()
 
 bool SysCoreThread::StateCheckInThread()
 {
-	GetMTGS().RethrowException();
 	return _parent::StateCheckInThread() && (_reset_stuff_as_needed(), true);
 }
 
@@ -376,12 +375,12 @@ void SysCoreThread::OnCleanupInThread()
 	DoCDVDclose();
 	FWclose();
 	FileMcd_EmuClose();
-	GetMTGS().Suspend();
+	GetMTGS().WaitForClose();
 	USBshutdown();
 	SPU2shutdown();
 	PADshutdown();
 	DEV9shutdown();
-	GetMTGS().Cancel();
+	GetMTGS().ShutdownThread();
 
 	_mm_setcsr(m_mxcsr_saved.bitmask);
 	Threading::DisableHiresScheduler();

pcsx2/VMManager.cpp

@@ -673,7 +673,7 @@ bool VMManager::Initialize(const VMBootParameters& boot_params)
 		return false;
 	}
 
-	ScopedGuard close_gs = []() { GetMTGS().Suspend(); };
+	ScopedGuard close_gs = []() { GetMTGS().WaitForClose(); };
 
 	Console.WriteLn("Opening SPU2...");
 	if (SPU2init() != 0 || SPU2open() != 0)
@@ -827,14 +827,11 @@ void VMManager::Shutdown(bool allow_save_resume_state /* = true */)
 	DoCDVDclose();
 	FWclose();
 	FileMcd_EmuClose();
-	GetMTGS().Suspend();
+	GetMTGS().WaitForClose();
 	USBshutdown();
 	SPU2shutdown();
 	PADshutdown();
 	DEV9shutdown();
-
-	// GS mess here...
-	GetMTGS().Cancel();
 	GSshutdown();
 
 	s_vm_memory->DecommitAll();

pcsx2/gui/AppInit.cpp

@@ -717,11 +717,6 @@ Pcsx2App::Pcsx2App()
 Pcsx2App::~Pcsx2App()
 {
 	pxDoAssert = pxAssertImpl_LogIt;
-	try
-	{
-		vu1Thread.Cancel();
-	}
-	DESTRUCTOR_CATCHALL
 }
 
 void Pcsx2App::CleanUp()