Browse Source

[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>
master
BoiledElectricity 1 day ago
committed by crueter
parent
commit
ce14fc91fb
No known key found for this signature in database GPG Key ID: 425ACD2D4830EBC6
  1. 3
      src/core/file_sys/registered_cache.cpp
  2. 8
      src/qt_common/game_list/model.cpp
  3. 4
      src/qt_common/game_list/model.h
  4. 45
      src/yuzu/game/game_list.cpp

3
src/core/file_sys/registered_cache.cpp

@ -503,6 +503,9 @@ NcaID PlaceholderCache::Generate() {
VirtualFile RegisteredCache::OpenFileOrDirectoryConcat(const VirtualDir& open_dir, VirtualFile RegisteredCache::OpenFileOrDirectoryConcat(const VirtualDir& open_dir,
std::string_view path) const { std::string_view path) const {
if (open_dir == nullptr) {
return nullptr;
}
const auto file = open_dir->GetFileRelative(path); const auto file = open_dir->GetFileRelative(path);
if (file != nullptr) { if (file != nullptr) {
return file; return file;

8
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()); 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() { void GameListModel::WorkerEvent() {
current_worker->ProcessEvents(this); current_worker->ProcessEvents(this);
} }
@ -202,6 +208,7 @@ void GameListModel::RefreshGameDirectory() {
if (!UISettings::values.game_dirs.empty() && current_worker != nullptr) { if (!UISettings::values.game_dirs.empty() && current_worker != nullptr) {
LOG_INFO(Frontend, "Change detected in the games directory. Reloading game list."); LOG_INFO(Frontend, "Change detected in the games directory. Reloading game list.");
StopWorker();
QtCommon::system->GetFileSystemController().CreateFactories(*QtCommon::vfs); QtCommon::system->GetFileSystemController().CreateFactories(*QtCommon::vfs);
PopulateAsync(UISettings::values.game_dirs); PopulateAsync(UISettings::values.game_dirs);
} }
@ -210,6 +217,7 @@ void GameListModel::RefreshGameDirectory() {
void GameListModel::RefreshExternalContent() { void GameListModel::RefreshExternalContent() {
if (!UISettings::values.game_dirs.empty() && current_worker != nullptr) { if (!UISettings::values.game_dirs.empty() && current_worker != nullptr) {
LOG_INFO(Frontend, "External content directory changed. Clearing metadata cache."); LOG_INFO(Frontend, "External content directory changed. Clearing metadata cache.");
StopWorker();
QtCommon::Game::ResetMetadata(false); QtCommon::Game::ResetMetadata(false);
QtCommon::system->GetFileSystemController().CreateFactories(*QtCommon::vfs); QtCommon::system->GetFileSystemController().CreateFactories(*QtCommon::vfs);
PopulateAsync(UISettings::values.game_dirs); PopulateAsync(UISettings::values.game_dirs);

4
src/qt_common/game_list/model.h

@ -52,6 +52,10 @@ public:
void DonePopulating(const QStringList& watch_list); void DonePopulating(const QStringList& watch_list);
void PopulateAsync(QVector<UISettings::GameDir>& game_dirs); 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(); void WorkerEvent();
bool IsEmpty() const; bool IsEmpty() const;

45
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 // Clear out the old directories to watch for changes and add the new ones
auto* watcher = item_model->GetWatcher(); 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 LIMIT_WATCH_DIRECTORIES = 5000;
constexpr int SLICE_SIZE = 25; 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__ #ifdef __APPLE__
const bool old_signals_blocked = watcher->blockSignals(true);
const bool old_signals_blocked = watcher->blockSignals(true);
#endif #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__ #ifdef __APPLE__
watcher->blockSignals(old_signals_blocked);
watcher->blockSignals(old_signals_blocked);
#endif #endif
}
m_currentView->setEnabled(true); m_currentView->setEnabled(true);
@ -304,6 +321,7 @@ void GameList::RefreshGameDirectory() {
if (!UISettings::values.game_dirs.empty()) { if (!UISettings::values.game_dirs.empty()) {
LOG_INFO(Frontend, "Change detected in the games directory. Reloading game list."); LOG_INFO(Frontend, "Change detected in the games directory. Reloading game list.");
item_model->StopWorker();
QtCommon::system->GetFileSystemController().CreateFactories(*QtCommon::vfs); QtCommon::system->GetFileSystemController().CreateFactories(*QtCommon::vfs);
PopulateAsync(UISettings::values.game_dirs); PopulateAsync(UISettings::values.game_dirs);
} }
@ -312,6 +330,7 @@ void GameList::RefreshGameDirectory() {
void GameList::RefreshExternalContent() { void GameList::RefreshExternalContent() {
if (!UISettings::values.game_dirs.empty()) { if (!UISettings::values.game_dirs.empty()) {
LOG_INFO(Frontend, "External content directory changed. Clearing metadata cache."); LOG_INFO(Frontend, "External content directory changed. Clearing metadata cache.");
item_model->StopWorker();
QtCommon::Game::ResetMetadata(false); QtCommon::Game::ResetMetadata(false);
QtCommon::system->GetFileSystemController().CreateFactories(*QtCommon::vfs); QtCommon::system->GetFileSystemController().CreateFactories(*QtCommon::vfs);
PopulateAsync(UISettings::values.game_dirs); PopulateAsync(UISettings::values.game_dirs);

Loading…
Cancel
Save