From f41770f57a58d044fd730229a095e98bd6f0bc41 Mon Sep 17 00:00:00 2001 From: Elad <18193363+elad335@users.noreply.github.com> Date: Tue, 24 Mar 2026 19:47:32 +0200 Subject: [PATCH] Thread.cpp: Fix game exit on access violation Fixes game closing in Twisted Metal. This commit also aloows to debug such states by using cpu_flag::req_exit as a broker --- Utilities/Thread.cpp | 71 +++++++++++++++++++++++++----- rpcs3/Emu/CPU/CPUThread.cpp | 12 ++++- rpcs3/Emu/CPU/CPUThread.h | 3 +- rpcs3/Emu/Cell/Modules/cellGem.cpp | 6 +++ rpcs3/Emu/System.cpp | 14 ++++-- rpcs3/rpcs3qt/debugger_frame.cpp | 11 +++-- rpcs3/rpcs3qt/main_window.cpp | 4 +- 7 files changed, 99 insertions(+), 22 deletions(-) diff --git a/Utilities/Thread.cpp b/Utilities/Thread.cpp index 6395c32505..3458a2ac1f 100644 --- a/Utilities/Thread.cpp +++ b/Utilities/Thread.cpp @@ -1269,7 +1269,7 @@ namespace rsx extern std::function g_access_violation_handler; } -bool handle_access_violation(u32 addr, bool is_writing, ucontext_t* context) noexcept +bool handle_access_violation(u32 addr, bool is_writing, bool is_exec, ucontext_t* context) noexcept { g_tls_fault_all++; @@ -1503,7 +1503,9 @@ bool handle_access_violation(u32 addr, bool is_writing, ucontext_t* context) noe static_cast(context); #endif /* ARCH_ */ - if (vm::check_addr(addr, is_writing ? vm::page_writable : vm::page_readable)) + const auto requred_page_perms = (is_writing ? vm::page_writable : vm::page_readable) + (is_exec ? vm::page_executable : 0); + + if (vm::check_addr(addr, requred_page_perms)) { return true; } @@ -1511,9 +1513,7 @@ bool handle_access_violation(u32 addr, bool is_writing, ucontext_t* context) noe // Hack: allocate memory in case the emulator is stopping const auto hack_alloc = [&]() { - g_tls_access_violation_recovered = true; - - if (vm::check_addr(addr, is_writing ? vm::page_writable : vm::page_readable)) + if (vm::check_addr(addr, requred_page_perms)) { return true; } @@ -1525,14 +1525,42 @@ bool handle_access_violation(u32 addr, bool is_writing, ucontext_t* context) noe return false; } + extern void ppu_register_range(u32 addr, u32 size); + + bool reprotected = false; + if (vm::writer_lock mlock; area->flags & vm::preallocated || vm::check_addr(addr, 0)) { // For allocated memory with protection lower than required (such as protection::no or read-only while writing to it) utils::memory_protect(vm::base(addr & -0x1000), 0x1000, utils::protection::rw); + reprotected = true; + } + + if (reprotected) + { + if (is_exec && !vm::check_addr(addr, vm::page_executable)) + { + ppu_register_range(addr & -0x10000, 0x10000); + } + + g_tls_access_violation_recovered = true; return true; } - return area->falloc(addr & -0x10000, 0x10000) || vm::check_addr(addr, is_writing ? vm::page_writable : vm::page_readable); + const bool allocated = area->falloc(addr & -0x10000, 0x10000); + + if (allocated) + { + if (is_exec && !vm::check_addr(addr, vm::page_executable)) + { + ppu_register_range(addr & -0x10000, 0x10000); + } + + g_tls_access_violation_recovered = true; + return true; + } + + return false; }; if (cpu && (cpu->get_class() == thread_class::ppu || cpu->get_class() == thread_class::spu)) @@ -1766,8 +1794,13 @@ bool handle_access_violation(u32 addr, bool is_writing, ucontext_t* context) noe } } - if (Emu.IsStopped() && !hack_alloc()) + if (Emu.IsStopped()) { + while (!hack_alloc()) + { + thread_ctrl::wait_for(1000); + } + return false; } @@ -1806,6 +1839,7 @@ static LONG exception_handler(PEXCEPTION_POINTERS pExp) noexcept if (pExp->ExceptionRecord->ExceptionCode == EXCEPTION_ACCESS_VIOLATION && !is_executing) { u32 addr = 0; + bool is_exec = false; if (auto [addr0, ok] = vm::try_get_addr(ptr); ok) { @@ -1813,14 +1847,21 @@ static LONG exception_handler(PEXCEPTION_POINTERS pExp) noexcept } else if (const usz exec64 = (ptr - vm::g_exec_addr) / 2; exec64 <= u32{umax}) { + is_exec = true; addr = static_cast(exec64); } - else + else if (const usz exec64 = (ptr - vm::g_exec_addr - vm::g_exec_addr_seg_offset); exec64 <= u32{umax}) { + is_exec = true; + addr = static_cast(exec64); + } + else + { + std::this_thread::sleep_for(1ms); return EXCEPTION_CONTINUE_SEARCH; } - if (thread_ctrl::get_current() && handle_access_violation(addr, is_writing, pExp->ContextRecord)) + if (thread_ctrl::get_current() && handle_access_violation(addr, is_writing, is_exec, pExp->ContextRecord)) { return EXCEPTION_CONTINUE_EXECUTION; } @@ -2027,12 +2068,13 @@ static void signal_handler(int /*sig*/, siginfo_t* info, void* uct) noexcept #endif const u64 exec64 = (reinterpret_cast(info->si_addr) - reinterpret_cast(vm::g_exec_addr)) / 2; + const u64 exec64_2 = (reinterpret_cast(info->si_addr) - reinterpret_cast(vm::g_exec_addr)) - vm::g_exec_addr_seg_offset; const auto cause = is_executing ? "executing" : is_writing ? "writing" : "reading"; if (auto [addr, ok] = vm::try_get_addr(info->si_addr); ok && !is_executing) { // Try to process access violation - if (thread_ctrl::get_current() && handle_access_violation(addr, is_writing, context)) + if (thread_ctrl::get_current() && handle_access_violation(addr, is_writing, false, context)) { return; } @@ -2040,7 +2082,14 @@ static void signal_handler(int /*sig*/, siginfo_t* info, void* uct) noexcept if (exec64 < 0x100000000ull && !is_executing) { - if (thread_ctrl::get_current() && handle_access_violation(static_cast(exec64), is_writing, context)) + if (thread_ctrl::get_current() && handle_access_violation(static_cast(exec64), is_writing, true, context)) + { + return; + } + } + else if (exec64_2 < 0x100000000ull && !is_executing) + { + if (thread_ctrl::get_current() && handle_access_violation(static_cast(exec64_2), is_writing, true, context)) { return; } diff --git a/rpcs3/Emu/CPU/CPUThread.cpp b/rpcs3/Emu/CPU/CPUThread.cpp index eb6f3498a2..4bd5fc9157 100644 --- a/rpcs3/Emu/CPU/CPUThread.cpp +++ b/rpcs3/Emu/CPU/CPUThread.cpp @@ -888,6 +888,14 @@ bool cpu_thread::check_state() noexcept store = true; } + if (flags & cpu_flag::req_exit) + { + // A request for the thread to quit has been made + flags -= cpu_flag::req_exit; + flags += cpu_flag::exit; + store = true; + } + // Can't process dbg_step if we only paused temporarily if (cpu_can_stop && flags & cpu_flag::dbg_step) { @@ -1157,13 +1165,13 @@ void cpu_thread::notify() cpu_thread& cpu_thread::operator=(thread_state) { - if (state & cpu_flag::exit) + if (state & (cpu_flag::exit + cpu_flag::req_exit)) { // Must be notified elsewhere or self-raised return *this; } - const auto old = state.fetch_add(cpu_flag::exit); + const auto old = state.fetch_add(cpu_flag::req_exit); if (old & cpu_flag::wait && old.none_of(cpu_flag::again + cpu_flag::exit)) { diff --git a/rpcs3/Emu/CPU/CPUThread.h b/rpcs3/Emu/CPU/CPUThread.h index 88995f5e3b..e723fd2d4b 100644 --- a/rpcs3/Emu/CPU/CPUThread.h +++ b/rpcs3/Emu/CPU/CPUThread.h @@ -29,6 +29,7 @@ enum class cpu_flag : u32 yield, // Thread is being requested to yield its execution time if it's running preempt, // Thread is being requested to preempt the execution of all CPU threads + req_exit, // Request the thread to exit dbg_global_pause, // Emulation paused dbg_pause, // Thread paused dbg_step, // Thread forced to pause after one step (one instruction, etc) @@ -39,7 +40,7 @@ enum class cpu_flag : u32 // Test stopped state constexpr bool is_stopped(bs_t state) { - return !!(state & (cpu_flag::stop + cpu_flag::exit + cpu_flag::again)); + return !!(state & (cpu_flag::stop + cpu_flag::exit + cpu_flag::again + cpu_flag::req_exit)); } // Test paused state diff --git a/rpcs3/Emu/Cell/Modules/cellGem.cpp b/rpcs3/Emu/Cell/Modules/cellGem.cpp index d45dace1ca..f9f5ea4100 100644 --- a/rpcs3/Emu/Cell/Modules/cellGem.cpp +++ b/rpcs3/Emu/Cell/Modules/cellGem.cpp @@ -1774,6 +1774,12 @@ public: shared_mutex mutex; + gem_tracker& operator=(thread_state) noexcept + { + wake_up_tracker(); + return *this; + } + private: atomic_t m_wake_up_tracker = 0; atomic_t m_tracker_done = 0; diff --git a/rpcs3/Emu/System.cpp b/rpcs3/Emu/System.cpp index 42854504f7..7818bd20d5 100644 --- a/rpcs3/Emu/System.cpp +++ b/rpcs3/Emu/System.cpp @@ -49,6 +49,7 @@ #include #include +#include #include "Utilities/JIT.h" @@ -3075,7 +3076,7 @@ void Emulator::GracefulShutdown(bool allow_autoexit, bool async_op, bool savesta std::vector>> ppu_thread_list; // If EXITGAME signal is not read, force kill after a second. - constexpr int loop_timeout_ms = 50; + constexpr int loop_timeout_ms = 16; int kill_timeout_ms = 1000; int elapsed_ms = 0; @@ -3092,8 +3093,10 @@ void Emulator::GracefulShutdown(bool allow_autoexit, bool async_op, bool savesta Resume(); }, nullptr, true, read_counter); + std::shared_lock rlock(id_manager::g_mutex, std::defer_lock); + // Check if the EXITGAME signal was read. We allow the game to terminate itself if that's the case. - if (!read_sysutil_signal && read_counter != get_sysutil_cb_manager_read_count()) + if (!read_sysutil_signal && read_counter != get_sysutil_cb_manager_read_count() && rlock.try_lock()) { sys_log.notice("The game received the exit request. Waiting for it to terminate itself..."); kill_timeout_ms += 5000; // Grant a couple more seconds @@ -3103,7 +3106,12 @@ void Emulator::GracefulShutdown(bool allow_autoexit, bool async_op, bool savesta idm::select>([&](u32 id, cpu_thread&) { ppu_thread_list.emplace_back(idm::get_unlocked>(id)); - }); + }, idm::unlocked); + } + + if (rlock) + { + rlock.unlock(); } if (static_cast(info) != m_stop_ctr || Emu.IsStopped()) diff --git a/rpcs3/rpcs3qt/debugger_frame.cpp b/rpcs3/rpcs3qt/debugger_frame.cpp index db4efa1aae..b8ed1d642f 100644 --- a/rpcs3/rpcs3qt/debugger_frame.cpp +++ b/rpcs3/rpcs3qt/debugger_frame.cpp @@ -872,7 +872,7 @@ std::function debugger_frame::make_check_cpu(cpu_thread* cpu, boo { constexpr cpu_thread* null_cpu = nullptr; - if (Emu.IsStopped()) + if (Emu.IsStopped(true)) { return []() { return null_cpu; }; } @@ -921,7 +921,7 @@ std::function debugger_frame::make_check_cpu(cpu_thread* cpu, boo return [cpu, type, shared = std::move(shared), emulation_id = Emu.GetEmulationIdentifier()]() mutable -> cpu_thread* { - if (emulation_id != Emu.GetEmulationIdentifier() || Emu.IsStopped()) + if (emulation_id != Emu.GetEmulationIdentifier() || Emu.IsStopped(true)) { // Invalidate all data after Emu.Kill() shared.reset(); @@ -1040,7 +1040,7 @@ void debugger_frame::UpdateUnitList() const u64 emulation_id = static_cast>(Emu.GetEmulationIdentifier()); const u64 threads_created = cpu_thread::g_threads_created; const u64 threads_deleted = cpu_thread::g_threads_deleted; - const system_state emu_state = Emu.GetStatus(); + const system_state emu_state = Emu.GetStatus(false); std::unique_lock lock{id_manager::g_mutex, std::defer_lock}; @@ -1077,6 +1077,11 @@ void debugger_frame::UpdateUnitList() { std::function func_cpu = make_check_cpu(std::addressof(cpu), true); + if (cpu.state & cpu_flag::exit) + { + return; + } + // Space at the end is to pad a gap on the right cpu_list.emplace_back(QString::fromStdString((id >> 24 == 0x55 ? "RSX[0x55555555]" : cpu.get_name()) + ' '), std::move(func_cpu)); diff --git a/rpcs3/rpcs3qt/main_window.cpp b/rpcs3/rpcs3qt/main_window.cpp index 8f9324d73e..94157571e8 100644 --- a/rpcs3/rpcs3qt/main_window.cpp +++ b/rpcs3/rpcs3qt/main_window.cpp @@ -128,7 +128,7 @@ extern void qt_events_aware_op(int repeat_duration_ms, std::function wra } else { - QTimer::singleShot(repeat_duration_ms, *check_iteration); + QTimer::singleShot(repeat_duration_ms, event_loop, *check_iteration); } }); @@ -138,7 +138,7 @@ extern void qt_events_aware_op(int repeat_duration_ms, std::function wra event_loop = new QEventLoop(); // Queue event initially - QTimer::singleShot(0, *check_iteration); + QTimer::singleShot(0, event_loop, *check_iteration); // Event loop event_loop->exec();