From bdc68d659ab71ab6c307d2d822858b6c63a9372e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 23 Aug 2026 18:04:18 +0200 Subject: [PATCH] GameInfoCache: Don't let a work item switch on an unidentified fileType GetInfo() masked out any flag that a *pending* work item was already going to fetch, FILE_TYPE included. But every work item starts by switching on info->fileType, so "another item will compute it" isn't good enough - if that item hadn't reached Identify_File yet, the second one fell through to default:, marked its flags ready and loaded nothing. The data then looked present forever, so e.g. a PIC1 requested while an ICON load was in flight could just never show up. Easy to hit since the screens request different flag combinations for the same path, and BackgroundAudio calls GetInfo from the audio thread. Always redo the identification unless FILE_TYPE is already in hasFlags (i.e. final), and have Run() switch on a local copy so a concurrent item can't shift it underneath us mid-switch. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FzzCUp8y1ahgVueb1Cq92Y --- UI/GameInfoCache.cpp | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/UI/GameInfoCache.cpp b/UI/GameInfoCache.cpp index 6e98c48177..1f1b07de07 100644 --- a/UI/GameInfoCache.cpp +++ b/UI/GameInfoCache.cpp @@ -569,16 +569,25 @@ public: std::string errorString; + // Work on a local copy: another work item for the same GameInfo can be running alongside us, + // so info_->fileType isn't stable to switch on. GetInfo() guarantees we either compute it + // ourselves here, or that it's already in hasFlags and thus final. + IdentifiedFileType fileType; if (flags_ & GameInfoFlags::FILE_TYPE) { - info_->fileType = Identify_File(info_->GetFileLoader().get(), &errorString); + fileType = Identify_File(info_->GetFileLoader().get(), &errorString); + std::lock_guard lock(info_->lock); + info_->fileType = fileType; + } else { + std::lock_guard lock(info_->lock); + fileType = info_->fileType; } - switch (info_->fileType) { + switch (fileType) { case IdentifiedFileType::PSP_PBP: case IdentifiedFileType::PSP_PBP_DIRECTORY: { auto pbpLoader = info_->GetFileLoader(); - if (info_->fileType == IdentifiedFileType::PSP_PBP_DIRECTORY) { + if (fileType == IdentifiedFileType::PSP_PBP_DIRECTORY) { Path ebootPath = ResolvePBPFile(gamePath_); if (ebootPath != gamePath_) { pbpLoader.reset(ConstructFileLoader(ebootPath)); @@ -611,12 +620,12 @@ public: if (pbp.GetSubFile(PBP_PARAM_SFO, &sfoData)) { std::lock_guard lock(info_->lock); info_->paramSFO.ReadSFO(sfoData); - info_->ParseParamSFO(info_->fileType); + info_->ParseParamSFO(fileType); // Assuming PSP_PBP_DIRECTORY without ID or with disc_total < 1 in GAME dir must be homebrew if ((info_->id.empty() || !info_->disc_total) && gamePath_.FilePathContainsNoCase("PSP/GAME/") - && info_->fileType == IdentifiedFileType::PSP_PBP_DIRECTORY) { + && fileType == IdentifiedFileType::PSP_PBP_DIRECTORY) { info_->id = g_paramSFO.GenerateFakeID(gamePath_); info_->id_version = info_->id + "_1.00"; info_->region = GameRegion::HOMEBREW; // Homebrew @@ -724,7 +733,7 @@ handleELF: if (ReadFileToString(&umd, "/PARAM.SFO", ¶mSFOcontents, 0)) { std::lock_guard lock(info_->lock); info_->paramSFO.ReadSFO((const u8 *)paramSFOcontents.data(), paramSFOcontents.size()); - info_->ParseParamSFO(info_->fileType); + info_->ParseParamSFO(fileType); info_->MarkReadyNoLock(GameInfoFlags::PARAM_SFO); } } @@ -781,7 +790,7 @@ handleELF: if (ReadFileToString(&umd, "/PSP_GAME/PARAM.SFO", ¶mSFOcontents, nullptr)) { std::lock_guard lock(info_->lock); info_->paramSFO.ReadSFO((const u8 *)paramSFOcontents.data(), paramSFOcontents.size()); - info_->ParseParamSFO(info_->fileType); + info_->ParseParamSFO(fileType); info_->MarkReadyNoLock(GameInfoFlags::PARAM_SFO); } } @@ -812,7 +821,7 @@ handleELF: case IdentifiedFileType::PSP_ISO_NP: case IdentifiedFileType::PSP_UMD_VIDEO_ISO: { - std::string_view gameRoot = info_->fileType == IdentifiedFileType::PSP_UMD_VIDEO_ISO ? "/UMD_VIDEO/" : "/PSP_GAME/"; + std::string_view gameRoot = fileType == IdentifiedFileType::PSP_UMD_VIDEO_ISO ? "/UMD_VIDEO/" : "/PSP_GAME/"; SequentialHandleAllocator handles; // Let's assume it's an ISO. // TODO: This will currently read in the whole directory tree. Not really necessary for just a @@ -842,7 +851,7 @@ handleELF: { std::lock_guard lock(info_->lock); info_->paramSFO.ReadSFO((const u8 *)paramSFOcontents.data(), paramSFOcontents.size()); - info_->ParseParamSFO(info_->fileType); + info_->ParseParamSFO(fileType); // quick-update the info while we have the lock, so we don't need to wait for the image load to display the title. info_->MarkReadyNoLock(GameInfoFlags::PARAM_SFO); @@ -930,7 +939,7 @@ handleELF: } if (flags_ & GameInfoFlags::SAVEDATA_SIZE) { - switch (info_->fileType) { + switch (fileType) { case IdentifiedFileType::PSP_ISO: case IdentifiedFileType::PSP_ISO_NP: case IdentifiedFileType::PSP_DISC_DIRECTORY: @@ -1090,6 +1099,12 @@ std::shared_ptr GameInfoCache::GetInfo(Draw::DrawContext *draw, const } GameInfoFlags willHaveFlags = info->hasFlags | info->pendingFlags; // We don't want to re-fetch data that we have, so or in pendingFlags. wanted = (GameInfoFlags)((int)wantFlags & ~(int)willHaveFlags); // & is reserved for testing so we have to cast to int. ugh. + // FILE_TYPE is special: every work item switches on info->fileType, so it's not enough that + // some *pending* item is going to compute it - that item may not have got there yet, and we'd + // switch on UNKNOWN and mark our flags ready having loaded nothing. Cheap enough to redo. + if (wanted != GameInfoFlags::EMPTY && !(info->hasFlags & GameInfoFlags::FILE_TYPE)) { + wanted |= GameInfoFlags::FILE_TYPE; + } info->pendingFlags |= wanted; if (outHasFlags) { *outHasFlags = info->hasFlags;