From e6b5fbb40a1ebc51865f0a0dea9cd84b2cf3b9e0 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Thu, 24 Dec 2015 13:18:46 -0800 Subject: [PATCH] Fix race conditions when reading game title. Fixes #5030. --- UI/GameInfoCache.cpp | 12 ++++++++---- UI/GameInfoCache.h | 6 ++++-- UI/GameScreen.cpp | 6 +++--- UI/MainScreen.cpp | 8 ++++---- UI/SavedataScreen.cpp | 7 ++++--- 5 files changed, 23 insertions(+), 16 deletions(-) diff --git a/UI/GameInfoCache.cpp b/UI/GameInfoCache.cpp index 1c62ff1bc977..75f80c79d18b 100644 --- a/UI/GameInfoCache.cpp +++ b/UI/GameInfoCache.cpp @@ -208,6 +208,9 @@ bool GameInfo::LoadFromPath(const std::string &gamePath) { delete fileLoader; fileLoader = ConstructFileLoader(gamePath); filePath_ = gamePath; + + // This is a fallback title, while we're loading / if unable to load. + title = File::GetFilename(filePath_); } return GetFileLoader()->Exists(); @@ -279,6 +282,11 @@ void GameInfo::ParseParamSFO() { paramSFOLoaded = true; } +std::string GameInfo::GetTitle() { + lock_guard guard(lock); + return title; +} + static bool ReadFileToString(IFileSystem *fs, const char *filename, std::string *contents, recursive_mutex *mtx) { PSPFileInfo info = fs->GetFileInfo(filename); if (!info.exists) { @@ -331,10 +339,7 @@ class GameInfoWorkItem : public PrioritizedWorkQueueItem { std::string filename = gamePath_; { lock_guard lock(info_->lock); - info_->path = gamePath_; info_->fileType = Identify_File(info_->GetFileLoader()); - // Fallback title - info_->title = File::GetFilename(info_->path); } switch (info_->fileType) { @@ -403,7 +408,6 @@ class GameInfoWorkItem : public PrioritizedWorkQueueItem { // An elf on its own has no usable information, no icons, no nothing. { lock_guard lock(info_->lock); - info_->title = File::GetFilename(filename); info_->id = "ELF000000"; info_->id_version = "ELF000000_1.00"; info_->paramSFOLoaded = true; diff --git a/UI/GameInfoCache.h b/UI/GameInfoCache.h index fa448272e584..74d2823a1a26 100644 --- a/UI/GameInfoCache.h +++ b/UI/GameInfoCache.h @@ -116,6 +116,7 @@ class GameInfo { std::vector GetSaveDataDirectories(); + std::string GetTitle(); // Hold this when reading or writing from the GameInfo. // Don't need to hold it when just passing around the pointer, @@ -123,8 +124,6 @@ class GameInfo { // to it. recursive_mutex lock; - std::string path; - std::string title; // for easy access, also available in paramSFO. std::string id; std::string id_version; int disc_total; @@ -166,6 +165,9 @@ class GameInfo { bool pending; protected: + // Note: this can change while loading, use GetTitle(). + std::string title; + FileLoader *fileLoader; std::string filePath_; }; diff --git a/UI/GameScreen.cpp b/UI/GameScreen.cpp index 30f7d53e8fe2..9824068c5e63 100644 --- a/UI/GameScreen.cpp +++ b/UI/GameScreen.cpp @@ -66,7 +66,7 @@ void GameScreen::CreateViews() { leftColumn->Add(new Choice(di->T("Back"), "", false, new AnchorLayoutParams(150, WRAP_CONTENT, 10, NONE, NONE, 10)))->OnClick.Handle(this, &GameScreen::OnSwitchBack); if (info) { texvGameIcon_ = leftColumn->Add(new Thin3DTextureView(0, IS_DEFAULT, new AnchorLayoutParams(144 * 2, 80 * 2, 10, 10, NONE, NONE))); - tvTitle_ = leftColumn->Add(new TextView(info->title, ALIGN_LEFT, false, new AnchorLayoutParams(10, 200, NONE, NONE))); + tvTitle_ = leftColumn->Add(new TextView(info->GetTitle(), ALIGN_LEFT, false, new AnchorLayoutParams(10, 200, NONE, NONE))); tvTitle_->SetShadow(true); // This one doesn't need to be updated. leftColumn->Add(new TextView(gamePath_, ALIGN_LEFT, true, new AnchorLayoutParams(10, 250, NONE, NONE)))->SetShadow(true); @@ -159,7 +159,7 @@ void GameScreen::update(InputState &input) { GameInfo *info = g_gameInfoCache.GetInfo(thin3d, gamePath_, GAMEINFO_WANTBG | GAMEINFO_WANTSIZE); if (tvTitle_) - tvTitle_->SetText(info->title + " (" + info->id + ")"); + tvTitle_->SetText(info->GetTitle() + " (" + info->id + ")"); if (info->iconTexture && texvGameIcon_) { texvGameIcon_->SetTexture(info->iconTexture); // Fade the icon with the background. @@ -277,7 +277,7 @@ void GameScreen::CallbackDeleteGame(bool yes) { UI::EventReturn GameScreen::OnCreateShortcut(UI::EventParams &e) { GameInfo *info = g_gameInfoCache.GetInfo(NULL, gamePath_, 0); if (info) { - host->CreateDesktopShortcut(gamePath_, info->title); + host->CreateDesktopShortcut(gamePath_, info->GetTitle()); } return UI::EVENT_DONE; } diff --git a/UI/MainScreen.cpp b/UI/MainScreen.cpp index e498e8b86344..755d4e8bef06 100644 --- a/UI/MainScreen.cpp +++ b/UI/MainScreen.cpp @@ -277,8 +277,9 @@ void GameButton::Draw(UIContext &dc) { float tw, th; dc.Draw()->Flush(); dc.PushScissor(bounds_); - if (title_.empty() && !ginfo->title.empty()) { - title_ = ReplaceAll(ginfo->title + discNumInfo, "&", "&&"); + const std::string currentTitle = ginfo->GetTitle(); + if (!currentTitle.empty()) { + title_ = ReplaceAll(currentTitle + discNumInfo, "&", "&&"); title_ = ReplaceAll(title_, "\n", " "); } @@ -310,8 +311,7 @@ void GameButton::Draw(UIContext &dc) { } else { dc.Draw()->Flush(); } - if (!ginfo->id.empty() && ginfo->hasConfig) - { + if (ginfo->hasConfig && !ginfo->id.empty()) { dc.Draw()->DrawImage(I_GEAR, x, y + h - ui_images[I_GEAR].h, 1.0f); } if (overlayColor) { diff --git a/UI/SavedataScreen.cpp b/UI/SavedataScreen.cpp index 5e9b1d864494..9f8b47a34d8f 100644 --- a/UI/SavedataScreen.cpp +++ b/UI/SavedataScreen.cpp @@ -226,8 +226,9 @@ void SavedataButton::Draw(UIContext &dc) { dc.Draw()->Flush(); dc.PushScissor(bounds_); - if (title_.empty() && !ginfo->title.empty()) { - title_ = CleanSaveString(ginfo->title); + const std::string currentTitle = ginfo->GetTitle(); + if (!currentTitle.empty()) { + title_ = CleanSaveString(currentTitle); } if (subtitle_.empty() && ginfo->gameSize > 0) { std::string savedata_title = ginfo->paramSFO.GetValueString("SAVEDATA_TITLE"); @@ -360,7 +361,7 @@ void SavedataScreen::CreateViews() { UI::EventReturn SavedataScreen::OnSavedataButtonClick(UI::EventParams &e) { GameInfo *ginfo = g_gameInfoCache.GetInfo(screenManager()->getThin3DContext(), e.s, 0); - screenManager()->push(new SavedataPopupScreen(e.s, ginfo->title)); + screenManager()->push(new SavedataPopupScreen(e.s, ginfo->GetTitle())); // the game path: e.s; return UI::EVENT_DONE; }