Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bugs related to lingering spu/ppu thread copies #13964

Merged
merged 6 commits into from
Jun 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion Utilities/StrFmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ std::vector<std::string> fmt::split(std::string_view source, std::initializer_li
return result;
}

std::string fmt::trim(const std::string& source, const std::string& values)
std::string fmt::trim(const std::string& source, std::string_view values)
{
usz begin = source.find_first_not_of(values);

Expand All @@ -599,6 +599,12 @@ std::string fmt::trim(const std::string& source, const std::string& values)
return source.substr(begin, source.find_last_not_of(values) + 1);
}

void fmt::trim_back(std::string& source, std::string_view values)
{
const usz index = source.find_last_not_of(values);
source.resize(index + 1);
}

std::string fmt::to_upper(std::string_view string)
{
std::string result;
Expand Down
3 changes: 2 additions & 1 deletion Utilities/StrUtil.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ namespace fmt
}

std::vector<std::string> split(std::string_view source, std::initializer_list<std::string_view> separators, bool is_skip_empty = true);
std::string trim(const std::string& source, const std::string& values = " \t");
std::string trim(const std::string& source, std::string_view values = " \t");
void trim_back(std::string& source, std::string_view values = " \t");

template <typename T>
std::string merge(const T& source, const std::string& separator)
Expand Down
4 changes: 2 additions & 2 deletions rpcs3/Crypto/unpkg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ bool package_reader::read_header()
}

pkg_log.notice("Path: '%s'", m_path);
pkg_log.notice("Header: pkg_magic = 0x%x = \"%s\"", +m_header.pkg_magic, std::string(reinterpret_cast<const char*>(&m_header.pkg_magic), 4));
pkg_log.notice("Header: pkg_magic = 0x%x = \"%s\"", +m_header.pkg_magic, std::string_view(reinterpret_cast<const char*>(&m_header.pkg_magic + 1), 3)); // Skip 0x7F
Megamouse marked this conversation as resolved.
Show resolved Hide resolved
pkg_log.notice("Header: pkg_type = 0x%x = %d", m_header.pkg_type, m_header.pkg_type);
pkg_log.notice("Header: pkg_platform = 0x%x = %d", m_header.pkg_platform, m_header.pkg_platform);
pkg_log.notice("Header: meta_offset = 0x%x = %d", m_header.meta_offset, m_header.meta_offset);
Expand All @@ -94,7 +94,7 @@ bool package_reader::read_header()
return false;
}

pkg_log.notice("Extended header: magic = 0x%x = \"%s\"", +ext_header.magic, std::string(reinterpret_cast<const char*>(&ext_header.magic), 4));
pkg_log.notice("Extended header: magic = 0x%x = \"%s\"", +ext_header.magic, std::string_view(reinterpret_cast<const char*>(&ext_header.magic + 1), 3));
pkg_log.notice("Extended header: unknown_1 = 0x%x = %d", ext_header.unknown_1, ext_header.unknown_1);
pkg_log.notice("Extended header: ext_hdr_size = 0x%x = %d", ext_header.ext_hdr_size, ext_header.ext_hdr_size);
pkg_log.notice("Extended header: ext_data_size = 0x%x = %d", ext_header.ext_data_size, ext_header.ext_data_size);
Expand Down
5 changes: 4 additions & 1 deletion rpcs3/Emu/CPU/CPUThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1312,7 +1312,10 @@ bool cpu_thread::suspend_work::push(cpu_thread* _this) noexcept

void cpu_thread::cleanup() noexcept
{
ensure(!s_cpu_counter);
if (u64 count = s_cpu_counter)
{
fmt::throw_exception("cpu_thread::cleanup(): %u threads are still active! (created=%u, destroyed=%u)", count, +g_threads_created, +g_threads_deleted);
}

sys_log.notice("All CPU threads have been stopped. [+: %u]", +g_threads_created);

Expand Down
3 changes: 2 additions & 1 deletion rpcs3/Emu/Cell/PPUThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1153,7 +1153,8 @@ void ppu_thread::dump_regs(std::string& ret) const
}
}

fmt::append(ret, "\n");
fmt::trim_back(ret);
ret += '\n';
}

for (uint i = 0; i < 32; ++i)
Expand Down
7 changes: 2 additions & 5 deletions rpcs3/Emu/Cell/SPUThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1767,11 +1767,8 @@ void spu_thread::cleanup()
// Free range lock (and signals cleanup was called to the destructor)
vm::free_range_lock(range_lock);

// Signal the debugger about the termination
if (!state.test_and_set(cpu_flag::exit))
{
state.notify_one();
}
// Terminate and join thread
static_cast<named_thread<spu_thread>&>(*this) = thread_state::finished;
}

spu_thread::~spu_thread()
Expand Down
2 changes: 1 addition & 1 deletion rpcs3/Emu/Cell/lv2/lv2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1123,7 +1123,7 @@ class ppu_syscall_usage

if (!m_stats.empty())
{
ppu_log.notice("PPU Syscall Usage Stats: %s", m_stats);
ppu_log.notice("PPU Syscall Usage Stats:%s", m_stats);
}
}

Expand Down
3 changes: 3 additions & 0 deletions rpcs3/Emu/Cell/lv2/sys_interrupt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,9 @@ error_code _sys_interrupt_thread_disestablish(ppu_thread& ppu, u32 ih, vm::ptr<u
if (const auto thread = idm::withdraw<named_thread<ppu_thread>>(ih))
{
*r13 = thread->gpr[13];

// It is detached from IDM now so join must be done explicitly now
*thread = thread_state::finished;
return CELL_OK;
}

Expand Down
33 changes: 29 additions & 4 deletions rpcs3/Emu/Cell/lv2/sys_ppu_thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,19 @@ struct ppu_thread_cleaner
ppu_thread_cleaner(const ppu_thread_cleaner&) = delete;

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

ppu_thread_cleaner& operator=(thread_state state) noexcept
{
reader_lock lock(id_manager::g_mutex);

if (old)
{
// It is detached from IDM now so join must be done explicitly now
*static_cast<named_thread<ppu_thread>*>(old.get()) = state;
}

return *this;
}
};

void ppu_thread_exit(ppu_thread& ppu, ppu_opcode_t, be_t<u32>*, struct ppu_intrp_func*)
Expand Down Expand Up @@ -74,10 +87,10 @@ void _sys_ppu_thread_exit(ppu_thread& ppu, u64 errorcode)
sys_ppu_thread.trace("_sys_ppu_thread_exit(errorcode=0x%llx)", errorcode);

ppu_join_status old_status;
{
// Avoid cases where cleaning causes the destructor to be called inside IDM lock scope (for performance)
std::shared_ptr<void> old_ppu;

// Avoid cases where cleaning causes the destructor to be called inside IDM lock scope (for performance)
std::shared_ptr<void> old_ppu;
{
lv2_obj::notify_all_t notify;
lv2_obj::prepare_for_sleep(ppu);

Expand Down Expand Up @@ -130,6 +143,12 @@ void _sys_ppu_thread_exit(ppu_thread& ppu, u64 errorcode)
}

ppu_thread_exit(ppu, {}, nullptr, nullptr);

if (old_ppu)
{
// It is detached from IDM now so join must be done explicitly now
*static_cast<named_thread<ppu_thread>*>(old_ppu.get()) = thread_state::finished;
}
}

s32 sys_ppu_thread_yield(ppu_thread& ppu)
Expand Down Expand Up @@ -239,7 +258,7 @@ error_code sys_ppu_thread_detach(ppu_thread& ppu, u32 thread_id)

CellError result = CELL_ESRCH;

idm::withdraw<named_thread<ppu_thread>>(thread_id, [&](ppu_thread& thread)
auto [ptr, _] = idm::withdraw<named_thread<ppu_thread>>(thread_id, [&](ppu_thread& thread)
{
result = thread.joiner.atomic_op([](ppu_join_status& value) -> CellError
{
Expand Down Expand Up @@ -279,6 +298,12 @@ error_code sys_ppu_thread_detach(ppu_thread& ppu, u32 thread_id)

if (result)
{
if (result == CELL_EAGAIN)
{
// Join thread (it is detached from IDM now so it must be done explicitly now)
*ptr = thread_state::finished;
}

return result;
}

Expand Down
2 changes: 1 addition & 1 deletion rpcs3/rpcs3qt/kernel_explorer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,7 @@ void kernel_explorer::log(u32 level, QTreeWidgetItem* item)

for (u32 j = 0; j < level; j++)
{
m_log_buf += QChar::Nbsp;
m_log_buf += QChar::Space;
}

m_log_buf.append(item->text(0));
Expand Down
9 changes: 9 additions & 0 deletions rpcs3/rpcs3qt/register_editor_dialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <QHBoxLayout>
#include <QPushButton>
#include <QMessageBox>
#include <QCompleter>
#include <charconv>

#include "util/v128.hpp"
Expand Down Expand Up @@ -81,6 +82,14 @@ register_editor_dialog::register_editor_dialog(QWidget *parent, CPUDisAsm* _disa
button_cancel->setFixedWidth(80);

m_register_combo = new QComboBox(this);
m_register_combo->setMaxVisibleItems(20);
m_register_combo->setEditable(true);
m_register_combo->setInsertPolicy(QComboBox::NoInsert);
m_register_combo->lineEdit()->setPlaceholderText(tr("Search a register"));
m_register_combo->completer()->setCompletionMode(QCompleter::PopupCompletion);
m_register_combo->completer()->setMaxVisibleItems(20);
m_register_combo->completer()->setFilterMode(Qt::MatchContains);

m_value_line = new QLineEdit(this);
m_value_line->setFixedWidth(200);

Expand Down