From 490f58ff3c916e2afcf3bf5981dd9d7893d3923e Mon Sep 17 00:00:00 2001
From: Nekotekina <nekotekina@gmail.com>
Date: Fri, 28 Feb 2020 20:45:27 +0300
Subject: [PATCH] Try to purge thread_state::detached

It's rarely necessary, but can cause unexpected problems.
---
 Utilities/Thread.cpp                  |  6 ++++--
 Utilities/Thread.h                    | 27 +++++++--------------------
 rpcs3/Emu/Cell/PPUThread.cpp          |  6 ------
 rpcs3/Emu/Cell/PPUThread.h            |  2 --
 rpcs3/Emu/Cell/lv2/sys_ppu_thread.cpp | 21 ++++++++++++++++++++-
 5 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/Utilities/Thread.cpp b/Utilities/Thread.cpp
index 461f84edf3..d8660003a8 100644
--- a/Utilities/Thread.cpp
+++ b/Utilities/Thread.cpp
@@ -1858,11 +1858,13 @@ bool thread_base::finalize(int) noexcept
 		fsoft, fhard, ctxvol, ctxinv);
 
 	// Return true if need to delete thread object
-	const bool result = m_state.exchange(thread_state::finished) == thread_state::detached;
+	const bool ok = m_state.exchange(thread_state::finished) <= thread_state::aborting;
 
 	// Signal waiting threads
 	m_state.notify_all();
-	return result;
+
+	// No detached thread supported atm
+	return !ok;
 }
 
 void thread_base::finalize() noexcept
diff --git a/Utilities/Thread.h b/Utilities/Thread.h
index 05d6995026..553faa84c2 100644
--- a/Utilities/Thread.h
+++ b/Utilities/Thread.h
@@ -38,8 +38,7 @@ enum class thread_class : u32
 enum class thread_state : u32
 {
 	created,  // Initial state
-	detached, // The thread has been detached to destroy its own named_thread object (can be dangerously misused)
-	aborting, // The thread has been joined in the destructor or explicitly aborted (mutually exclusive with detached)
+	aborting, // The thread has been joined in the destructor or explicitly aborted
 	finished  // Final state, always set at the end of thread execution
 };
 
@@ -82,13 +81,6 @@ struct result_storage<void>
 template <class Context, typename... Args>
 using result_storage_t = result_storage<std::invoke_result_t<Context, Args...>>;
 
-// Detect on_cleanup() static member function (should return void) (in C++20 can use destroying delete instead)
-template <typename T, typename = void>
-struct thread_on_cleanup : std::bool_constant<false> {};
-
-template <typename T>
-struct thread_on_cleanup<T, decltype(named_thread<T>::on_cleanup(std::declval<named_thread<T>*>()))> : std::bool_constant<true> {};
-
 template <typename T, typename = void>
 struct thread_thread_name : std::bool_constant<false> {};
 
@@ -305,15 +297,7 @@ class named_thread final : public Context, result_storage_t<Context>, thread_bas
 		// Perform self-cleanup if necessary
 		if (_this->entry_point())
 		{
-			// Call on_cleanup() static member function if it's available
-			if constexpr (thread_on_cleanup<Context>())
-			{
-				Context::on_cleanup(_this);
-			}
-			else
-			{
-				delete _this;
-			}
+			delete _this;
 		}
 
 		thread::finalize();
@@ -426,10 +410,12 @@ public:
 		return thread::m_state.load();
 	}
 
-	// Try to abort/detach
+	// Try to abort by assigning thread_state::aborting (UB if assigning different state)
 	named_thread& operator=(thread_state s)
 	{
-		if (s < thread_state::finished && thread::m_state.compare_and_swap_test(thread_state::created, s))
+		ASSUME(s == thread_state::aborting);
+
+		if (s == thread_state::aborting && thread::m_state.compare_and_swap_test(thread_state::created, s))
 		{
 			if (s == thread_state::aborting)
 			{
@@ -443,6 +429,7 @@ public:
 	// Context type doesn't need virtual destructor
 	~named_thread()
 	{
+		// Assign aborting state forcefully
 		operator=(thread_state::aborting);
 		thread::join();
 
diff --git a/rpcs3/Emu/Cell/PPUThread.cpp b/rpcs3/Emu/Cell/PPUThread.cpp
index df10d5af22..458633ca28 100644
--- a/rpcs3/Emu/Cell/PPUThread.cpp
+++ b/rpcs3/Emu/Cell/PPUThread.cpp
@@ -424,12 +424,6 @@ extern bool ppu_patch(u32 addr, u32 value)
 	return true;
 }
 
-void ppu_thread::on_cleanup(named_thread<ppu_thread>* _this)
-{
-	// Remove thread id
-	idm::remove<named_thread<ppu_thread>>(_this->id);
-}
-
 std::string ppu_thread::dump() const
 {
 	std::string ret = cpu_thread::dump();
diff --git a/rpcs3/Emu/Cell/PPUThread.h b/rpcs3/Emu/Cell/PPUThread.h
index e076e433d8..dbcd3b5874 100644
--- a/rpcs3/Emu/Cell/PPUThread.h
+++ b/rpcs3/Emu/Cell/PPUThread.h
@@ -44,8 +44,6 @@ public:
 	static const u32 id_step = 1;
 	static const u32 id_count = 2048;
 
-	static void on_cleanup(named_thread<ppu_thread>*);
-
 	virtual std::string dump() const override;
 	virtual void cpu_task() override final;
 	virtual void cpu_sleep() override;
diff --git a/rpcs3/Emu/Cell/lv2/sys_ppu_thread.cpp b/rpcs3/Emu/Cell/lv2/sys_ppu_thread.cpp
index d6ef445525..e689c00b99 100644
--- a/rpcs3/Emu/Cell/lv2/sys_ppu_thread.cpp
+++ b/rpcs3/Emu/Cell/lv2/sys_ppu_thread.cpp
@@ -42,7 +42,26 @@ void _sys_ppu_thread_exit(ppu_thread& ppu, u64 errorcode)
 	if (jid == umax)
 	{
 		// Detach detached thread, id will be removed on cleanup
-		static_cast<named_thread<ppu_thread>&>(ppu) = thread_state::detached;
+		static thread_local struct cleanup_t
+		{
+			const u32 id;
+
+			cleanup_t(u32 id)
+				: id(id)
+			{
+			}
+
+			cleanup_t(const cleanup_t&) = delete;
+
+			~cleanup_t()
+			{
+				if (!idm::remove<named_thread<ppu_thread>>(id))
+				{
+					sys_ppu_thread.fatal("Failed to remove detached thread! (id=0x%x)", id);
+				}
+			}
+		}
+		to_cleanup(ppu.id);
 	}
 	else if (jid != 0)
 	{