Use std::thread instead of a custom cross-platform thread API

Properly cleanup threads and resources

Name all emulation threads

src/cli/main.cpp

@@ -3,6 +3,7 @@
 
 #include "openxbox/core.h"
 #include "openxbox/settings.h"
+#include "openxbox/thread.h"
 
 // Debugging stuff
 #define DUMP_XBE_INFO 0
@@ -27,6 +28,8 @@ char *basename(char *path)
  * Program entry point
  */
 int main(int argc, const char *argv[]) {
+    openxbox::Thread_SetName("[Core] Main Thread");
+
 	using namespace openxbox;
 
 	auto info = GetOpenXBOXInfo();

src/common/openxbox/log.h

@@ -11,7 +11,7 @@ namespace openxbox {
 #define LOG_LEVEL_DEBUG   (5)
 #define LOG_LEVEL_SPEW    (6)
 
-#define LOG_LEVEL LOG_LEVEL_DEBUG
+#define LOG_LEVEL LOG_LEVEL_SPEW
 
 #if 1
 #define log_fatal(...)   log_print(LOG_LEVEL_FATAL,   __VA_ARGS__)

src/common/openxbox/thread.h

@@ -4,45 +4,9 @@
 
 namespace openxbox {
 
-// TODO: thread groups
-
 /*!
- * Opaque thread structure.
+ * Sets the current thread's name.
  */
-struct Thread;
-
-/*!
- * Thread function.
- */
-typedef uint32_t (*ThreadFunc)(void *data);
-
-/*!
- * Creates and starts a new thread with the given name.
- */
-Thread *Thread_Create(char *name, ThreadFunc func, void *data);
-
-/*!
- * Joins a thread, waiting for it to exit.
- * Returns the exit code when the target thread has exited.
- * The thread object is freed upon returning.
- */
-uint32_t Thread_Join(Thread *thread);
-
-/*!
- * Waits for all threads created with the API to exit.
- */
-void Thread_JoinAll();
-
-/*!
- * Terminates the current thread with the given exit code.
- * The corresponding Thread object is freed.
- * Has no effect on threads that were not created by the Thread API.
- */
-void Thread_Exit(uint32_t exitCode);
-
-/*!
- * Returns a reference to the current thread.
- */
-Thread *Thread_Self();
+void Thread_SetName(char *name);
 
 }

src/common/openxbox/win32/thread.cpp

@@ -11,154 +11,28 @@ namespace openxbox {
 const DWORD MS_VC_EXCEPTION = 0x406D1388;
 
 #pragma pack(push,8)
-typedef struct tagTHREADNAME_INFO
-{
-	DWORD dwType; // Must be 0x1000.
-	LPCSTR szName; // Pointer to name (in user addr space).
-	DWORD dwThreadID; // Thread ID (-1=caller thread).
-	DWORD dwFlags; // Reserved for future use, must be zero.
+typedef struct tagTHREADNAME_INFO {
+    DWORD dwType; // Must be 0x1000.
+    LPCSTR szName; // Pointer to name (in user addr space).
+    DWORD dwThreadID; // Thread ID (-1=caller thread).
+    DWORD dwFlags; // Reserved for future use, must be zero.
 } THREADNAME_INFO;
 #pragma pack(pop)
 
-void SetThreadName(DWORD dwThreadID, char* threadName)
-{
-	THREADNAME_INFO info;
-	info.dwType = 0x1000;
-	info.szName = threadName;
-	info.dwThreadID = dwThreadID;
-	info.dwFlags = 0;
+void Thread_SetName(char *threadName) {
+    THREADNAME_INFO info;
+    info.dwType = 0x1000;
+    info.szName = threadName;
+    info.dwThreadID = GetCurrentThreadId();
+    info.dwFlags = 0;
 
-	__try
-	{
-		RaiseException(MS_VC_EXCEPTION, 0, sizeof(info) / sizeof(ULONG_PTR), (ULONG_PTR*)&info);
-	}
-	__except (EXCEPTION_EXECUTE_HANDLER)
-	{
-	}
-}
-
-struct Thread {
-	HANDLE hThread;
-	DWORD dwThreadId;
-	uint32_t refCount;
-};
-
-static std::map<DWORD, Thread *> g_threads;
-
-inline void _Thread_Add(Thread *thread) {
-	thread->refCount = 0;
-	g_threads[thread->dwThreadId] = thread;
-}
-
-inline void _Thread_Ref(Thread *thread) {
-	thread->refCount++;
-}
-
-inline void _Thread_Unref(Thread *thread) {
-	if (thread == nullptr) {
-		return;
-	}
-	thread->refCount--;
-	if (thread->refCount == 0) {
-		g_threads.erase(thread->dwThreadId);
-		delete thread;
-	}
-}
-
-struct ThreadParams {
-	ThreadFunc func;
-	void *data;
-};
-
-DWORD WINAPI Thread_Routine(LPVOID param) {
-	ThreadParams *params = (ThreadParams *)param;
-
-	// Make a local copy so we can free the memory
-	ThreadParams localParams = *params;
-	delete params;
-
-	DWORD result = (DWORD)localParams.func(localParams.data);
-	_Thread_Unref(Thread_Self());
-	return result;
-}
-
-Thread *Thread_Create(char *name, ThreadFunc func, void *data) {
-	assert(func != nullptr);
-	ThreadParams *params = new ThreadParams{ func, data };
-
-	Thread *thread = new Thread;
-	thread->hThread = CreateThread(NULL, 0, Thread_Routine, params, CREATE_SUSPENDED, &thread->dwThreadId);
-	if (thread->hThread == INVALID_HANDLE_VALUE) {
-		delete thread;
-		return nullptr;
-	}
-	_Thread_Add(thread);
-	_Thread_Ref(thread);
-	SetThreadName(thread->dwThreadId, name);
-	ResumeThread(thread->hThread);
-
-	return thread;
-}
-
-uint32_t Thread_Join(Thread *thread) {
-	assert(thread != nullptr);
-	_Thread_Ref(thread);
-
-	DWORD result;
-	do {
-		result = WaitForSingleObject(thread->hThread, INFINITE);
-		if (result == WAIT_OBJECT_0) {
-			DWORD exitCode;
-			if (GetExitCodeThread(thread->hThread, &exitCode)) {
-				_Thread_Unref(thread);
-				return exitCode;
-			}
-			else {
-				assert(0); // FIXME: something is wrong
-			}
-		}
-		// TODO: what to do if the wait fails?
-	} while (result != WAIT_OBJECT_0);
-
-	assert(0); // FIXME: should never reach here
-	return -1;
-}
-
-void Thread_Exit(uint32_t exitCode) {
-	Thread *self = Thread_Self();
-	assert(self != nullptr);
-
-	HANDLE hThread = self->hThread;
-	_Thread_Unref(self);
-	TerminateThread(hThread, exitCode);
-}
-
-Thread *Thread_Self() {
-	DWORD dwThreadId = GetCurrentThreadId();
-
-	return (g_threads.count(dwThreadId))
-		? g_threads[dwThreadId]
-		: nullptr;
-}
-
-void Thread_JoinAll() {
-	std::map<DWORD, Thread *> threads(g_threads);
-	
-	HANDLE *hThreads = new HANDLE[threads.size()];
-	int count = 0;
-	for (auto it = threads.begin(); it != threads.end(); it++, count++) {
-		hThreads[count] = it->second->hThread;
-	}
-
-	for (int i = 0; i < count; i += 64) {
-		DWORD countMax = min(64, count - i);
-		DWORD result = WaitForMultipleObjects(countMax, &hThreads[i], TRUE, INFINITE);
-		// TODO: handle failures gracefully
-	}
-
-	delete[] hThreads;
+    __try {
+        RaiseException(MS_VC_EXCEPTION, 0, sizeof(info) / sizeof(ULONG_PTR), (ULONG_PTR*)&info);
+    }
+    __except (EXCEPTION_EXECUTE_HANDLER) {
+    }
 }
 
 }
 
-#endif
+#endif // _WIN32

src/core/openxbox/hw/basic/char.h

@@ -45,6 +45,7 @@ public:
     virtual bool Init() = 0;
     virtual int Write(const uint8_t *buf, int len) = 0;
     virtual void AcceptInput() = 0;
+    virtual void Stop() = 0;
 
     // IOCTLs
     virtual void SetBreakEnable(bool breakEnable) = 0;

src/core/openxbox/hw/basic/char_null.cpp

@@ -16,6 +16,10 @@ void NullCharDriver::AcceptInput() {
     // Nothing to do
 }
 
+void NullCharDriver::Stop() {
+    // Nothing to do
+}
+
 // ----- IOCTLs ---------------------------------------------------------------
 
 void NullCharDriver::SetBreakEnable(bool breakEnable) {

src/core/openxbox/hw/basic/char_null.h

@@ -11,6 +11,7 @@ public:
     bool Init() override;
     int Write(const uint8_t *buf, int len) override;
     void AcceptInput() override;
+    void Stop() override;
 
     // IOCTLs
     void SetBreakEnable(bool breakEnable) override;

src/core/openxbox/hw/basic/i8254.cpp

@@ -11,6 +11,7 @@ namespace openxbox {
 
 // i8254 timer thread function
 static uint32_t i8254ThreadFunc(void *data) {
+    Thread_SetName("[HW] i8254");
     i8254 *pit = (i8254 *)data;
     pit->Run();
     return 0;
@@ -23,6 +24,11 @@ i8254::i8254(i8259 *pic, float tickRate)
 {
 }
 
+i8254::~i8254() {
+    m_running = false;
+    m_timerThread.join();
+}
+
 void i8254::Reset() {
     m_running = false;
 }
@@ -45,7 +51,7 @@ bool i8254::IOWrite(uint32_t port, uint32_t value, uint8_t size) {
     // start operating, and then simply issue IRQ 0 in a timer thread.
     if (value == 0x34) {
         m_running = true;
-        Thread_Create("[HW] i8254", i8254ThreadFunc, this);
+        m_timerThread = std::thread(i8254ThreadFunc, this);
     }
     return true;
 }

src/core/openxbox/hw/basic/i8254.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <cstdint>
+#include <thread>
 
 #include "i8259.h"
 #include "openxbox/io.h"
@@ -18,6 +19,7 @@ namespace openxbox {
 class i8254 : public IODevice {
 public:
     i8254(i8259 *pic, float tickRate = 1000.0f);
+    ~i8254();
     void Reset();
     
     bool MapIO(IOMapper *mapper);
@@ -30,6 +32,8 @@ private:
     i8259 *m_pic;
     float m_tickRate;
     bool m_running;
+
+    std::thread m_timerThread;
 };
 
 }

src/core/openxbox/hw/basic/serial.cpp

@@ -118,10 +118,10 @@ Serial::~Serial() {
     m_fifoTimeoutTimer->Stop();
     m_modemStatusPoll->Stop();
 
-    delete m_recvFifo;
-    delete m_xmitFifo;
     delete m_fifoTimeoutTimer;
     delete m_modemStatusPoll;
+    delete m_recvFifo;
+    delete m_xmitFifo;
 }
 
 bool Serial::Init(CharDriver *chr) {
@@ -167,6 +167,10 @@ void Serial::Reset() {
     m_lastBreakEnable = 0;
 }
 
+void Serial::Stop() {
+    m_chr->Stop();
+}
+
 bool Serial::MapIO(IOMapper *mapper) {
     if (!mapper->MapIODevice(m_ioBase, PORT_SERIAL_COUNT, this)) return false;
     

src/core/openxbox/hw/basic/serial.h

@@ -27,6 +27,7 @@ public:
 
     bool Init(CharDriver *chr);
     void Reset();
+    void Stop();
     
     inline void SetIRQ(uint8_t irq) { m_irq = irq; }
     inline void SetBaudBase(int baudBase) { m_baudbase = baudBase; }

src/core/openxbox/hw/basic/superio.cpp

@@ -28,6 +28,14 @@ SuperIO::SuperIO(i8259 *pic, CharDriver *chrs[SUPERIO_SERIAL_PORT_COUNT]) {
     }
 }
 
+SuperIO::~SuperIO() {
+    // Shutdown serial ports
+    for (int i = 0; i < SUPERIO_SERIAL_PORT_COUNT; i++) {
+        m_serialPorts[i]->Stop();
+        delete m_serialPorts[i];
+    }
+}
+
 void SuperIO::Init() {
 }
 

src/core/openxbox/hw/basic/superio.h

@@ -44,6 +44,7 @@ namespace openxbox {
 class SuperIO : public IODevice {
 public:
     SuperIO(i8259 *pic, CharDriver *chrs[SUPERIO_SERIAL_PORT_COUNT]);
+    ~SuperIO();
 
     void Init();
 	void Reset();

src/core/openxbox/hw/basic/win32/char_serial.cpp

@@ -1,3 +1,5 @@
+#ifdef _WIN32
+
 #include "char_serial.h"
 
 namespace openxbox {
@@ -54,6 +56,10 @@ void Win32SerialDriver::AcceptInput() {
     // TODO: implement
 }
 
+void Win32SerialDriver::Stop() {
+    Close();
+}
+
 void Win32SerialDriver::Close() {
     m_comm->Stop();
 
@@ -129,3 +135,5 @@ void Win32SerialDriver::CommEvent(SerialCommEvent evt) {
 }
 
 }
+
+#endif // _WIN32

src/core/openxbox/hw/basic/win32/char_serial.h

@@ -16,6 +16,7 @@ public:
     bool Init() override;
     int Write(const uint8_t *buf, int len) override;
     void AcceptInput() override;
+    void Stop() override;
 
     // IOCTLs
     void SetBreakEnable(bool breakEnable) override;

src/core/openxbox/hw/basic/win32/mt_serial.cpp

@@ -1,8 +1,12 @@
+#ifdef _WIN32
+
 #include "mt_serial.h"
 
 #include <cstdio>
 #include <thread>
 
+#include "openxbox/thread.h"
+
 namespace openxbox {
 
 #define WRITE_QUEUE_SIZE 4096
@@ -237,10 +241,18 @@ void SerialComm::Write(const uint8_t *buf, uint32_t len) {
 }
 
 void SerialComm::ReaderAndEventsThreadProc(SerialComm *instance) {
+    char threadName[27];
+    sprintf(threadName, "[HW] COM%u Reader/Events", instance->m_portNum);
+    Thread_SetName(threadName);
+
     instance->ReaderAndEventsLoop();
 }
 
 void SerialComm::WriterThreadProc(SerialComm *instance) {
+    char threadName[20];
+    sprintf(threadName, "[HW] COM%u Writer", instance->m_portNum);
+    Thread_SetName(threadName);
+
     instance->WriterLoop();
 }
 
@@ -678,3 +690,5 @@ void SerialComm::WriterLoop() {
 }
 
 }
+
+#endif // _WIN32

src/core/openxbox/hw/pci/nv2a.cpp

@@ -1,5 +1,6 @@
 #include "nv2a.h"
 #include "openxbox/log.h"
+#include "openxbox/thread.h"
 
 #include <cassert>
 #include <cstring>
@@ -2297,6 +2298,8 @@ void NV2ADevice::pfifo_run_pusher() {
 }
 
 void NV2ADevice::PFIFO_Puller_Thread(NV2ADevice *nv2a) {
+    Thread_SetName("[HW] NV2A PFIFO Puller");
+
     Cache1State *state = &nv2a->m_PFIFO.cache1;
     while (nv2a->m_running) {
         // Scope the lock so that it automatically unlocks at tne end of this block
@@ -2417,6 +2420,8 @@ void NV2ADevice::UpdateIRQ() {
 }
 
 void NV2ADevice::VBlankThread(NV2ADevice *nv2a) {
+    Thread_SetName("[HW] NV2A VBlank");
+
     using namespace std::chrono;
     auto nextStop = high_resolution_clock::now();
     auto interval = duration<long long, std::ratio<1, 1000000>>((long long)(1000000.0f / 60.0f));

src/core/openxbox/xbox.cpp

@@ -54,6 +54,7 @@ const static uint8_t kDefaultEEPROM[] = {
 
 // CPU emulation thread function
 static uint32_t EmuCpuThreadFunc(void *data) {
+    Thread_SetName("[HW] CPU");
     Xbox *xbox = (Xbox *)data;
     return xbox->RunCpu();
 }
@@ -355,19 +356,14 @@ void Xbox::InitializePreRun() {
 int Xbox::Run() {
 	m_should_run = true;
 
-	// --- CPU emulation ------------------------------------------------------
-
 	// Start CPU emulation on a new thread
-	Thread *cpuIdleThread = Thread_Create("[HW] CPU", EmuCpuThreadFunc, this);
+    uint32_t result;
+    std::thread cpuIdleThread([&] { result = EmuCpuThreadFunc(this); });
 
-	// --- Emulator subsystems ------------------------------------------------
+    // Wait for the thread to exit
+	cpuIdleThread.join();
 
-	// TODO: start threads to handle other subsystems
-	// - One or more for each piece of hardware
-	//   - NV2A
-	//   - MCPX (multiple components)
-
-	return Thread_Join(cpuIdleThread);
+    return result;
 }
 
 /*!
@@ -380,7 +376,7 @@ int Xbox::RunCpu()
 	struct CpuExitInfo *exit_info;
 
 	if (!m_should_run) {
-		Thread_Exit(-1);
+        return -1;
 	}
 
 	while (m_should_run) {