From 6119e8fad2677e731e6105af6063cabe7be069f8 Mon Sep 17 00:00:00 2001 From: lizzie Date: Tue, 7 Jul 2026 19:52:22 +0200 Subject: [PATCH] [hid_core] fix LM2 crash due to callbacks not being removed properly (#4178) the function returned the key AFTER this one, which points to an invalid object basically its like "oh im returning the index AFTER Myself" off-by-1 error basically; now it returns OUR own key. Signed-off-by: lizzie Reviewed-on: https://git.eden-emu.dev/eden-emu/eden/pulls/4178 Reviewed-by: MaranBr Reviewed-by: CamilleLaVey --- src/core/core.cpp | 2 +- src/hid_core/frontend/emulated_console.cpp | 16 ++++++---- src/hid_core/frontend/emulated_controller.cpp | 22 ++++++------- src/hid_core/frontend/emulated_devices.cpp | 16 ++++++---- src/hid_core/hid_core.cpp | 3 +- src/hid_core/resources/npad/npad.cpp | 32 +++++++++---------- src/input_common/input_engine.cpp | 13 ++++---- 7 files changed, 51 insertions(+), 53 deletions(-) diff --git a/src/core/core.cpp b/src/core/core.cpp index eb37953351..8b9133f963 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -108,7 +108,7 @@ FileSys::VirtualFile GetGameFileFromPath(const FileSys::VirtualFilesystem& vfs, struct System::Impl { explicit Impl(System& system) - : kernel{system}, fs_controller{system}, hid_core{system.Kernel()}, cpu_manager{system}, + : kernel{system}, fs_controller{system}, hid_core{kernel}, cpu_manager{system}, reporter{system}, applet_manager{system}, frontend_applets{system}, profile_manager{} {} u64 program_id; diff --git a/src/hid_core/frontend/emulated_console.cpp b/src/hid_core/frontend/emulated_console.cpp index 114c22fb74..3807b9e182 100644 --- a/src/hid_core/frontend/emulated_console.cpp +++ b/src/hid_core/frontend/emulated_console.cpp @@ -1,7 +1,11 @@ +// SPDX-FileCopyrightText: Copyright 2026 Eden Emulator Project +// SPDX-License-Identifier: GPL-3.0-or-later + // SPDX-FileCopyrightText: Copyright 2021 yuzu Emulator Project // SPDX-License-Identifier: GPL-2.0-or-later #include "common/settings.h" +#include "common/assert.h" #include "hid_core/frontend/emulated_console.h" #include "hid_core/frontend/input_converter.h" @@ -308,17 +312,15 @@ void EmulatedConsole::TriggerOnChange(ConsoleTriggerType type) { int EmulatedConsole::SetCallback(ConsoleUpdateCallback update_callback) { std::scoped_lock lock{callback_mutex}; + ++last_callback_key; callback_list.insert_or_assign(last_callback_key, std::move(update_callback)); - return last_callback_key++; + return last_callback_key; } void EmulatedConsole::DeleteCallback(int key) { std::scoped_lock lock{callback_mutex}; - const auto& iterator = callback_list.find(key); - if (iterator == callback_list.end()) { - LOG_ERROR(Input, "Tried to delete non-existent callback {}", key); - return; - } - callback_list.erase(iterator); + auto const it = callback_list.find(key); + ASSERT_MSG(it != callback_list.end(), "Tried to delete non-existent callback {}", key); + callback_list.erase(it); } } // namespace Core::HID diff --git a/src/hid_core/frontend/emulated_controller.cpp b/src/hid_core/frontend/emulated_controller.cpp index 3ebb9133dd..6022f90951 100644 --- a/src/hid_core/frontend/emulated_controller.cpp +++ b/src/hid_core/frontend/emulated_controller.cpp @@ -10,6 +10,7 @@ #include #include "common/thread.h" +#include "common/assert.h" #include "hid_core/frontend/emulated_controller.h" #include "hid_core/frontend/input_converter.h" #include "hid_core/hid_util.h" @@ -1876,12 +1877,9 @@ NpadColor EmulatedController::GetNpadColor(u32 color) { void EmulatedController::TriggerOnChange(ControllerTriggerType type, bool is_npad_service_update) { std::unique_lock lock{callback_mutex}; - for (const auto& poller_pair : callback_list) { - const ControllerUpdateCallback& poller = poller_pair.second; - if (!is_npad_service_update && poller.is_npad_service) { - continue; - } - if (poller.on_change) { + for (auto const& p : callback_list) { + auto const& poller = p.second; + if (is_npad_service_update || !poller.is_npad_service) { poller.on_change(type); } } @@ -1889,18 +1887,16 @@ void EmulatedController::TriggerOnChange(ControllerTriggerType type, bool is_npa int EmulatedController::SetCallback(ControllerUpdateCallback update_callback) { std::unique_lock lock{callback_mutex}; + ++last_callback_key; callback_list.insert_or_assign(last_callback_key, std::move(update_callback)); - return last_callback_key++; + return last_callback_key; } void EmulatedController::DeleteCallback(int key) { std::unique_lock lock{callback_mutex}; - const auto& iterator = callback_list.find(key); - if (iterator == callback_list.end()) { - LOG_ERROR(Input, "Tried to delete non-existent callback {}", key); - return; - } - callback_list.erase(iterator); + auto const it = callback_list.find(key); + ASSERT_MSG(it != callback_list.end(), "Tried to delete non-existent callback {}", key); + callback_list.erase(it); } void EmulatedController::StatusUpdate() { diff --git a/src/hid_core/frontend/emulated_devices.cpp b/src/hid_core/frontend/emulated_devices.cpp index f0b543c54c..ae3ee31f3d 100644 --- a/src/hid_core/frontend/emulated_devices.cpp +++ b/src/hid_core/frontend/emulated_devices.cpp @@ -1,9 +1,13 @@ +// SPDX-FileCopyrightText: Copyright 2026 Eden Emulator Project +// SPDX-License-Identifier: GPL-3.0-or-later + // SPDX-FileCopyrightText: Copyright 2021 yuzu Emulator Project // SPDX-License-Identifier: GPL-2.0-or-later #include #include +#include "common/assert.h" #include "hid_core/frontend/emulated_devices.h" #include "hid_core/frontend/input_converter.h" @@ -467,17 +471,15 @@ void EmulatedDevices::TriggerOnChange(DeviceTriggerType type) { int EmulatedDevices::SetCallback(InterfaceUpdateCallback update_callback) { std::scoped_lock lock{callback_mutex}; + ++last_callback_key; callback_list.insert_or_assign(last_callback_key, std::move(update_callback)); - return last_callback_key++; + return last_callback_key; } void EmulatedDevices::DeleteCallback(int key) { std::scoped_lock lock{callback_mutex}; - const auto& iterator = callback_list.find(key); - if (iterator == callback_list.end()) { - LOG_ERROR(Input, "Tried to delete non-existent callback {}", key); - return; - } - callback_list.erase(iterator); + auto const it = callback_list.find(key); + ASSERT_MSG(it != callback_list.end(), "Tried to delete non-existent callback {}", key); + callback_list.erase(it); } } // namespace Core::HID diff --git a/src/hid_core/hid_core.cpp b/src/hid_core/hid_core.cpp index 12cd0c1f27..cbc193b741 100644 --- a/src/hid_core/hid_core.cpp +++ b/src/hid_core/hid_core.cpp @@ -24,7 +24,8 @@ HIDCore::HIDCore(Kernel::KernelCore& kernel_) , player_8{std::make_unique(NpadIdType::Player8)} , other{std::make_unique(NpadIdType::Other)} , handheld{std::make_unique(NpadIdType::Handheld)} - , console{std::make_unique()}, devices{std::make_unique()} + , console{std::make_unique()} + , devices{std::make_unique()} , kernel{kernel_} {} diff --git a/src/hid_core/resources/npad/npad.cpp b/src/hid_core/resources/npad/npad.cpp index ab5a3f7823..044ad0fb32 100644 --- a/src/hid_core/resources/npad/npad.cpp +++ b/src/hid_core/resources/npad/npad.cpp @@ -45,29 +45,17 @@ NPad::NPad(Core::HID::HIDCore& hid_core_, KernelHelpers::ServiceContext& service AbstractPad{hid_core_.kernel}, }} { - for (std::size_t aruid_index = 0; aruid_index < AruidIndexMax; ++aruid_index) { - for (std::size_t i = 0; i < controller_data[aruid_index].size(); ++i) { - auto& controller = controller_data[aruid_index][i]; - controller.device = hid_core.GetEmulatedControllerByIndex(i); - Core::HID::ControllerUpdateCallback engine_callback{ - .on_change = [this, i, kernel = &hid_core.kernel](Core::HID::ControllerTriggerType type) { - ControllerUpdate(*kernel, type, i); - }, - .is_npad_service = true, - }; - controller.callback_key = controller.device->SetCallback(engine_callback); - } - } for (std::size_t i = 0; i < abstracted_pads.size(); ++i) { abstracted_pads[i].SetNpadId(IndexToNpadIdType(i)); } } NPad::~NPad() { - for (std::size_t aruid_index = 0; aruid_index < AruidIndexMax; ++aruid_index) { - for (std::size_t i = 0; i < controller_data[aruid_index].size(); ++i) { - auto& controller = controller_data[aruid_index][i]; - controller.device->DeleteCallback(controller.callback_key); + for (std::size_t aruid = 0; aruid < AruidIndexMax; ++aruid) { + for (std::size_t i = 0; i < controller_data[aruid].size(); ++i) { + if (auto& controller = controller_data[aruid][i]; controller.device && controller.callback_key) { + controller.device->DeleteCallback(controller.callback_key); + } } } } @@ -101,6 +89,16 @@ Result NPad::Activate(u64 aruid) { for (std::size_t i = 0; i < controller_data[aruid_index].size(); ++i) { auto& controller = controller_data[aruid_index][i]; controller.shared_memory = &data->shared_memory_format->npad.npad_entry[i].internal_state; + controller.device = hid_core.GetEmulatedControllerByIndex(i); + if (!controller.callback_key) { + Core::HID::ControllerUpdateCallback engine_callback{ + .on_change = [this, i](Core::HID::ControllerTriggerType type) { + ControllerUpdate(hid_core.kernel, type, i); + }, + .is_npad_service = true, + }; + controller.callback_key = controller.device->SetCallback(engine_callback); + } } // Prefill controller buffers diff --git a/src/input_common/input_engine.cpp b/src/input_common/input_engine.cpp index ccfe3444ae..21d257d088 100644 --- a/src/input_common/input_engine.cpp +++ b/src/input_common/input_engine.cpp @@ -4,6 +4,7 @@ // SPDX-License-Identifier: GPL-2.0-or-later #include "common/logging.h" +#include "common/assert.h" #include "input_common/input_engine.h" namespace InputCommon { @@ -463,8 +464,9 @@ const std::string& InputEngine::GetEngineName() const { int InputEngine::SetCallback(InputIdentifier input_identifier) { std::scoped_lock lock{mutex_callback}; + ++last_callback_key; callback_list.insert_or_assign(last_callback_key, std::move(input_identifier)); - return last_callback_key++; + return last_callback_key; } void InputEngine::SetMappingCallback(MappingCallback callback) { @@ -474,12 +476,9 @@ void InputEngine::SetMappingCallback(MappingCallback callback) { void InputEngine::DeleteCallback(int key) { std::scoped_lock lock{mutex_callback}; - const auto& iterator = callback_list.find(key); - if (iterator == callback_list.end()) { - LOG_ERROR(Input, "Tried to delete non-existent callback {}", key); - return; - } - callback_list.erase(iterator); + auto const it = callback_list.find(key); + ASSERT_MSG(it != callback_list.end(), "Tried to delete non-existent callback {}", key); + callback_list.erase(it); } } // namespace InputCommon