From e45360de2ba1f24395579e9cb3836cdd7a82cff6 Mon Sep 17 00:00:00 2001 From: kd-11 Date: Fri, 7 Feb 2020 12:57:21 +0300 Subject: [PATCH] overlays: Fix use after free - Overlay can be closed when secondary thread is asleep! Wait for it to wake before proceeding with deletion. --- .../RSX/Overlays/overlay_message_dialog.cpp | 30 +++++++++++-------- rpcs3/Emu/RSX/Overlays/overlays.cpp | 12 ++++++-- rpcs3/Emu/RSX/Overlays/overlays.h | 22 +++++++++----- 3 files changed, 42 insertions(+), 22 deletions(-) diff --git a/rpcs3/Emu/RSX/Overlays/overlay_message_dialog.cpp b/rpcs3/Emu/RSX/Overlays/overlay_message_dialog.cpp index 99f6352841..491629d6f3 100644 --- a/rpcs3/Emu/RSX/Overlays/overlay_message_dialog.cpp +++ b/rpcs3/Emu/RSX/Overlays/overlay_message_dialog.cpp @@ -229,26 +229,30 @@ namespace rsx } else { - thread_ctrl::spawn("dialog input thread", [&] + std::scoped_lock lock(m_threadpool_mutex); + if (!exit) { - if (interactive) + m_workers.emplace_back([&] { - if (auto error = run_input_loop()) + if (interactive) { - rsx_log.error("Dialog input loop exited with error code=%d", error); + if (auto error = run_input_loop()) + { + rsx_log.error("Dialog input loop exited with error code=%d", error); + } } - } - else - { - while (!exit) + else { - refresh(); + while (!exit) + { + refresh(); - // Only update the screen at about 60fps since updating it everytime slows down the process - std::this_thread::sleep_for(16ms); + // Only update the screen at about 60fps since updating it everytime slows down the process + std::this_thread::sleep_for(16ms); + } } - } - }); + }); + } } return CELL_OK; diff --git a/rpcs3/Emu/RSX/Overlays/overlays.cpp b/rpcs3/Emu/RSX/Overlays/overlays.cpp index 54077269a8..a9cd9f4de6 100644 --- a/rpcs3/Emu/RSX/Overlays/overlays.cpp +++ b/rpcs3/Emu/RSX/Overlays/overlays.cpp @@ -308,11 +308,19 @@ namespace rsx // Unreachable return 0; } - + void user_interface::close(bool use_callback) { // Force unload - exit = true; + exit.release(true); + { + reader_lock lock(m_threadpool_mutex); + for (auto& worker : m_workers) + { + worker.join(); + } + } + if (auto manager = g_fxo->get()) { if (auto dlg = manager->get()) diff --git a/rpcs3/Emu/RSX/Overlays/overlays.h b/rpcs3/Emu/RSX/Overlays/overlays.h index af4679c8aa..e88f3d2131 100644 --- a/rpcs3/Emu/RSX/Overlays/overlays.h +++ b/rpcs3/Emu/RSX/Overlays/overlays.h @@ -17,6 +17,8 @@ #include "Utilities/CPUStats.h" #include "Utilities/Timer.h" +#include + // Utils std::string utf8_to_ascii8(const std::string& utf8_string); std::string utf16_to_ascii8(const std::u16string& utf16_string); @@ -48,8 +50,9 @@ namespace rsx }; // Interactable UI element - struct user_interface : overlay + class user_interface : public overlay { + public: // Move this somewhere to avoid duplication enum selection_code { @@ -76,19 +79,24 @@ namespace rsx pad_button_max_enum }; + protected: Timer input_timer; - bool exit = false; + atomic_t exit{ false }; - s32 return_code = CELL_OK; std::function on_close; + shared_mutex m_threadpool_mutex; + std::list m_workers; + + public: + s32 return_code = CELL_OK; + + public: void update() override {} + compiled_resource get_compiled() override = 0; - virtual void on_button_pressed(pad_button /*button_press*/) - { - close(); - } + virtual void on_button_pressed(pad_button /*button_press*/) {} void close(bool use_callback = true);