Try to purge thread_state::detached

It's rarely necessary, but can cause unexpected problems.
This commit is contained in:
Nekotekina 2020-02-28 20:45:27 +03:00
parent cb047fcc75
commit 490f58ff3c
5 changed files with 31 additions and 31 deletions

View File

@ -1858,11 +1858,13 @@ bool thread_base::finalize(int) noexcept
fsoft, fhard, ctxvol, ctxinv); fsoft, fhard, ctxvol, ctxinv);
// Return true if need to delete thread object // 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 // Signal waiting threads
m_state.notify_all(); m_state.notify_all();
return result;
// No detached thread supported atm
return !ok;
} }
void thread_base::finalize() noexcept void thread_base::finalize() noexcept

View File

@ -38,8 +38,7 @@ enum class thread_class : u32
enum class thread_state : u32 enum class thread_state : u32
{ {
created, // Initial state 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
aborting, // The thread has been joined in the destructor or explicitly aborted (mutually exclusive with detached)
finished // Final state, always set at the end of thread execution finished // Final state, always set at the end of thread execution
}; };
@ -82,13 +81,6 @@ struct result_storage<void>
template <class Context, typename... Args> template <class Context, typename... Args>
using result_storage_t = result_storage<std::invoke_result_t<Context, 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> template <typename T, typename = void>
struct thread_thread_name : std::bool_constant<false> {}; 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 // Perform self-cleanup if necessary
if (_this->entry_point()) if (_this->entry_point())
{ {
// Call on_cleanup() static member function if it's available delete _this;
if constexpr (thread_on_cleanup<Context>())
{
Context::on_cleanup(_this);
}
else
{
delete _this;
}
} }
thread::finalize(); thread::finalize();
@ -426,10 +410,12 @@ public:
return thread::m_state.load(); 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) 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) if (s == thread_state::aborting)
{ {
@ -443,6 +429,7 @@ public:
// Context type doesn't need virtual destructor // Context type doesn't need virtual destructor
~named_thread() ~named_thread()
{ {
// Assign aborting state forcefully
operator=(thread_state::aborting); operator=(thread_state::aborting);
thread::join(); thread::join();

View File

@ -424,12 +424,6 @@ extern bool ppu_patch(u32 addr, u32 value)
return true; 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 ppu_thread::dump() const
{ {
std::string ret = cpu_thread::dump(); std::string ret = cpu_thread::dump();

View File

@ -44,8 +44,6 @@ public:
static const u32 id_step = 1; static const u32 id_step = 1;
static const u32 id_count = 2048; static const u32 id_count = 2048;
static void on_cleanup(named_thread<ppu_thread>*);
virtual std::string dump() const override; virtual std::string dump() const override;
virtual void cpu_task() override final; virtual void cpu_task() override final;
virtual void cpu_sleep() override; virtual void cpu_sleep() override;

View File

@ -42,7 +42,26 @@ void _sys_ppu_thread_exit(ppu_thread& ppu, u64 errorcode)
if (jid == umax) if (jid == umax)
{ {
// Detach detached thread, id will be removed on cleanup // 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) else if (jid != 0)
{ {