From 0d05f64e795f73c06343d6eca6e6bf655944792b Mon Sep 17 00:00:00 2001 From: Zephyron Date: Mon, 21 Jul 2025 21:23:51 +1000 Subject: [PATCH] fix: Prevent controller crashes and memory leaks in game list grid view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix memory leak in PopulateGridView() and FilterGridView() by properly deleting old QStandardItemModel instances before creating new ones - Add safety checks in controller navigation to prevent crashes when accessing uninitialized controller data - Improve controller input handling to only send events to visible and properly initialized views - Add null pointer checks in SetViewMode() to prevent crashes when setting current index on empty models - Add proper cleanup in GameList destructor to prevent memory leaks on application shutdown The main issue was that switching between list and grid views created new models without properly cleaning up old ones, leading to memory leaks. Additionally, controller navigation would send keyboard events to both views simultaneously, causing crashes when one view was not properly initialized or visible. Fixes crashes when using controller navigation in grid view mode. Thanks to Beta Testers acarajé & Hayate Yoshida (吉田 疾風) for finding the bug. Signed-off-by: Zephyron --- src/citron/game_list.cpp | 45 +++++++++++++++++++++-- src/citron/util/controller_navigation.cpp | 17 +++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/src/citron/game_list.cpp b/src/citron/game_list.cpp index e42d85294..f71e11adb 100644 --- a/src/citron/game_list.cpp +++ b/src/citron/game_list.cpp @@ -203,6 +203,13 @@ void GameList::FilterGridView(const QString& filter_text) { // Repopulate the grid view with filtered items QStandardItemModel* hierarchical_model = item_model; + // Delete the previous flat model if it exists to prevent memory leaks + if (QAbstractItemModel* old_model = list_view->model()) { + if (old_model != item_model) { + old_model->deleteLater(); + } + } + // Create a new flat model for grid view QStandardItemModel* flat_model = new QStandardItemModel(this); @@ -470,9 +477,19 @@ GameList::GameList(FileSys::VirtualFilesystem vfs_, FileSys::ManualContentProvid if (!this->isActiveWindow()) { return; } + + // Only send events to visible and properly initialized views QKeyEvent* event = new QKeyEvent(QEvent::KeyPress, key, Qt::NoModifier); - QCoreApplication::postEvent(tree_view, event); - QCoreApplication::postEvent(list_view, event); + + if (tree_view->isVisible() && tree_view->model()) { + QCoreApplication::postEvent(tree_view, event); + } + + if (list_view->isVisible() && list_view->model()) { + // Create a new event for the list view to avoid double deletion + QKeyEvent* list_event = new QKeyEvent(QEvent::KeyPress, key, Qt::NoModifier); + QCoreApplication::postEvent(list_view, list_event); + } }); // We must register all custom types with the Qt Automoc system so that we are able to use @@ -496,6 +513,13 @@ void GameList::UnloadController() { GameList::~GameList() { UnloadController(); + + // Clean up any custom models that might have been created for grid view + if (QAbstractItemModel* current_model = list_view->model()) { + if (current_model != item_model) { + current_model->deleteLater(); + } + } } void GameList::SetFilterFocus() { @@ -1114,12 +1138,18 @@ void GameList::SetViewMode(bool grid_view) { PopulateGridView(); tree_view->setVisible(false); list_view->setVisible(true); - list_view->setCurrentIndex(list_view->model()->index(0, 0)); + // Only set current index if the model has items + if (list_view->model() && list_view->model()->rowCount() > 0) { + list_view->setCurrentIndex(list_view->model()->index(0, 0)); + } } else { // Restore the hierarchical model for tree view list_view->setVisible(false); tree_view->setVisible(true); - tree_view->setCurrentIndex(item_model->index(0, 0)); + // Only set current index if the model has items + if (item_model && item_model->rowCount() > 0) { + tree_view->setCurrentIndex(item_model->index(0, 0)); + } } } @@ -1127,6 +1157,13 @@ void GameList::PopulateGridView() { // Store the current hierarchical model QStandardItemModel* hierarchical_model = item_model; + // Delete the previous flat model if it exists to prevent memory leaks + if (QAbstractItemModel* old_model = list_view->model()) { + if (old_model != item_model) { + old_model->deleteLater(); + } + } + // Create a new flat model for grid view QStandardItemModel* flat_model = new QStandardItemModel(this); diff --git a/src/citron/util/controller_navigation.cpp b/src/citron/util/controller_navigation.cpp index e7a33abe1..0967b758b 100644 --- a/src/citron/util/controller_navigation.cpp +++ b/src/citron/util/controller_navigation.cpp @@ -1,4 +1,5 @@ // SPDX-FileCopyrightText: Copyright 2021 yuzu Emulator Project +// SPDX-FileCopyrightText: Copyright 2025 Citron Emulator Project // SPDX-License-Identifier: GPL-2.0-or-later #include "common/settings_input.h" @@ -42,6 +43,12 @@ void ControllerNavigation::ControllerUpdateEvent(Core::HID::ControllerTriggerTyp if (!Settings::values.controller_navigation) { return; } + + // Safety check: ensure controllers are properly initialized + if (!is_controller_set || !player1_controller || !handheld_controller) { + return; + } + if (type == Core::HID::ControllerTriggerType::Button) { ControllerUpdateButton(); return; @@ -54,6 +61,11 @@ void ControllerNavigation::ControllerUpdateEvent(Core::HID::ControllerTriggerTyp } void ControllerNavigation::ControllerUpdateButton() { + // Safety check: ensure controllers are properly initialized + if (!is_controller_set || !player1_controller || !handheld_controller) { + return; + } + const auto controller_type = player1_controller->GetNpadStyleIndex(); const auto& player1_buttons = player1_controller->GetButtonsValues(); const auto& handheld_buttons = handheld_controller->GetButtonsValues(); @@ -91,6 +103,11 @@ void ControllerNavigation::ControllerUpdateButton() { } void ControllerNavigation::ControllerUpdateStick() { + // Safety check: ensure controllers are properly initialized + if (!is_controller_set || !player1_controller || !handheld_controller) { + return; + } + const auto controller_type = player1_controller->GetNpadStyleIndex(); const auto& player1_sticks = player1_controller->GetSticksValues(); const auto& handheld_sticks = player1_controller->GetSticksValues();