From ce14fc91fb84c51650f57c059396756e1fc3c4cf Mon Sep 17 00:00:00 2001 From: BoiledElectricity Date: Tue, 16 Jun 2026 16:20:02 +0200 Subject: [PATCH] [game_list] fix crash and flashing from the directory watcher (#4099) So the game list watcher could rebuild the content providers (CreateFactories) while the populate worker was still scanning them, and the worker would walk a torn-down RegisteredCache and segfault in OpenFileOrDirectoryConcat. Fixed by stopping and joining the worker before rebuilding. On macOS the watcher also re-armed itself every populate. Re-adding the same paths makes QFileSystemWatcher re-emit directoryChanged (the FSEvent comes in async, so the blockSignals guard misses it), so it just kept refreshing and the list flashed forever. Now it only re-arms when the watched dirs actually changed. Also null-guarded OpenFileOrDirectoryConcat so a torn-down cache cant null-deref there. Related: https://github.com/eden-emulator/Issue-Reports/issues/336 Reviewed-on: https://git.eden-emu.dev/eden-emu/eden/pulls/4099 Reviewed-by: Lizzie Reviewed-by: MaranBr --- src/core/file_sys/registered_cache.cpp | 3 ++ src/qt_common/game_list/model.cpp | 8 +++++ src/qt_common/game_list/model.h | 4 +++ src/yuzu/game/game_list.cpp | 45 ++++++++++++++++++-------- 4 files changed, 47 insertions(+), 13 deletions(-) diff --git a/src/core/file_sys/registered_cache.cpp b/src/core/file_sys/registered_cache.cpp index af41820a36..ebecbaf74f 100644 --- a/src/core/file_sys/registered_cache.cpp +++ b/src/core/file_sys/registered_cache.cpp @@ -503,6 +503,9 @@ NcaID PlaceholderCache::Generate() { VirtualFile RegisteredCache::OpenFileOrDirectoryConcat(const VirtualDir& open_dir, std::string_view path) const { + if (open_dir == nullptr) { + return nullptr; + } const auto file = open_dir->GetFileRelative(path); if (file != nullptr) { return file; diff --git a/src/qt_common/game_list/model.cpp b/src/qt_common/game_list/model.cpp index 2f8deaa0c5..91ec80c525 100644 --- a/src/qt_common/game_list/model.cpp +++ b/src/qt_common/game_list/model.cpp @@ -57,6 +57,12 @@ void GameListModel::PopulateAsync(QVector& game_dirs) { QThreadPool::globalInstance()->start(current_worker.get()); } +void GameListModel::StopWorker() { + // ~GameListWorker sets stop_requested and blocks until run() finishes, so this returns only + // once the worker is no longer touching the content providers. + current_worker.reset(); +} + void GameListModel::WorkerEvent() { current_worker->ProcessEvents(this); } @@ -202,6 +208,7 @@ void GameListModel::RefreshGameDirectory() { if (!UISettings::values.game_dirs.empty() && current_worker != nullptr) { LOG_INFO(Frontend, "Change detected in the games directory. Reloading game list."); + StopWorker(); QtCommon::system->GetFileSystemController().CreateFactories(*QtCommon::vfs); PopulateAsync(UISettings::values.game_dirs); } @@ -210,6 +217,7 @@ void GameListModel::RefreshGameDirectory() { void GameListModel::RefreshExternalContent() { if (!UISettings::values.game_dirs.empty() && current_worker != nullptr) { LOG_INFO(Frontend, "External content directory changed. Clearing metadata cache."); + StopWorker(); QtCommon::Game::ResetMetadata(false); QtCommon::system->GetFileSystemController().CreateFactories(*QtCommon::vfs); PopulateAsync(UISettings::values.game_dirs); diff --git a/src/qt_common/game_list/model.h b/src/qt_common/game_list/model.h index d0a116beb6..ce1fcc87b7 100644 --- a/src/qt_common/game_list/model.h +++ b/src/qt_common/game_list/model.h @@ -52,6 +52,10 @@ public: void DonePopulating(const QStringList& watch_list); void PopulateAsync(QVector& game_dirs); + // Stops and joins the running populate worker, if any. Must be called before rebuilding the + // content providers (CreateFactories), otherwise the worker keeps scanning a cache that is + // being torn down underneath it. + void StopWorker(); void WorkerEvent(); bool IsEmpty() const; diff --git a/src/yuzu/game/game_list.cpp b/src/yuzu/game/game_list.cpp index 521c5607f5..3ea84ba0cb 100644 --- a/src/yuzu/game/game_list.cpp +++ b/src/yuzu/game/game_list.cpp @@ -257,30 +257,47 @@ void GameList::OnPopulatingCompleted(const QStringList& watch_list) { // Clear out the old directories to watch for changes and add the new ones auto* watcher = item_model->GetWatcher(); - auto watch_dirs = watcher->directories(); - if (!watch_dirs.isEmpty()) { - watcher->removePaths(watch_dirs); - } + const QStringList current_watch = watcher->directories(); constexpr int LIMIT_WATCH_DIRECTORIES = 5000; constexpr int SLICE_SIZE = 25; - int len = (std::min)(static_cast(watch_list.size()), LIMIT_WATCH_DIRECTORIES); + + QStringList desired_watch = watch_list; + if (desired_watch.size() > LIMIT_WATCH_DIRECTORIES) { + desired_watch = desired_watch.mid(0, LIMIT_WATCH_DIRECTORIES); + } + + // Only re-arm the watcher when the set of directories actually changed. Re-adding the same + // paths makes the macOS QFileSystemWatcher re-emit directoryChanged (the FSEvent arrives + // asynchronously, so the blockSignals guard below does not catch it), which retriggers a full + // refresh and loops forever, making the game list visibly flash. Comparing the sets breaks + // that loop: at steady state we leave the watcher untouched and no spurious event is produced. + QStringList sorted_current = current_watch; + QStringList sorted_desired = desired_watch; + sorted_current.sort(); + sorted_desired.sort(); + + if (sorted_current != sorted_desired) { + if (!current_watch.isEmpty()) { + watcher->removePaths(current_watch); + } #ifdef __APPLE__ - const bool old_signals_blocked = watcher->blockSignals(true); + const bool old_signals_blocked = watcher->blockSignals(true); #endif - for (int i = 0; i < len; i += SLICE_SIZE) { - auto chunk = watch_list.mid(i, SLICE_SIZE); - if (!chunk.isEmpty()) { - watcher->addPaths(chunk); + for (int i = 0; i < desired_watch.size(); i += SLICE_SIZE) { + auto chunk = desired_watch.mid(i, SLICE_SIZE); + if (!chunk.isEmpty()) { + watcher->addPaths(chunk); + } + QCoreApplication::processEvents(); } - QCoreApplication::processEvents(); - } #ifdef __APPLE__ - watcher->blockSignals(old_signals_blocked); + watcher->blockSignals(old_signals_blocked); #endif + } m_currentView->setEnabled(true); @@ -304,6 +321,7 @@ void GameList::RefreshGameDirectory() { if (!UISettings::values.game_dirs.empty()) { LOG_INFO(Frontend, "Change detected in the games directory. Reloading game list."); + item_model->StopWorker(); QtCommon::system->GetFileSystemController().CreateFactories(*QtCommon::vfs); PopulateAsync(UISettings::values.game_dirs); } @@ -312,6 +330,7 @@ void GameList::RefreshGameDirectory() { void GameList::RefreshExternalContent() { if (!UISettings::values.game_dirs.empty()) { LOG_INFO(Frontend, "External content directory changed. Clearing metadata cache."); + item_model->StopWorker(); QtCommon::Game::ResetMetadata(false); QtCommon::system->GetFileSystemController().CreateFactories(*QtCommon::vfs); PopulateAsync(UISettings::values.game_dirs);