CPUThread fixes, thread_t cleanup

Utilities/File.cpp

@@ -19,7 +19,7 @@ std::unique_ptr<wchar_t[]> to_wchar(const std::string& source)
 
 	if (!MultiByteToWideChar(CP_UTF8, 0, source.c_str(), size, buffer.get(), size))
 	{
-		throw EXCEPTION("Conversion failed (0x%llx)", GET_API_ERROR);
+		throw EXCEPTION("System error 0x%x", GetLastError());
 	}
 
 	return buffer;
@@ -38,14 +38,14 @@ void to_utf8(std::string& result, const wchar_t* source)
 
 	if (size <= 0)
 	{
-		throw EXCEPTION("Conversion failed (0x%llx)", GET_API_ERROR);
+		throw EXCEPTION("System error 0x%x", GetLastError());
 	}
 
 	result.resize(size);
 
 	if (!WideCharToMultiByte(CP_UTF8, 0, source, length, &result.front(), size, NULL, NULL))
 	{
-		throw EXCEPTION("Conversion failed (0x%llx)", GET_API_ERROR);
+		throw EXCEPTION("System error 0x%x", GetLastError());
 	}
 }
 

Utilities/Thread.cpp

@@ -1307,14 +1307,11 @@ void thread_t::start(std::function<std::string()> name, std::function<void()> fu
 		}
 
 		//ctrl->set_current(false);
+
 		vm::reservation_free();
 
 		g_thread_count--;
 
-		ctrl->joinable = false;
-
-		ctrl->join_cv.notify_all();
-
 #if defined(_MSC_VER)
 		_set_se_translator(old_se_translator);
 #endif
@@ -1331,39 +1328,16 @@ void thread_t::detach()
 	// +clear m_thread
 	const auto ctrl = std::move(m_thread);
 
+	{
+		// lock for reliable notification
+		std::lock_guard<std::mutex> lock(mutex);
+
 		cv.notify_all();
+	}
 
 	ctrl->m_thread.detach();
 }
 
-void thread_t::join(std::unique_lock<std::mutex>& lock)
-{
-	if (!m_thread)
-	{
-		throw EXCEPTION("Invalid thread");
-	}
-
-	if (g_tls_this_thread == m_thread.get())
-	{
-		throw EXCEPTION("Deadlock");
-	}
-
-	// +clear m_thread
-	const auto ctrl = std::move(m_thread);
-
-	cv.notify_all();
-
-	// wait for completion
-	while (ctrl->joinable)
-	{
-		CHECK_EMU_STATUS;
-
-		ctrl->join_cv.wait_for(lock, std::chrono::milliseconds(1));
-	}
-
-	ctrl->m_thread.join();
-}
-
 void thread_t::join()
 {
 	if (!m_thread)
@@ -1379,7 +1353,12 @@ void thread_t::join()
 	// +clear m_thread
 	const auto ctrl = std::move(m_thread);
 
+	{
+		// lock for reliable notification
+		std::lock_guard<std::mutex> lock(mutex);
+
 		cv.notify_all();
+	}
 
 	ctrl->m_thread.join();
 }

Utilities/Thread.h

@@ -13,12 +13,6 @@ class thread_ctrl_t final
 	// name getter
 	const std::function<std::string()> name;
 
-	// condition variable, notified before thread exit
-	std::condition_variable join_cv;
-
-	// thread status (set to false after execution)
-	std::atomic<bool> joinable{ true };
-
 	// true if TLS of some thread points to owner
 	std::atomic<bool> assigned{ false };
 
@@ -71,9 +65,6 @@ public:
 	// detach thread -> empty state
 	void detach();
 
-	// join thread (provide locked unique_lock, for example, lv2_lock, for interruptibility) -> empty state
-	void join(std::unique_lock<std::mutex>& lock);
-
 	// join thread -> empty state
 	void join();
 

rpcs3/Emu/ARMv7/Modules/sceLibc.cpp

@@ -181,7 +181,7 @@ namespace sce_libc_func
 
 		CHECK_EMU_STATUS;
 
-		for (auto func : decltype(g_atexit)(std::move(g_atexit)))
+		for (auto& func : decltype(g_atexit)(std::move(g_atexit)))
 		{
 			func(context);
 		}

rpcs3/Emu/CPU/CPUThread.cpp

@@ -50,7 +50,6 @@ CPUThread::CPUThread(CPUThreadType type, const std::string& name, std::function<
 				}
 
 				m_state &= ~CPU_STATE_RETURN;
-				cv.notify_one();
 				continue;
 			}
 
@@ -94,28 +93,31 @@ void CPUThread::Run()
 	SendDbgCommand(DID_STARTED_THREAD, this);
 }
 
-void CPUThread::Resume()
-{
-	SendDbgCommand(DID_RESUME_THREAD, this);
-
-	m_state &= ~CPU_STATE_PAUSED;
-
-	cv.notify_one();
-
-	SendDbgCommand(DID_RESUMED_THREAD, this);
-}
-
 void CPUThread::Pause()
 {
 	SendDbgCommand(DID_PAUSE_THREAD, this);
 
 	m_state |= CPU_STATE_PAUSED;
 
-	cv.notify_one();
-
 	SendDbgCommand(DID_PAUSED_THREAD, this);
 }
 
+void CPUThread::Resume()
+{
+	SendDbgCommand(DID_RESUME_THREAD, this);
+
+	{
+		// lock for reliable notification
+		std::lock_guard<std::mutex> lock(mutex);
+
+		m_state &= ~CPU_STATE_PAUSED;
+
+		cv.notify_one();
+	}
+
+	SendDbgCommand(DID_RESUMED_THREAD, this);
+}
+
 void CPUThread::Stop()
 {
 	SendDbgCommand(DID_STOP_THREAD, this);
@@ -126,6 +128,9 @@ void CPUThread::Stop()
 	}
 	else
 	{
+		// lock for reliable notification
+		std::lock_guard<std::mutex> lock(mutex);
+
 		m_state |= CPU_STATE_STOPPED;
 
 		cv.notify_one();
@@ -138,10 +143,15 @@ void CPUThread::Exec()
 {
 	SendDbgCommand(DID_EXEC_THREAD, this);
 
+	{
+		// lock for reliable notification
+		std::lock_guard<std::mutex> lock(mutex);
+
 		m_state &= ~CPU_STATE_STOPPED;
 
 		cv.notify_one();
 	}
+}
 
 void CPUThread::Exit()
 {
@@ -162,21 +172,17 @@ void CPUThread::Step()
 		state |= CPU_STATE_STEP;
 		state &= ~CPU_STATE_PAUSED;
 	});
-
-	cv.notify_one();
 }
 
 void CPUThread::Sleep()
 {
 	m_state += CPU_STATE_MAX;
 	m_state |= CPU_STATE_SLEEP;
-
-	cv.notify_one();
 }
 
 void CPUThread::Awake()
 {
-	// must be called after the corresponding Sleep() call
+	// must be called after the balanced Sleep() call
 
 	m_state.atomic_op([](u64& state)
 	{
@@ -191,6 +197,9 @@ void CPUThread::Awake()
 		}
 	});
 
+	// lock for reliable notification because the condition being checked is probably externally set
+	std::lock_guard<std::mutex> lock(mutex);
+
 	cv.notify_one();
 }
 
@@ -206,7 +215,7 @@ bool CPUThread::CheckStatus()
 
 		if (!lock) lock.lock();
 
-		cv.wait_for(lock, std::chrono::milliseconds(1));
+		cv.wait(lock);
 	}
 
 	if (m_state.load() & CPU_STATE_RETURN || IsStopped())

rpcs3/Emu/SysCalls/Callback.cpp

@@ -110,4 +110,3 @@ void CallbackManager::Clear()
 
 	m_cb_thread.reset();
 }
-

rpcs3/Emu/SysCalls/Modules.cpp

@@ -24,8 +24,8 @@ u32 add_ppu_func(ModuleFunc func)
 	{
 		if (f.id == func.id)
 		{
-			// TODO: if NIDs overlap or if the same function is added twice
+			// if NIDs overlap or if the same function is added twice
-			assert(!"add_ppu_func(): NID already exists");
+			throw EXCEPTION("NID already exists: 0x%08x (%s)", f.id, f.name);
 		}
 	}
 
@@ -332,9 +332,7 @@ void hook_ppu_func(vm::ptr<u32> base, u32 pos, u32 size)
 			//}
 			default:
 			{
-				LOG_ERROR(LOADER, "Unknown search pattern type (%d)", sub.ops[x].type);
+				throw EXCEPTION("Unknown search pattern type (%d)", sub.ops[x].type);
-				assert(0);
-				return;
 			}
 			}
 
@@ -392,12 +390,13 @@ void hook_ppu_funcs(vm::ptr<u32> base, u32 size)
 				continue;
 			}
 
-			enum GroupSearchResult : u32
+			enum : u32
 			{
 				GSR_SUCCESS = 0, // every function from this group has been found once
 				GSR_MISSING = 1, // (error) some function not found
 				GSR_EXCESS = 2, // (error) some function found twice or more
 			};
+
 			u32 res = GSR_SUCCESS;
 
 			// analyse

rpcs3/Emu/SysCalls/Modules/cellAudio.cpp

@@ -42,11 +42,11 @@ s32 cellAudioInit()
 
 	// alloc memory (only once until the emulator is stopped)
 	g_audio.buffer = g_audio.buffer ? g_audio.buffer : Memory.MainMem.AllocAlign(AUDIO_PORT_OFFSET * AUDIO_PORT_COUNT, 4096);
-	g_audio.indexes = g_audio.indexes ? g_audio.indexes : Memory.MainMem.AllocAlign(sizeof(u64) * AUDIO_PORT_COUNT, __alignof(u64));
+	g_audio.indexes = g_audio.indexes ? g_audio.indexes : Memory.MainMem.AllocAlign(sizeof32(u64) * AUDIO_PORT_COUNT, alignof32(u64));
 
 	// clear memory
 	memset(vm::get_ptr<void>(g_audio.buffer), 0, AUDIO_PORT_OFFSET * AUDIO_PORT_COUNT);
-	memset(vm::get_ptr<void>(g_audio.indexes), 0, sizeof(u64) * AUDIO_PORT_COUNT);
+	memset(vm::get_ptr<void>(g_audio.indexes), 0, sizeof32(u64) * AUDIO_PORT_COUNT);
 
 	// check thread status
 	if (g_audio.thread.joinable())

rpcs3/Emu/SysCalls/lv2/sys_spu.cpp

@@ -1133,6 +1133,8 @@ s32 sys_raw_spu_create(vm::ptr<u32> id, vm::ptr<void> attr)
 
 	LV2_LOCK;
 
+	// TODO: check number set by sys_spu_initialize()
+
 	const auto thread = Emu.GetCPU().NewRawSPUThread();
 
 	if (!thread)

rpcs3/Emu/System.cpp

@@ -293,7 +293,7 @@ void Emulator::Pause()
 	// update pause start time
 	if (m_pause_start_time.exchange(start))
 	{
-		LOG_ERROR(GENERAL, "Pause(): Concurrent access");
+		LOG_ERROR(GENERAL, "Emulator::Pause() error: concurrent access");
 	}
 
 	SendDbgCommand(DID_PAUSE_EMU);
@@ -308,8 +308,10 @@ void Emulator::Pause()
 
 void Emulator::Resume()
 {
+	// get pause start time
 	const u64 time = m_pause_start_time.exchange(0);
 
+	// try to increment summary pause time
 	if (time)
 	{
 		m_pause_amend_time += get_system_time() - time;
@@ -323,14 +325,14 @@ void Emulator::Resume()
 
 	if (!time)
 	{
-		LOG_ERROR(GENERAL, "Resume(): Concurrent access");
+		LOG_ERROR(GENERAL, "Emulator::Resume() error: concurrent access");
 	}
 
 	SendDbgCommand(DID_RESUME_EMU);
 
 	for (auto& t : GetCPU().GetAllThreads())
 	{
-		t->Awake(); // untrigger status check
+		t->Awake(); // untrigger status check and signal
 	}
 
 	SendDbgCommand(DID_RESUMED_EMU);
@@ -349,9 +351,14 @@ void Emulator::Stop()
 
 	SendDbgCommand(DID_STOP_EMU);
 
+	{
+		LV2_LOCK;
+
+		// notify all threads
 		for (auto& t : GetCPU().GetAllThreads())
 		{
-		t->Sleep(); // trigger status check
+			t->Stop(); // signal / trigger status check
+		}
 	}
 
 	while (g_thread_count)