Skip to content

Commit

Permalink
named_thread: fix bugs in std::forward usage
Browse files Browse the repository at this point in the history
Fix few misused threads.
  • Loading branch information
Nekotekina committed Mar 1, 2021
1 parent b20433d commit dc7dbc0
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 210 deletions.
277 changes: 74 additions & 203 deletions Utilities/Thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,74 @@ class thread_ctrl final
static const u64 process_affinity_mask;
};

// Used internally
template <bool Discard, typename Ctx, typename... Args>
class thread_future_t : public thread_future, result_storage<Ctx, std::conditional_t<Discard, int, void>, Args...>
{
[[no_unique_address]] decltype(std::make_tuple(std::forward<Args>(std::declval<Args>())...)) m_args;

[[no_unique_address]] Ctx m_func;

using future = thread_future_t;

public:
thread_future_t(Ctx&& func, Args&&... args)
: m_args(std::forward<Args>(args)...)
, m_func(std::forward<Ctx>(func))
{
thread_future::exec.raw() = +[](thread_base* tb, thread_future* tf)
{
const auto _this = static_cast<future*>(tf);

if (!tb) [[unlikely]]
{
if constexpr (!future::empty && !Discard)
{
_this->init();
}

return;
}

if constexpr (future::empty || Discard)
{
std::apply(_this->m_func, std::move(_this->m_args));
}
else
{
new (_this->_get()) decltype(auto)(std::apply(_this->m_func, std::move(_this->m_args)));
}
};
}

~thread_future_t()
{
if constexpr (!future::empty && !Discard)
{
if (!this->exec)
{
this->destroy();
}
}
}

decltype(auto) get()
{
if constexpr (!future::empty && !Discard)
{
return *this->_get();
}
}

decltype(auto) get() const
{
if constexpr (!future::empty && !Discard)
{
return *this->_get();
}
}
};

// Derived from the callable object Context, possibly a lambda
template <class Context>
class named_thread final : public Context, result_storage<Context>, thread_base
Expand Down Expand Up @@ -467,74 +535,9 @@ class named_thread final : public Context, result_storage<Context>, thread_base

if constexpr (v1)
{
class future : public thread_future, result_storage<Context, void, Arg, Args...>
{
// A tuple to store arguments
decltype(std::make_tuple(std::forward<Arg, Args...>(arg, args...))) m_args;

public:
future(Arg&& arg, Args&&... args)
: m_args(std::forward<Arg, Args...>(arg, args...))
{
thread_future::exec.raw() = +[](thread_base* tb, thread_future* tf)
{
const auto _this = static_cast<future*>(tf);

if (!tb) [[unlikely]]
{
if constexpr (!future::empty && !Discard)
{
_this->init();
}

return;
}

if constexpr (future::empty || Discard)
{
std::apply(*static_cast<Context*>(static_cast<named_thread*>(tb)), _this->m_args);
}
else
{
new (_this->_get()) decltype(auto)(std::apply(*static_cast<Context*>(static_cast<named_thread*>(tb)), _this->m_args));
}
};
}
using future = thread_future_t<Discard, Context&, Arg, Args...>;

future(const future&) = delete;

future& operator=(const future&) = delete;

~future()
{
if constexpr (!future::empty && !Discard)
{
// Should be set to null if executed
if (!this->exec)
{
this->destroy();
}
}
}

decltype(auto) get()
{
if constexpr (!future::empty && !Discard)
{
return *this->_get();
}
}

decltype(auto) get() const
{
if constexpr (!future::empty && !Discard)
{
return *this->_get();
}
}
};

single_ptr<future> target = make_single<future>(std::forward<Arg, Args...>(arg, args...));
single_ptr<future> target = make_single<future>(*static_cast<Context*>(this), std::forward<Arg>(arg), std::forward<Args>(args)...);

if constexpr (!Discard)
{
Expand All @@ -553,75 +556,9 @@ class named_thread final : public Context, result_storage<Context>, thread_base
}
else if constexpr (v2)
{
class future : public thread_future, result_storage<std::decay_t<Arg>, void, Args...>
{
decltype(std::make_tuple(std::forward<Args...>(args...))) m_args;

std::decay_t<Arg> m_func;

public:
future(Arg func, Args&&... args)
: m_args(std::forward<Args...>(args...))
, m_func(func)
{
thread_future::exec.raw() = +[](thread_base* tb, thread_future* tf)
{
const auto _this = static_cast<future*>(tf);

if (!tb) [[unlikely]]
{
if constexpr (!future::empty && !Discard)
{
_this->init();
}

return;
}

if constexpr (future::empty || Discard)
{
std::apply(_this->m_func, _this->m_args);
}
else
{
new (_this->_get()) decltype(auto)(std::apply(_this->m_func, _this->m_args));
}
};
}

future(const future&) = delete;
using future = thread_future_t<Discard, Arg, Args...>;

future& operator=(const future&) = delete;

~future()
{
if constexpr (!future::empty && !Discard)
{
if (!this->exec)
{
this->destroy();
}
}
}

decltype(auto) get()
{
if constexpr (!future::empty && !Discard)
{
return *this->_get();
}
}

decltype(auto) get() const
{
if constexpr (!future::empty && !Discard)
{
return *this->_get();
}
}
};

single_ptr<future> target = make_single<future>(std::forward<Arg, Args...>(arg, args...));
single_ptr<future> target = make_single<future>(std::forward<Arg>(arg), std::forward<Args>(args)...);

if constexpr (!Discard)
{
Expand All @@ -637,75 +574,9 @@ class named_thread final : public Context, result_storage<Context>, thread_base
}
else if constexpr (v3)
{
class future : public thread_future, result_storage<std::decay_t<Arg>, void, Context&, Args...>
{
decltype(std::make_tuple(std::forward<Args...>(args...))) m_args;

std::decay_t<Arg> m_func;

public:
future(Arg func, Args&&... args)
: m_args(std::forward<Args...>(args...))
, m_func(func)
{
thread_future::exec.raw() = +[](thread_base* tb, thread_future* tf)
{
const auto _this = static_cast<future*>(tf);

if (!tb) [[unlikely]]
{
if constexpr (!future::empty && !Discard)
{
_this->init();
}

return;
}

if constexpr (future::empty || Discard)
{
std::apply(_this->m_func, *static_cast<Context*>(static_cast<named_thread*>(tb)), _this->m_args);
}
else
{
new (_this->_get()) decltype(auto)(std::apply(_this->m_func, *static_cast<Context*>(static_cast<named_thread*>(tb)), _this->m_args));
}
};
}

future(const future&) = delete;

future& operator=(const future&) = delete;

~future()
{
if constexpr (!future::empty && !Discard)
{
if (!this->exec)
{
this->destroy();
}
}
}

decltype(auto) get()
{
if constexpr (!future::empty && !Discard)
{
return *this->_get();
}
}

decltype(auto) get() const
{
if constexpr (!future::empty && !Discard)
{
return *this->_get();
}
}
};
using future = thread_future_t<Discard, Arg, Context&, Args...>;

single_ptr<future> target = make_single<future>(std::forward<Arg, Args...>(arg, args...));
single_ptr<future> target = make_single<future>(std::forward<Arg>(arg), *static_cast<Context*>(this), std::forward<Args>(args)...);

if constexpr (!Discard)
{
Expand Down
7 changes: 6 additions & 1 deletion rpcs3/Emu/RSX/Overlays/overlay_message_dialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,11 @@ namespace rsx
close(true, true);
}

struct msg_dialog_thread
{
static constexpr auto thread_name = "MsgDialog Thread"sv;
};

error_code message_dialog::show(bool is_blocking, const std::string& text, const MsgDialogType& type, std::function<void(s32 status)> on_close)
{
visible = false;
Expand Down Expand Up @@ -246,7 +251,7 @@ namespace rsx
{
if (!exit)
{
g_fxo->init<named_thread>("MsgDialog Thread", [&, tbit = alloc_thread_bit()]()
g_fxo->get<named_thread<msg_dialog_thread>>()->operator()([&, tbit = alloc_thread_bit()]()
{
g_thread_bit = tbit;

Expand Down
7 changes: 6 additions & 1 deletion rpcs3/Emu/RSX/Overlays/overlay_osk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -781,6 +781,11 @@ namespace rsx
return m_cached_resource;
}

struct osk_dialog_thread
{
static constexpr auto thread_name = "OSK Thread"sv;
};

void osk_dialog::Create(const std::string& title, const std::u16string& message, char16_t* init_text, u32 charlimit, u32 prohibit_flags, u32 panel_flag, u32 first_view_panel)
{
state = OskDialogState::Open;
Expand Down Expand Up @@ -1012,7 +1017,7 @@ namespace rsx

update_panel();

g_fxo->init<named_thread>("OSK Thread", [this, tbit = alloc_thread_bit()]
g_fxo->get<named_thread<osk_dialog_thread>>()->operator()([this, tbit = alloc_thread_bit()]
{
g_thread_bit = tbit;

Expand Down
11 changes: 6 additions & 5 deletions rpcs3/util/shared_ptr.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ namespace stx
#pragma GCC diagnostic push
#ifdef __clang__
#pragma GCC diagnostic ignored "-Wundefined-var-template"
#pragma GCC diagnostic ignored "-Wundefined-internal"
#endif
#endif

Expand All @@ -21,7 +22,7 @@ namespace stx
template <typename T, typename U>
constexpr bool is_same_ptr() noexcept
{
#if !defined(_MSC_VER) && !defined(__clang__)
#ifdef _MSC_VER
return true;
#else
if constexpr (std::is_void_v<T> || std::is_void_v<U> || std::is_same_v<T, U>)
Expand All @@ -30,14 +31,14 @@ namespace stx
}
else if constexpr (std::is_convertible_v<U*, T*>)
{
const auto u = std::addressof(sample<U>);
const volatile void* x = u;
constexpr auto u = std::addressof(sample<U>);
constexpr volatile void* x = u;
return static_cast<T*>(u) == x;
}
else if constexpr (std::is_convertible_v<T*, U*>)
{
const auto t = std::addressof(sample<T>);
const volatile void* x = t;
constexpr auto t = std::addressof(sample<T>);
constexpr volatile void* x = t;
return static_cast<U*>(t) == x;
}
else
Expand Down

0 comments on commit dc7dbc0

Please sign in to comment.