Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #7714 from cristian64/avoid_leaking_gamelistmodel 
DolphinQt: Avoid leaking the GameListModel instance to gracefully shutdown the GameTracker and prevent a crash on exit
  • Loading branch information
@leoetlino
leoetlino committed Nov 18, 2020
2 parents 0339889 + ee13e6e commit 31d7be5
Show file tree
Hide file tree
Showing 21 changed files with 191 additions and 117 deletions.
21 changes: 21 additions & 0 deletions Source/Core/Common/WorkQueueThread.h
View file
Expand Up @@ -27,20 +27,40 @@ class WorkQueueThread
{
Shutdown();
m_shutdown.Clear();
m_cancelled.Clear();
m_function = std::move(function);
m_thread = std::thread(&WorkQueueThread::ThreadLoop, this);
}

template <typename... Args>
void EmplaceItem(Args&&... args)
{
if (!m_cancelled.IsSet())
{
std::lock_guard lg(m_lock);
m_items.emplace(std::forward<Args>(args)...);
}
m_wakeup.Set();
}

void Clear()
{
{
std::lock_guard lg(m_lock);
m_items = std::queue<T>();
}
m_wakeup.Set();
}

void Cancel()
{
m_cancelled.Set();
Clear();
Shutdown();
}

bool IsCancelled() const { return m_cancelled.IsSet(); }

private:
void Shutdown()
{
Expand Down Expand Up @@ -81,6 +101,7 @@ class WorkQueueThread
std::thread m_thread;
Common::Event m_wakeup;
Common::Flag m_shutdown;
Common::Flag m_cancelled;
std::mutex m_lock;
std::queue<T> m_items;
};
Expand Down
10 changes: 4 additions & 6 deletions Source/Core/DolphinQt/CheatsManager.cpp
View file
Expand Up @@ -33,7 +33,6 @@

#include "DolphinQt/Config/ARCodeWidget.h"
#include "DolphinQt/Config/GeckoCodeWidget.h"
#include "DolphinQt/GameList/GameListModel.h"
#include "DolphinQt/Settings.h"

constexpr u32 MAX_RESULTS = 50;
Expand Down Expand Up @@ -152,7 +151,8 @@ static bool Compare(T mem_value, T value, CompareType op)
}
}

CheatsManager::CheatsManager(QWidget* parent) : QDialog(parent)
CheatsManager::CheatsManager(const GameListModel& game_list_model, QWidget* parent)
: QDialog(parent), m_game_list_model(game_list_model)
{
setWindowTitle(tr("Cheats Manager"));
setWindowFlags(windowFlags() & ~Qt::WindowContextHelpButtonHint);
Expand All @@ -175,11 +175,9 @@ void CheatsManager::OnStateChanged(Core::State state)
if (state != Core::State::Running && state != Core::State::Paused)
return;

auto* model = Settings::Instance().GetGameListModel();

for (int i = 0; i < model->rowCount(QModelIndex()); i++)
for (int i = 0; i < m_game_list_model.rowCount(QModelIndex()); i++)
{
auto file = model->GetGameFile(i);
auto file = m_game_list_model.GetGameFile(i);

if (file->GetGameID() == SConfig::GetInstance().GetGameID())
{
Expand Down
4 changes: 3 additions & 1 deletion Source/Core/DolphinQt/CheatsManager.h
View file
Expand Up @@ -11,6 +11,7 @@
#include <QDialog>

#include "Common/CommonTypes.h"
#include "DolphinQt/GameList/GameListModel.h"

class ARCodeWidget;
class QComboBox;
Expand Down Expand Up @@ -39,7 +40,7 @@ class CheatsManager : public QDialog
{
Q_OBJECT
public:
explicit CheatsManager(QWidget* parent = nullptr);
explicit CheatsManager(const GameListModel& game_list_model, QWidget* parent = nullptr);
~CheatsManager();

private:
Expand All @@ -61,6 +62,7 @@ class CheatsManager : public QDialog
void OnMatchContextMenu();
void OnWatchItemChanged(QTableWidgetItem* item);

const GameListModel& m_game_list_model;
std::vector<Result> m_results;
std::vector<Result> m_watch;
std::shared_ptr<const UICommon::GameFile> m_game_file;
Expand Down
77 changes: 46 additions & 31 deletions Source/Core/DolphinQt/GameList/GameList.cpp
View file
Expand Up @@ -41,7 +41,6 @@

#include "DolphinQt/Config/PropertiesDialog.h"
#include "DolphinQt/ConvertDialog.h"
#include "DolphinQt/GameList/GameListModel.h"
#include "DolphinQt/GameList/GridProxyModel.h"
#include "DolphinQt/GameList/ListProxyModel.h"
#include "DolphinQt/MenuBar.h"
Expand All @@ -54,27 +53,26 @@

#include "UICommon/GameFile.h"

GameList::GameList(QWidget* parent) : QStackedWidget(parent)
GameList::GameList(QWidget* parent) : QStackedWidget(parent), m_model(this)
{
m_model = Settings::Instance().GetGameListModel();
m_list_proxy = new ListProxyModel(this);
m_list_proxy->setSortCaseSensitivity(Qt::CaseInsensitive);
m_list_proxy->setSortRole(GameListModel::SORT_ROLE);
m_list_proxy->setSourceModel(m_model);
m_list_proxy->setSourceModel(&m_model);
m_grid_proxy = new GridProxyModel(this);
m_grid_proxy->setSourceModel(m_model);
m_grid_proxy->setSourceModel(&m_model);

MakeListView();
MakeGridView();
MakeEmptyView();

if (Settings::GetQSettings().contains(QStringLiteral("gridview/scale")))
m_model->SetScale(Settings::GetQSettings().value(QStringLiteral("gridview/scale")).toFloat());
m_model.SetScale(Settings::GetQSettings().value(QStringLiteral("gridview/scale")).toFloat());

connect(m_list, &QTableView::doubleClicked, this, &GameList::GameSelected);
connect(m_grid, &QListView::doubleClicked, this, &GameList::GameSelected);
connect(m_model, &QAbstractItemModel::rowsInserted, this, &GameList::ConsiderViewChange);
connect(m_model, &QAbstractItemModel::rowsRemoved, this, &GameList::ConsiderViewChange);
connect(&m_model, &QAbstractItemModel::rowsInserted, this, &GameList::ConsiderViewChange);
connect(&m_model, &QAbstractItemModel::rowsRemoved, this, &GameList::ConsiderViewChange);

addWidget(m_list);
addWidget(m_grid);
Expand All @@ -94,7 +92,7 @@ GameList::GameList(QWidget* parent) : QStackedWidget(parent)

void GameList::PurgeCache()
{
m_model->PurgeCache();
m_model.PurgeCache();
}

void GameList::MakeListView()
Expand Down Expand Up @@ -176,7 +174,7 @@ GameList::~GameList()
{
Settings::GetQSettings().setValue(QStringLiteral("tableheader/state"),
m_list->horizontalHeader()->saveState());
Settings::GetQSettings().setValue(QStringLiteral("gridview/scale"), m_model->GetScale());
Settings::GetQSettings().setValue(QStringLiteral("gridview/scale"), m_model.GetScale());
}

void GameList::UpdateColumnVisibility()
Expand Down Expand Up @@ -205,9 +203,13 @@ void GameList::UpdateColumnVisibility()

void GameList::MakeEmptyView()
{
const QString refreshing_msg = tr("Refreshing...");
const QString empty_msg = tr("Dolphin could not find any GameCube/Wii ISOs or WADs.\n"
"Double-click here to set a games directory...");

m_empty = new QLabel(this);
m_empty->setText(tr("Dolphin could not find any GameCube/Wii ISOs or WADs.\n"
"Double-click here to set a games directory..."));
m_empty->setText(refreshing_msg);
m_empty->setEnabled(false);
m_empty->setAlignment(Qt::AlignHCenter | Qt::AlignVCenter);

auto event_filter = new DoubleClickEventFilter{m_empty};
Expand All @@ -218,6 +220,21 @@ void GameList::MakeEmptyView()
if (!dir.isEmpty())
Settings::Instance().AddPath(dir);
});

QSizePolicy size_policy{m_empty->sizePolicy()};
size_policy.setRetainSizeWhenHidden(true);
m_empty->setSizePolicy(size_policy);

connect(&Settings::Instance(), &Settings::GameListRefreshRequested, this,
[this, refreshing_msg = refreshing_msg] {
m_empty->setText(refreshing_msg);
m_empty->setEnabled(false);
});
connect(&Settings::Instance(), &Settings::GameListRefreshCompleted, this,
[this, empty_msg = empty_msg] {
m_empty->setText(empty_msg);
m_empty->setEnabled(true);
});
}

void GameList::resizeEvent(QResizeEvent* event)
Expand Down Expand Up @@ -368,21 +385,19 @@ void GameList::ShowContextMenu(const QPoint&)

menu->addSeparator();

auto* model = Settings::Instance().GetGameListModel();

auto* tags_menu = menu->addMenu(tr("Tags"));

auto path = game->GetFilePath();
auto game_tags = model->GetGameTags(path);
auto game_tags = m_model.GetGameTags(path);

for (const auto& tag : model->GetAllTags())
for (const auto& tag : m_model.GetAllTags())
{
auto* tag_action = tags_menu->addAction(tag);

tag_action->setCheckable(true);
tag_action->setChecked(game_tags.contains(tag));

connect(tag_action, &QAction::toggled, [path, tag, model](bool checked) {
connect(tag_action, &QAction::toggled, [path, tag, model = &m_model](bool checked) {
if (!checked)
model->RemoveGameTag(path, tag);
else
Expand Down Expand Up @@ -635,7 +650,7 @@ void GameList::DeleteFile()

if (deletion_successful)
{
m_model->RemoveGame(game->GetFilePath());
m_model.RemoveGame(game->GetFilePath());
}
else
{
Expand Down Expand Up @@ -686,7 +701,7 @@ std::shared_ptr<const UICommon::GameFile> GameList::GetSelectedGame() const
if (sel_model->hasSelection())
{
QModelIndex model_index = proxy->mapToSource(sel_model->selectedIndexes()[0]);
return m_model->GetGameFile(model_index.row());
return m_model.GetGameFile(model_index.row());
}
return {};
}
Expand Down Expand Up @@ -714,7 +729,7 @@ QList<std::shared_ptr<const UICommon::GameFile>> GameList::GetSelectedGames() co
for (const auto& index : index_list)
{
QModelIndex model_index = proxy->mapToSource(index);
selected_list.push_back(m_model->GetGameFile(model_index.row()));
selected_list.push_back(m_model.GetGameFile(model_index.row()));
}
}
return selected_list;
Expand All @@ -728,18 +743,18 @@ bool GameList::HasMultipleSelected() const

std::shared_ptr<const UICommon::GameFile> GameList::FindGame(const std::string& path) const
{
return m_model->FindGame(path);
return m_model.FindGame(path);
}

std::shared_ptr<const UICommon::GameFile>
GameList::FindSecondDisc(const UICommon::GameFile& game) const
{
return m_model->FindSecondDisc(game);
return m_model.FindSecondDisc(game);
}

std::string GameList::GetNetPlayName(const UICommon::GameFile& game) const
{
return m_model->GetNetPlayName(game);
return m_model.GetNetPlayName(game);
}

void GameList::SetViewColumn(int col, bool view)
Expand All @@ -756,7 +771,7 @@ void GameList::SetPreferredView(bool list)

void GameList::ConsiderViewChange()
{
if (m_model->rowCount(QModelIndex()) > 0)
if (m_model.rowCount(QModelIndex()) > 0)
{
if (m_prefer_list)
setCurrentWidget(m_list);
Expand Down Expand Up @@ -905,7 +920,7 @@ void GameList::NewTag()
if (tag.isEmpty())
return;

Settings::Instance().GetGameListModel()->NewTag(tag);
m_model.NewTag(tag);
}

void GameList::DeleteTag()
Expand All @@ -915,12 +930,12 @@ void GameList::DeleteTag()
if (tag.isEmpty())
return;

Settings::Instance().GetGameListModel()->DeleteTag(tag);
m_model.DeleteTag(tag);
}

void GameList::SetSearchTerm(const QString& term)
{
m_model->SetSearchTerm(term);
m_model.SetSearchTerm(term);

m_list_proxy->invalidate();
m_grid_proxy->invalidate();
Expand All @@ -930,7 +945,7 @@ void GameList::SetSearchTerm(const QString& term)

void GameList::ZoomIn()
{
m_model->SetScale(m_model->GetScale() + 0.1);
m_model.SetScale(m_model.GetScale() + 0.1);

m_list_proxy->invalidate();
m_grid_proxy->invalidate();
Expand All @@ -940,10 +955,10 @@ void GameList::ZoomIn()

void GameList::ZoomOut()
{
if (m_model->GetScale() <= 0.1)
if (m_model.GetScale() <= 0.1)
return;

m_model->SetScale(m_model->GetScale() - 0.1);
m_model.SetScale(m_model.GetScale() - 0.1);

m_list_proxy->invalidate();
m_grid_proxy->invalidate();
Expand All @@ -955,7 +970,7 @@ void GameList::UpdateFont()
{
QFont f;

f.setPointSizeF(m_model->GetScale() * f.pointSize());
f.setPointSizeF(m_model.GetScale() * f.pointSize());

m_grid->setFont(f);
}
7 changes: 5 additions & 2 deletions Source/Core/DolphinQt/GameList/GameList.h
View file
Expand Up @@ -8,7 +8,8 @@

#include <QStackedWidget>

class GameListModel;
#include "DolphinQt/GameList/GameListModel.h"

class QLabel;
class QListView;
class QSortFilterProxyModel;
Expand Down Expand Up @@ -46,6 +47,8 @@ class GameList final : public QStackedWidget

void PurgeCache();

const GameListModel& GetGameListModel() const { return m_model; }

signals:
void GameSelected();
void NetPlayHost(const UICommon::GameFile& game);
Expand Down Expand Up @@ -85,7 +88,7 @@ class GameList final : public QStackedWidget
void ConsiderViewChange();
void UpdateFont();

GameListModel* m_model;
GameListModel m_model;
QSortFilterProxyModel* m_list_proxy;
QSortFilterProxyModel* m_grid_proxy;

Expand Down

0 comments on commit 31d7be5

Please sign in to comment.