From ff3759409b9edd54ee5a9b1b400ab0ad277fa0b3 Mon Sep 17 00:00:00 2001 From: Zephyron Date: Wed, 3 Dec 2025 11:51:09 +1000 Subject: [PATCH] fix: Add destruction callback to EmulatedController for safe pointer management Add SetDestructionCallback() method to allow observers to safely null out pointers when the controller is destroyed. This prevents use-after-free issues in UI components. Also includes cleanup and const correctness improvements in configure_input_player_widget. Signed-off-by: Zephyron --- .../configure_input_player_widget.cpp | 19 ++++++++-- .../configure_input_player_widget.h | 35 +++++++++---------- src/hid_core/frontend/emulated_controller.cpp | 10 +++++- src/hid_core/frontend/emulated_controller.h | 8 +++++ 4 files changed, 50 insertions(+), 22 deletions(-) diff --git a/src/citron/configuration/configure_input_player_widget.cpp b/src/citron/configuration/configure_input_player_widget.cpp index 7444f4e0c..ffe636842 100644 --- a/src/citron/configuration/configure_input_player_widget.cpp +++ b/src/citron/configuration/configure_input_player_widget.cpp @@ -12,6 +12,9 @@ PlayerControlPreview::PlayerControlPreview(QWidget* parent) : QFrame(parent) { is_controller_set = false; + controller = nullptr; + callback_key = -1; + QTimer* timer = new QTimer(this); connect(timer, &QTimer::timeout, this, QOverload<>::of(&PlayerControlPreview::UpdateInput)); @@ -31,6 +34,14 @@ void PlayerControlPreview::SetController(Core::HID::EmulatedController* controll UnloadController(); is_controller_set = true; controller = controller_; + + // This registers a function that the controller will call from its destructor. + // When called, it safely nulls our pointer so we don't use it after it's been freed. + controller->SetDestructionCallback([this]() { + this->controller = nullptr; + this->is_controller_set = false; + }); + Core::HID::ControllerUpdateCallback engine_callback{ .on_change = [this](Core::HID::ControllerTriggerType type) { ControllerUpdate(type); }, .is_npad_service = false, @@ -40,10 +51,14 @@ void PlayerControlPreview::SetController(Core::HID::EmulatedController* controll } void PlayerControlPreview::UnloadController() { - if (is_controller_set) { + // Only try to access the controller if the pointer is valid. + if (controller) { controller->DeleteCallback(callback_key); - is_controller_set = false; + // Also clear the destruction callback we set. + controller->SetDestructionCallback(nullptr); } + is_controller_set = false; + controller = nullptr; } void PlayerControlPreview::BeginMappingButton(std::size_t button_id) { diff --git a/src/citron/configuration/configure_input_player_widget.h b/src/citron/configuration/configure_input_player_widget.h index 22a8fa8f3..25ab38be4 100644 --- a/src/citron/configuration/configure_input_player_widget.h +++ b/src/citron/configuration/configure_input_player_widget.h @@ -14,12 +14,6 @@ #include "hid_core/frontend/emulated_controller.h" #include "hid_core/hid_types.h" -class QLabel; - -using AnalogParam = std::array; -using ButtonParam = std::array; - -// Widget for representing controller animations class PlayerControlPreview : public QFrame { Q_OBJECT @@ -206,30 +200,33 @@ private: // Draw primitive types template void DrawPolygon(QPainter& p, const std::array& polygon); - void DrawCircle(QPainter& p, QPointF center, float size); - void DrawRectangle(QPainter& p, QPointF center, float width, float height); - void DrawRoundRectangle(QPainter& p, QPointF center, float width, float height, float round); - void DrawText(QPainter& p, QPointF center, float text_size, const QString& text); + void DrawCircle(QPainter& p, const QPointF center, float size); + void DrawRectangle(QPainter& p, const QPointF center, float width, float height); + void DrawRoundRectangle(QPainter& p, const QPointF center, float width, float height, float round); + void DrawText(QPainter& p, const QPointF center, float text_size, const QString& text); void SetTextFont(QPainter& p, float text_size, const QString& font_family = QStringLiteral("sans-serif")); bool raw_joystick_visible = false; - bool is_controller_set{}; - bool is_connected{}; - bool needs_redraw{}; - Core::HID::NpadStyleIndex controller_type; + Core::HID::EmulatedController* controller = nullptr; + Core::HID::NpadStyleIndex controller_type{}; + int callback_key = -1; + bool is_controller_set = false; + bool is_connected = false; + bool needs_redraw = false; + + bool mapping_active = false; + int blink_counter = 0; - bool mapping_active{}; - int blink_counter{}; - int callback_key; QColor button_color{}; ColorMapping colors{}; Core::HID::LedPattern led_pattern{0, 0, 0, 0}; - std::size_t player_index{}; - Core::HID::EmulatedController* controller; + + std::size_t player_index = 0; std::size_t button_mapping_index{Settings::NativeButton::NumButtons}; std::size_t analog_mapping_index{Settings::NativeAnalog::NumAnalogs}; + Core::HID::ButtonValues button_values{}; Core::HID::SticksValues stick_values{}; Core::HID::TriggerValues trigger_values{}; diff --git a/src/hid_core/frontend/emulated_controller.cpp b/src/hid_core/frontend/emulated_controller.cpp index 909dd0eef..1a150bb91 100644 --- a/src/hid_core/frontend/emulated_controller.cpp +++ b/src/hid_core/frontend/emulated_controller.cpp @@ -24,7 +24,15 @@ constexpr Common::UUID VIRTUAL_UUID = EmulatedController::EmulatedController(NpadIdType npad_id_type_) : npad_id_type(npad_id_type_) {} -EmulatedController::~EmulatedController() = default; +EmulatedController::~EmulatedController() { + if (destruction_callback) { + destruction_callback(); + } +} + +void EmulatedController::SetDestructionCallback(std::function callback) { + destruction_callback = std::move(callback); +} NpadStyleIndex EmulatedController::MapSettingsTypeToNPad(Settings::ControllerType type) { switch (type) { diff --git a/src/hid_core/frontend/emulated_controller.h b/src/hid_core/frontend/emulated_controller.h index f51593e54..bd6a45b38 100644 --- a/src/hid_core/frontend/emulated_controller.h +++ b/src/hid_core/frontend/emulated_controller.h @@ -473,6 +473,12 @@ public: /// Changes sensitivity of the motion sensor void SetGyroscopeZeroDriftMode(GyroscopeZeroDriftMode mode); + /** + * Sets a callback to be invoked when this object is about to be destructed. + * This is used for observers to safely null out their pointers. + */ + void SetDestructionCallback(std::function callback); + /** * Adds a callback to the list of events * @param update_callback A ConsoleUpdateCallback that will be triggered @@ -654,6 +660,8 @@ private: std::unordered_map callback_list; int last_callback_key = 0; + std::function destruction_callback; + // Stores the current status of all controller input ControllerStatus controller; };