Fix race conditions when reading game title.

Fixes #5030.
hrydgard · Dec 24, 2015 · e6b5fbb · e6b5fbb
1 parent bc9c3ca
commit e6b5fbb
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 16 deletions.
diff --git 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
@@ -116,15 +116,14 @@ class GameInfo {
 
 	std::vector<std::string> 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,
 	// and obviously also not when creating it and holding the only pointer
 	// 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
@@ -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
@@ -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
@@ -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;
 }