[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 <lizzie@eden-emu.dev>
Reviewed-by: MaranBr <maranbr@eden-emu.dev>

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<UISettings::GameDir>& 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<UISettings::GameDir>& 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<int>(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);