From 4a18c23e2d2b2edf68c74a643da79c721f285d58 Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 4 May 2025 13:25:33 +0200 Subject: [PATCH] Wait for the reloading cells thread on DataFilesPage destruction Run a single thread and notify it when it has to reload cells. --- CHANGELOG.md | 1 + apps/launcher/datafilespage.cpp | 63 +++++++++++++++++++++++---------- apps/launcher/datafilespage.hpp | 15 ++++++-- 3 files changed, 58 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8656f792df..79671a0f86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -231,6 +231,7 @@ Bug #8378: Korean bitmap fonts are unusable Bug #8439: Creatures without models can crash the game Bug #8441: Freeze when using video main menu replacers + Bug #8445: Launcher crashes on exit when cell name loading thread is still running Bug #8462: Crashes when resizing the window on macOS Feature #1415: Infinite fall failsafe Feature #2566: Handle NAM9 records for manual cell references diff --git a/apps/launcher/datafilespage.cpp b/apps/launcher/datafilespage.cpp index c1bcc8f944..f5c0694a3b 100644 --- a/apps/launcher/datafilespage.cpp +++ b/apps/launcher/datafilespage.cpp @@ -159,6 +159,7 @@ Launcher::DataFilesPage::DataFilesPage(const Files::ConfigurationManager& cfg, C , mGameSettings(gameSettings) , mLauncherSettings(launcherSettings) , mNavMeshToolInvoker(new Process::ProcessInvoker(this)) + , mReloadCellsThread(&DataFilesPage::reloadCells, this) { ui.setupUi(this); setObjectName("DataFilesPage"); @@ -205,6 +206,16 @@ Launcher::DataFilesPage::DataFilesPage(const Files::ConfigurationManager& cfg, C slotAddonDataChanged(); } +Launcher::DataFilesPage::~DataFilesPage() +{ + { + const std::lock_guard lock(mReloadCellsMutex); + mAbortReloadCells = true; + mStartReloadCells.notify_one(); + } + mReloadCellsThread.join(); +} + void Launcher::DataFilesPage::buildView() { QToolButton* refreshButton = mSelector->refreshButton(); @@ -991,31 +1002,45 @@ bool Launcher::DataFilesPage::showDeleteMessageBox(const QString& text) void Launcher::DataFilesPage::slotAddonDataChanged() { QStringList selectedFiles = selectedFilePaths(); - if (previousSelectedFiles != selectedFiles) + if (mSelectedFiles != selectedFiles) { - previousSelectedFiles = selectedFiles; - // Loading cells for core Morrowind + Expansions takes about 0.2 seconds, which is enough to cause a - // barely perceptible UI lag. Splitting into its own thread to alleviate that. - std::thread loadCellsThread(&DataFilesPage::reloadCells, this, selectedFiles); - loadCellsThread.detach(); + const std::lock_guard lock(mReloadCellsMutex); + mSelectedFiles = std::move(selectedFiles); + mReloadCells = true; + mStartReloadCells.notify_one(); } } -// Mutex lock to run reloadCells synchronously. -static std::mutex reloadCellsMutex; - -void Launcher::DataFilesPage::reloadCells(QStringList selectedFiles) +void Launcher::DataFilesPage::reloadCells() { - // Use a mutex lock so that we can prevent two threads from executing the rest of this code at the same time - // Based on https://stackoverflow.com/a/5429695/531762 - std::unique_lock lock(reloadCellsMutex); + std::unique_lock lock(mReloadCellsMutex); - // The following code will run only if there is not another thread currently running it - CellNameLoader cellNameLoader; - QSet set = cellNameLoader.getCellNames(selectedFiles); - QStringList cellNamesList(set.begin(), set.end()); - std::sort(cellNamesList.begin(), cellNamesList.end()); - emit signalLoadedCellsChanged(cellNamesList); + while (true) + { + mStartReloadCells.wait(lock); + + if (mAbortReloadCells) + return; + + if (!std::exchange(mReloadCells, false)) + continue; + + QStringList selectedFiles = mSelectedFiles; + + lock.unlock(); + + CellNameLoader cellNameLoader; + QSet set = cellNameLoader.getCellNames(selectedFiles); + QStringList cellNamesList(set.begin(), set.end()); + std::sort(cellNamesList.begin(), cellNamesList.end()); + + emit signalLoadedCellsChanged(std::move(cellNamesList)); + + lock.lock(); + + if (mAbortReloadCells) + return; + } } void Launcher::DataFilesPage::startNavMeshTool() diff --git a/apps/launcher/datafilespage.hpp b/apps/launcher/datafilespage.hpp index 5d03cdf800..ac98743dab 100644 --- a/apps/launcher/datafilespage.hpp +++ b/apps/launcher/datafilespage.hpp @@ -10,6 +10,10 @@ #include #include +#include +#include +#include + class QSortFilterProxyModel; class QAbstractItemModel; class QMenu; @@ -47,6 +51,7 @@ namespace Launcher public: explicit DataFilesPage(const Files::ConfigurationManager& cfg, Config::GameSettings& gameSettings, Config::LauncherSettings& launcherSettings, MainDialog* parent = nullptr); + ~DataFilesPage(); QAbstractItemModel* profilesModel() const; @@ -119,7 +124,7 @@ namespace Launcher Config::LauncherSettings& mLauncherSettings; QString mPreviousProfile; - QStringList previousSelectedFiles; + QStringList mSelectedFiles; QString mDataLocal; QStringList mKnownArchives; QStringList mNewDataDirs; @@ -127,6 +132,12 @@ namespace Launcher Process::ProcessInvoker* mNavMeshToolInvoker; NavMeshToolProgress mNavMeshToolProgress; + bool mReloadCells = false; + bool mAbortReloadCells = false; + std::mutex mReloadCellsMutex; + std::condition_variable mStartReloadCells; + std::thread mReloadCellsThread; + void addArchive(const QString& name, Qt::CheckState selected, int row = -1); void addArchivesFromDir(const QString& dir); void buildView(); @@ -140,7 +151,7 @@ namespace Launcher void addProfile(const QString& profile, bool setAsCurrent); void checkForDefaultProfile(); void populateFileViews(const QString& contentModelName); - void reloadCells(QStringList selectedFiles); + void reloadCells(); void refreshDataFilesView(); void updateNavMeshProgress(int minDataSize); void slotCopySelectedItemsPaths();