fix: Prevent controller crashes and memory leaks in game list grid view

- 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 <zephyron@citron-emu.org>
This commit is contained in:
Zephyron
2025-07-21 21:23:51 +10:00
parent c7bc05f1d0
commit 0d05f64e79
2 changed files with 58 additions and 4 deletions

View File

@@ -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);
if (tree_view->isVisible() && tree_view->model()) {
QCoreApplication::postEvent(tree_view, event);
QCoreApplication::postEvent(list_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,19 +1138,32 @@ void GameList::SetViewMode(bool grid_view) {
PopulateGridView();
tree_view->setVisible(false);
list_view->setVisible(true);
// 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);
// Only set current index if the model has items
if (item_model && item_model->rowCount() > 0) {
tree_view->setCurrentIndex(item_model->index(0, 0));
}
}
}
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);

View File

@@ -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();