From aa603905dbcb377fb33c3de8dfb0a6176aaf4c29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 10:43:00 -0600 Subject: [PATCH 01/27] sceIo: Log sceIoWaitAsync with nothing pending at debug level Co-Authored-By: Claude Opus 5.5 (1M context) --- Core/HLE/sceIo.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Core/HLE/sceIo.cpp b/Core/HLE/sceIo.cpp index 4d12e598da..7eb1bdbcdd 100644 --- a/Core/HLE/sceIo.cpp +++ b/Core/HLE/sceIo.cpp @@ -2414,10 +2414,9 @@ static int sceIoWaitAsync(int id, u32 address) { return hleLogDebug(Log::sceIo, 0, "complete"); } else { - // This is normal in some games. - return hleLogInfo(Log::sceIo, SCE_KERNEL_ERROR_NOASYNC, "no async pending"); + // This is normal. + return hleLogDebug(Log::sceIo, SCE_KERNEL_ERROR_NOASYNC, "no async pending"); } - return 0; //completed } else { return hleLogError(Log::sceIo, SCE_KERNEL_ERROR_BADF, "invalid fd"); } From 6f82ae1c0ccbaa5e0a4687326d7e139af973692c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 10:48:29 -0600 Subject: [PATCH 02/27] Bump rcheevos --- ext/rcheevos | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/rcheevos b/ext/rcheevos index 2ac45d357b..1433173220 160000 --- a/ext/rcheevos +++ b/ext/rcheevos @@ -1 +1 @@ -Subproject commit 2ac45d357bce2906bb0f1438f3eaf8ce6e78e3c4 +Subproject commit 1433173220a7eaede6a9ed7a18e94117be1821e0 From 00208d3d8044c4847ee92e45c265d9d405bb48aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 10:51:07 -0600 Subject: [PATCH 03/27] Achievements: Enable HTTPS on UWP, retry failed icon downloads UWP compiled naett with the WinHTTP backend but never reported SYSPROP_SUPPORTS_HTTPS, so achievement traffic and badge icons went over plain http. When the media server redirected to https, icons failed. A failed icon download also left the URL marked pending, so it was never retried for the rest of the session. Mark it failed instead, and let IconCache retry it after 30 seconds. The set-completed badge also skipped RemoveHttpsIfNeeded. Co-Authored-By: Claude Opus 5.5 (1M context) --- Common/UI/IconCache.cpp | 20 ++++++++++++++++++++ Common/UI/IconCache.h | 3 +++ Core/RetroAchievements.cpp | 15 ++++++++++----- UWP/PPSSPP_UWPMain.cpp | 2 ++ 4 files changed, 35 insertions(+), 5 deletions(-) diff --git a/Common/UI/IconCache.cpp b/Common/UI/IconCache.cpp index 598aad75aa..0262cd9910 100644 --- a/Common/UI/IconCache.cpp +++ b/Common/UI/IconCache.cpp @@ -14,6 +14,9 @@ #define MAX_RUNTIME_CACHE_SIZE (1024 * 1024 * 4) #define MAX_SAVED_CACHE_SIZE (1024 * 1024 * 1) +// Seconds before MarkPending accepts a key whose download failed. +constexpr double FAILED_RETRY_DELAY = 30.0; + constexpr uint32_t ICON_CACHE_MAGIC = MK_FOURCC("pICN"); IconCache g_iconCache; @@ -127,6 +130,7 @@ void IconCache::ClearData() { ClearTextures(); std::unique_lock lock(lock_); cache_.clear(); + failed_.clear(); } void IconCache::FrameUpdate() { @@ -231,10 +235,26 @@ bool IconCache::MarkPending(std::string_view key) { if (pending_.find(key) != pending_.end()) { return false; } + auto failedIter = failed_.find(key); + if (failedIter != failed_.end()) { + if (time_now_d() < failedIter->second + FAILED_RETRY_DELAY) { + return false; + } + failed_.erase(failedIter); + } pending_.emplace(key); return true; } +void IconCache::MarkFailed(std::string_view key) { + std::unique_lock lock(lock_); + auto iter = pending_.find(key); + if (iter != pending_.end()) { + pending_.erase(iter); + } + failed_[std::string(key)] = time_now_d(); +} + void IconCache::CancelPending(std::string_view key) { std::unique_lock lock(lock_); auto iter = pending_.find(key); diff --git a/Common/UI/IconCache.h b/Common/UI/IconCache.h index e631f77ca6..1b22e9ba0f 100644 --- a/Common/UI/IconCache.h +++ b/Common/UI/IconCache.h @@ -37,6 +37,8 @@ public: // It's okay to call these from any thread. bool MarkPending(std::string_view key); // returns false if already pending or loaded void CancelPending(std::string_view key); + // Like CancelPending, but MarkPending refuses the key for a while, so a failing download isn't retried every frame. + void MarkFailed(std::string_view key); bool InsertIcon(std::string_view key, IconFormat format, std::string &&pngData); bool GetDimensions(std::string_view key, int *width, int *height); bool Contains(std::string_view key); @@ -65,6 +67,7 @@ private: std::map> cache_; std::set> pending_; + std::map> failed_; // key -> time of failure std::mutex lock_; diff --git a/Core/RetroAchievements.cpp b/Core/RetroAchievements.cpp index 5519b25ee4..e0aa262d6a 100644 --- a/Core/RetroAchievements.cpp +++ b/Core/RetroAchievements.cpp @@ -351,11 +351,11 @@ static void event_handler_callback(const rc_client_event_t *event, rc_client_t * const rc_client_game_t *gameInfo = rc_client_get_game_info(g_rcClient); std::string setTitle = gameInfo->title; - std::string badgeUrl = gameInfo->badge_url; + std::string badgeUrl = http::RemoveHttpsIfNeeded(gameInfo->badge_url); if (event->type == RC_CLIENT_EVENT_SUBSET_COMPLETED) { const rc_client_subset_t *subset = event->subset; setTitle = subset->title; - badgeUrl = subset->badge_url; + badgeUrl = http::RemoveHttpsIfNeeded(subset->badge_url); } DownloadImageIfMissing(badgeUrl); @@ -964,10 +964,15 @@ void DownloadImageIfMissing(std::string_view url) { if (g_iconCache.MarkPending(url)) { INFO_LOG(Log::Achievements, "Downloading image: %.*s", STR_VIEW(url)); g_DownloadManager.StartDownload(url, Path(), http::RequestFlags::Default, nullptr, "", [](http::Request &download) { - if (download.ResultCode() != 200) - return; std::string data; - download.buffer().TakeAll(&data); + if (download.ResultCode() == 200) { + download.buffer().TakeAll(&data); + } + if (data.empty()) { + WARN_LOG(Log::Achievements, "Failed to download image (%d): %s", download.ResultCode(), download.url().c_str()); + g_iconCache.MarkFailed(download.url()); + return; + } g_iconCache.InsertIcon(download.url(), IconFormat::PNG, std::move(data)); }); } diff --git a/UWP/PPSSPP_UWPMain.cpp b/UWP/PPSSPP_UWPMain.cpp index 2b64b1f191..30dfe8ab98 100644 --- a/UWP/PPSSPP_UWPMain.cpp +++ b/UWP/PPSSPP_UWPMain.cpp @@ -494,6 +494,8 @@ bool System_GetPropertyBool(SystemProperty prop) { } case SYSPROP_DEBUGGER_PRESENT: return IsDebuggerPresent(); + case SYSPROP_SUPPORTS_HTTPS: + return !g_Config.bDisableHTTPS; case SYSPROP_OK_BUTTON_LEFT: return true; case SYSPROP_CAN_GET_FREE_SPACE_FAST: From c27a79363d94651b1e24e95a7d27fd4d039559e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 10:57:55 -0600 Subject: [PATCH 04/27] docs: UWP build command, and the x86-hosted compiler PCH failure on ARM64 Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/building.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docs/building.md b/docs/building.md index f6f06fc631..46443f1a13 100644 --- a/docs/building.md +++ b/docs/building.md @@ -69,6 +69,21 @@ bug, since CI runs the equivalent of `UnitTest.exe all` on every commit across m platforms without apparent issue. If `all`/`Jit` hangs in your environment, run every other test by name instead (skip `Jit`) to still get real coverage. +## UWP build + +UWP has its own solution, `UWP\PPSSPP_UWP.sln`. Build the app with `/t:PPSSPP_UWP` (or a single +library such as `/t:CoreUWP`), same MSBuild setup as above: + +```powershell +& $msbuild "UWP\PPSSPP_UWP.sln" /t:PPSSPP_UWP /p:Configuration=Debug /p:Platform= /p:PreferredToolArchitecture=arm64 /m +``` + +On an ARM64 host, pass `/p:PreferredToolArchitecture=arm64`. Without it the UWP +projects use the 32-bit x86-hosted compiler (`bin\HostX86\arm64\CL.exe`), which runs emulated and +can't map the precompiled headers into its address space. Every file then fails at line 1 with +`C3859: Failed to create virtual memory for PCH` and `C1076: compiler limit: internal heap limit +reached`. That looks like running out of memory, but lowering `/m` or `CL_MPCount` doesn't help. + ## Android assets The APK's `assets/` directory is the repo-root `assets/` directory, wired up by From 2291a4a9a7c9f0c1f45e0be9f015e3cadda94344 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:05:45 -0600 Subject: [PATCH 05/27] Achievements: Don't leave boot blocked after an aborted login or load rc_client doesn't call back for a game load aborted by unload, or for anything pending when the client is destroyed. g_isIdentifying or g_isLoggingIn then stayed set, and IsBlockingExecution kept every later boot on a black screen until restart. Quitting during identification, or disabling achievements during a token login, was enough. Co-Authored-By: Claude Opus 5.5 (1M context) --- Core/RetroAchievements.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Core/RetroAchievements.cpp b/Core/RetroAchievements.cpp index e0aa262d6a..c2c6853f2b 100644 --- a/Core/RetroAchievements.cpp +++ b/Core/RetroAchievements.cpp @@ -801,6 +801,9 @@ bool LoginAsync(const char *username, const char *password) { void Logout() { rc_client_logout(g_rcClient); + // An aborted login or game load doesn't always call back, so clear the flags here. + g_isLoggingIn = false; + g_isIdentifying = false; // remove secret from config NativeClearSecret(RA_TOKEN_SECRET_NAME); g_Config.Save("Achievements logout"); @@ -832,6 +835,9 @@ bool Shutdown() { g_rcClient = nullptr; INFO_LOG(Log::Achievements, "Achievements shut down."); } + // A destroyed client doesn't call back for pending logins or game loads. + g_isLoggingIn = false; + g_isIdentifying = false; return true; } @@ -1157,6 +1163,8 @@ void UnloadGame() { g_gamePath.clear(); s_game_hash.clear(); } + // Unloading aborts a pending load, and rc_client doesn't call back for an aborted load. + g_isIdentifying = false; } void change_media_callback(int result, const char *error_message, rc_client_t *client, void *userdata) { From fde1a467ca550c3034c63e49367e862be7106092 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:06:15 -0600 Subject: [PATCH 06/27] Achievements: Report transport failures to rc_client as retryable naett reports a connection failure as -1, which rcheevos reads as RC_API_SERVER_RESPONSE_CLIENT_ERROR, a non-retryable client error. An unlock or leaderboard submission during a network dropout was then treated as a server rejection and dropped, on every platform using naett. Map any non-HTTP result to RC_API_SERVER_RESPONSE_RETRYABLE_CLIENT_ERROR. Co-Authored-By: Claude Opus 5.5 (1M context) --- Core/RetroAchievements.cpp | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/Core/RetroAchievements.cpp b/Core/RetroAchievements.cpp index c2c6853f2b..e82bd0b780 100644 --- a/Core/RetroAchievements.cpp +++ b/Core/RetroAchievements.cpp @@ -290,6 +290,25 @@ static uint32_t read_memory_callback(uint32_t address, uint8_t *buffer, uint32_t return num_bytes; } +static void complete_server_call(http::Request &download, rc_client_server_callback_t callback, void *callback_data) { + std::string buffer; + download.buffer().TakeAll(&buffer); + rc_api_server_response_t response{}; + int resultCode = download.ResultCode(); + if (resultCode <= 0) { + // No HTTP status: the request failed in transport (naett's codes are negative, and + // naettConnectionError is -1, which rcheevos reads as a non-retryable client error). + // Report it as retryable so that unlocks and leaderboard submissions aren't dropped. + buffer = StringFromFormat("Network error (%d)", resultCode); + response.http_status_code = RC_API_SERVER_RESPONSE_RETRYABLE_CLIENT_ERROR; + } else { + response.http_status_code = resultCode; + } + response.body = buffer.c_str(); + response.body_length = buffer.size(); + callback(&response, callback_data); +} + // This is the HTTP request dispatcher that is provided to the rc_client. Whenever the client // needs to talk to the server, it will call this function. static void server_call_callback(const rc_api_request_t *request, @@ -301,25 +320,13 @@ static void server_call_callback(const rc_api_request_t *request, if (request->post_data) { std::shared_ptr download = g_DownloadManager.AsyncPostWithCallback(url, std::string(request->post_data), "application/x-www-form-urlencoded", http::RequestFlags::ProgressBar | http::RequestFlags::ProgressBarDelayed, [callback, callback_data](http::Request &download) { - std::string buffer; - download.buffer().TakeAll(&buffer); - rc_api_server_response_t response{}; - response.body = buffer.c_str(); - response.body_length = buffer.size(); - response.http_status_code = download.ResultCode(); - callback(&response, callback_data); + complete_server_call(download, callback, callback_data); }, ac->T("Contacting RetroAchievements server...")); } else { std::shared_ptr download = g_DownloadManager.StartDownload(url, Path(), http::RequestFlags::ProgressBar | http::RequestFlags::ProgressBarDelayed, nullptr, ac->T("Contacting RetroAchievements server..."), [callback, callback_data](http::Request &download) { - std::string buffer; - download.buffer().TakeAll(&buffer); - rc_api_server_response_t response{}; - response.body = buffer.c_str(); - response.body_length = buffer.size(); - response.http_status_code = download.ResultCode(); - callback(&response, callback_data); + complete_server_call(download, callback, callback_data); }); } } From 27116cdfa2a1b26c1e4fdf7133051c09c75386fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:10:14 -0600 Subject: [PATCH 07/27] docs: The ARM64 fix for PCH build failures is the arm64 MSBuild PreferredToolArchitecture has no effect; the MSBuild.exe that runs decides the compiler host. The earlier note claimed otherwise, from a build that passed by luck. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/building.md | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/docs/building.md b/docs/building.md index 46443f1a13..fc1122729f 100644 --- a/docs/building.md +++ b/docs/building.md @@ -21,6 +21,14 @@ $msbuild = "$installPath\MSBuild\Current\Bin\MSBuild.exe" (swap `/t:UnitTest` for `/t:PPSSPPWindows` or another project name as needed; drop it entirely to build the whole solution). +On an ARM64 host, use `$installPath\MSBuild\Current\Bin\arm64\MSBuild.exe` instead. The MSBuild in +`Bin\` itself runs emulated and picks the 32-bit x86-hosted compiler (`bin\HostX86\arm64\CL.exe`), +which can't always map the precompiled headers into its address space. Builds then fail +intermittently, every file of a project at line 1, with `C3859: Failed to create virtual memory for +PCH` and `C1076: compiler limit: internal heap limit reached`. That looks like running out of +memory, but lowering `/m` or `CL_MPCount` doesn't help, and `/p:PreferredToolArchitecture=arm64` +has no effect. The build log's `CL.exe` path shows which host compiler ran. + `` is `ARM64` or `x64` - whichever the machine actually is, so look it up rather than picking a default. The output directory follows it (`Windows\\\`), which makes building one and running the other an easy mistake. It is easiest to make on Windows-on-ARM, @@ -75,15 +83,9 @@ UWP has its own solution, `UWP\PPSSPP_UWP.sln`. Build the app with `/t:PPSSPP_UW library such as `/t:CoreUWP`), same MSBuild setup as above: ```powershell -& $msbuild "UWP\PPSSPP_UWP.sln" /t:PPSSPP_UWP /p:Configuration=Debug /p:Platform= /p:PreferredToolArchitecture=arm64 /m +& $msbuild "UWP\PPSSPP_UWP.sln" /t:PPSSPP_UWP /p:Configuration=Debug /p:Platform= /m ``` -On an ARM64 host, pass `/p:PreferredToolArchitecture=arm64`. Without it the UWP -projects use the 32-bit x86-hosted compiler (`bin\HostX86\arm64\CL.exe`), which runs emulated and -can't map the precompiled headers into its address space. Every file then fails at line 1 with -`C3859: Failed to create virtual memory for PCH` and `C1076: compiler limit: internal heap limit -reached`. That looks like running out of memory, but lowering `/m` or `CL_MPCount` doesn't help. - ## Android assets The APK's `assets/` directory is the repo-root `assets/` directory, wired up by From c8b32dda0ad36c134f355bbbdf96388c390f7b64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:12:36 -0600 Subject: [PATCH 08/27] Achievements: Keep a shut-down client alive until its requests finish rc_client keeps raw pointers to itself in the callback data of award, leaderboard submit and ping requests, so destroying it with one in flight (disabling achievements mid-game) had the late callback write to freed memory. Count requests per client, retire a busy client instead of destroying it, and destroy it when its last request completes. Request timeouts in naett are per operation and the plain HTTP client has none for the body, so a retired client is also destroyed after 60 seconds regardless. Its late callbacks are then dropped, leaking their data. Clients are tracked by ID so a new client at a reused address can't get an old one's callbacks. At app exit, pending requests are cancelled without callbacks, so everything is destroyed immediately. Co-Authored-By: Claude Opus 5.5 (1M context) --- Core/RetroAchievements.cpp | 104 +++++++++++++++++++++++++++++++------ Core/RetroAchievements.h | 4 +- UI/NativeApp.cpp | 2 +- 3 files changed, 91 insertions(+), 19 deletions(-) diff --git a/Core/RetroAchievements.cpp b/Core/RetroAchievements.cpp index e82bd0b780..79f4eceaa6 100644 --- a/Core/RetroAchievements.cpp +++ b/Core/RetroAchievements.cpp @@ -16,6 +16,7 @@ // Then just place it next to PPSSPP and enable RAIntegration in PPSSPP achivement settings, then restart it. #include +#include #include #include #include @@ -169,6 +170,20 @@ double g_lastLoginAttemptTime; // rc_client implementation static rc_client_t *g_rcClient; + +// rc_client keeps raw pointers to itself in the callback data of some requests (award achievement, +// submit leaderboard entry, ping), so it can't be destroyed while those are in flight. Shutdown() +// retires a busy client instead, and it's destroyed when its last request completes, or after +// RETIRED_CLIENT_TIMEOUT, from which point its late callbacks are dropped. Keyed by an ID rather +// than the pointer, so a new client allocated at the same address doesn't get an old client's callbacks. +struct TrackedClient { + rc_client_t *client; + int outstandingRequests; + double retireTime; // 0.0 while the client is live. +}; +static std::map g_trackedClients; +static uint32_t g_nextClientId = 1; +constexpr double RETIRED_CLIENT_TIMEOUT = 60.0; static const std::string g_RAImageID = "I_RETROACHIEVEMENTS_LOGO"; constexpr double LOGIN_ATTEMPT_INTERVAL_S = 10.0; @@ -290,7 +305,20 @@ static uint32_t read_memory_callback(uint32_t address, uint8_t *buffer, uint32_t return num_bytes; } -static void complete_server_call(http::Request &download, rc_client_server_callback_t callback, void *callback_data) { +static void destroy_client(rc_client_t *client) { +#ifdef RC_CLIENT_SUPPORTS_RAINTEGRATION + rc_client_unload_raintegration(client); +#endif + rc_client_destroy(client); +} + +static void complete_server_call(uint32_t clientId, http::Request &download, rc_client_server_callback_t callback, void *callback_data) { + if (g_trackedClients.find(clientId) == g_trackedClients.end()) { + // The client timed out after being retired and is gone. callback_data belongs to it, and leaks. + WARN_LOG(Log::Achievements, "Dropping a server response for a destroyed client"); + return; + } + std::string buffer; download.buffer().TakeAll(&buffer); rc_api_server_response_t response{}; @@ -307,6 +335,15 @@ static void complete_server_call(http::Request &download, rc_client_server_callb response.body = buffer.c_str(); response.body_length = buffer.size(); callback(&response, callback_data); + + // Look it up again, the callback may have issued new requests. + auto iter = g_trackedClients.find(clientId); + iter->second.outstandingRequests--; + if (iter->second.retireTime != 0.0 && iter->second.outstandingRequests == 0) { + INFO_LOG(Log::Achievements, "Destroying retired client, its requests are done"); + destroy_client(iter->second.client); + g_trackedClients.erase(iter); + } } // This is the HTTP request dispatcher that is provided to the rc_client. Whenever the client @@ -317,16 +354,27 @@ static void server_call_callback(const rc_api_request_t *request, // If post data is provided, we need to make a POST request, otherwise, a GET request will suffice. auto ac = GetI18NCategory(I18NCat::ACHIEVEMENTS); std::string url = http::RemoveHttpsIfNeeded(request->url); + + uint32_t clientId = 0; + for (auto &iter : g_trackedClients) { + if (iter.second.client == client) { + clientId = iter.first; + iter.second.outstandingRequests++; + break; + } + } + _dbg_assert_(clientId != 0); + if (request->post_data) { std::shared_ptr download = g_DownloadManager.AsyncPostWithCallback(url, std::string(request->post_data), "application/x-www-form-urlencoded", http::RequestFlags::ProgressBar | http::RequestFlags::ProgressBarDelayed, - [callback, callback_data](http::Request &download) { - complete_server_call(download, callback, callback_data); + [clientId, callback, callback_data](http::Request &download) { + complete_server_call(clientId, download, callback, callback_data); }, ac->T("Contacting RetroAchievements server...")); } else { std::shared_ptr download = g_DownloadManager.StartDownload(url, Path(), http::RequestFlags::ProgressBar | http::RequestFlags::ProgressBarDelayed, nullptr, ac->T("Contacting RetroAchievements server..."), - [callback, callback_data](http::Request &download) { - complete_server_call(download, callback, callback_data); + [clientId, callback, callback_data](http::Request &download) { + complete_server_call(clientId, download, callback, callback_data); }); } } @@ -650,13 +698,7 @@ static void load_integration_callback(int result, const char *error_message, rc_ void Initialize() { if (!g_Config.bAchievementsEnable) { INFO_LOG(Log::Achievements, "Achievements are disabled, not initializing."); - if (g_rcClient) { -#ifdef RC_CLIENT_SUPPORTS_RAINTEGRATION - rc_client_unload_raintegration(g_rcClient); -#endif - rc_client_destroy(g_rcClient); - g_rcClient = nullptr; - } + Shutdown(); return; } if (g_rcClient) { @@ -669,6 +711,7 @@ void Initialize() { // Shouldn't happen really. return; } + g_trackedClients[g_nextClientId++] = TrackedClient{ g_rcClient, 0, 0.0 }; // Provide a logging function to simplify debugging rc_client_enable_logging(g_rcClient, RC_CLIENT_LOG_LEVEL_VERBOSE, log_message_callback); @@ -832,16 +875,34 @@ void UpdateSettings() { } } -bool Shutdown() { +bool Shutdown(bool waitForRequests) { g_activeChallenges.clear(); if (g_rcClient) { -#ifdef RC_CLIENT_SUPPORTS_RAINTEGRATION - rc_client_unload_raintegration(g_rcClient); -#endif - rc_client_destroy(g_rcClient); + for (auto iter = g_trackedClients.begin(); iter != g_trackedClients.end(); ++iter) { + if (iter->second.client != g_rcClient) { + continue; + } + if (waitForRequests && iter->second.outstandingRequests > 0) { + INFO_LOG(Log::Achievements, "Retiring client with %d requests in flight", iter->second.outstandingRequests); + // Events from here on would reach code that expects g_rcClient. + rc_client_set_event_handler(g_rcClient, [](const rc_client_event_t *, rc_client_t *) {}); + iter->second.retireTime = time_now_d(); + } else { + destroy_client(g_rcClient); + g_trackedClients.erase(iter); + } + break; + } g_rcClient = nullptr; INFO_LOG(Log::Achievements, "Achievements shut down."); } + if (!waitForRequests) { + // Any requests still pending are about to be cancelled, so their callbacks won't run. + for (auto &iter : g_trackedClients) { + destroy_client(iter.second.client); + } + g_trackedClients.clear(); + } // A destroyed client doesn't call back for pending logins or game loads. g_isLoggingIn = false; g_isIdentifying = false; @@ -866,6 +927,15 @@ void Idle() { rc_client_idle(g_rcClient); double now = time_now_d(); + for (auto iter = g_trackedClients.begin(); iter != g_trackedClients.end(); ) { + if (iter->second.retireTime != 0.0 && now > iter->second.retireTime + RETIRED_CLIENT_TIMEOUT) { + WARN_LOG(Log::Achievements, "Destroying retired client with %d requests still in flight", iter->second.outstandingRequests); + destroy_client(iter->second.client); + iter = g_trackedClients.erase(iter); + } else { + ++iter; + } + } // If failed to log in, occasionally try again while the user is at the menu. // Do not try if if in-game, that could get confusing. diff --git a/Core/RetroAchievements.h b/Core/RetroAchievements.h index aa70fcecba..02e537f87a 100644 --- a/Core/RetroAchievements.h +++ b/Core/RetroAchievements.h @@ -78,7 +78,9 @@ bool LoginProblems(std::string *errorString); bool HasToken(); /// Called when the system is being shut down. If Shutdown() returns false, the shutdown should be aborted if possible. -bool Shutdown(); +/// With waitForRequests, a client with requests in flight is kept alive until they complete (see Idle()). +/// Pass false only when pending requests are about to be cancelled, as at app exit. +bool Shutdown(bool waitForRequests = true); void DownloadImageIfMissing(std::string_view url); diff --git a/UI/NativeApp.cpp b/UI/NativeApp.cpp index 2f8a43ce88..ab2d522120 100644 --- a/UI/NativeApp.cpp +++ b/UI/NativeApp.cpp @@ -1853,7 +1853,7 @@ void NativeShutdown() { g_controlMapper.RemoveListener(&g_globalListener); - Achievements::Shutdown(); + Achievements::Shutdown(false); // CancelAll below drops the pending requests. if (g_Config.bAchievementsEnable) { FILE *iconCacheFile = File::OpenCFile(GetSysDirectory(DIRECTORY_CACHE) / "icon.cache", "wb"); From e628fe3cb2f412eb7709b40656daed76e36c5cdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:12:44 -0600 Subject: [PATCH 09/27] Achievements: Fix leaderboard screen use-after-free on close rc_client frees a request's async handle right after calling back. The error path didn't clear pendingAsyncCall_, so closing the screen after a failed fetch aborted a freed handle. Switching between Top players and Around me also overwrote the handle without aborting the old request, which could then call back into a closed screen, or leak an entry list. Co-Authored-By: Claude Opus 5.5 (1M context) --- UI/RetroAchievementScreens.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/UI/RetroAchievementScreens.cpp b/UI/RetroAchievementScreens.cpp index 1a24823a5e..3198ba8de0 100644 --- a/UI/RetroAchievementScreens.cpp +++ b/UI/RetroAchievementScreens.cpp @@ -258,20 +258,28 @@ RetroAchievementsLeaderboardScreen::RetroAchievementsLeaderboardScreen(const Pat void RetroAchievementsLeaderboardScreen::FetchEntries() { auto callback = [](int result, const char *error_message, rc_client_leaderboard_entry_list_t *list, rc_client_t *client, void *userdata) { + RetroAchievementsLeaderboardScreen *thiz = (RetroAchievementsLeaderboardScreen *)userdata; + // rc_client frees the handle after this returns, so it must not be aborted later. + thiz->pendingAsyncCall_ = nullptr; if (result != RC_OK) { g_OSD.Show(OSDType::MESSAGE_ERROR, error_message, 10.0f); return; } - - RetroAchievementsLeaderboardScreen *thiz = (RetroAchievementsLeaderboardScreen *)userdata; + if (thiz->pendingEntryList_) { + rc_client_destroy_leaderboard_entry_list(thiz->pendingEntryList_); + } thiz->pendingEntryList_ = list; - thiz->pendingAsyncCall_ = nullptr; }; + // An aborted request doesn't call back, so only the latest request can reach the callback. + if (pendingAsyncCall_) { + rc_client_abort_async(Achievements::GetClient(), pendingAsyncCall_); + pendingAsyncCall_ = nullptr; + } + // Store the handle so the destructor can abort it if we're closed before the // response arrives - without this, the callback above would run later against - // a freed `this` (its pendingAsyncCall_ null-check in the destructor was - // otherwise always a no-op since this was never assigned). + // a freed `this`. if (nearMe_) { pendingAsyncCall_ = rc_client_begin_fetch_leaderboard_entries_around_user(Achievements::GetClient(), leaderboardID_, 10, callback, this); } else { From 5c0a80251869db3641330bee46e869ff0e4a2a4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:12:45 -0600 Subject: [PATCH 10/27] Achievements: Clamp a truncated rich presence message rc_client_get_rich_presence_message returns the length it needed rather than what it wrote, so a message of 512 characters or more made the pause screen read past its buffer. Co-Authored-By: Claude Opus 5.5 (1M context) --- Core/RetroAchievements.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Core/RetroAchievements.cpp b/Core/RetroAchievements.cpp index 79f4eceaa6..8414b76238 100644 --- a/Core/RetroAchievements.cpp +++ b/Core/RetroAchievements.cpp @@ -238,7 +238,12 @@ size_t GetRichPresenceMessage(char *buffer, size_t bufSize) { if (!IsLoggedIn() || !rc_client_has_rich_presence(g_rcClient)) { return (size_t)-1; } - return rc_client_get_rich_presence_message(g_rcClient, buffer, bufSize); + size_t length = rc_client_get_rich_presence_message(g_rcClient, buffer, bufSize); + if (length >= bufSize) { + // On truncation, rcheevos returns the length it needed rather than what it wrote. + length = strnlen(buffer, bufSize); + } + return length; } bool WarnUserIfHardcoreModeActive(bool isSaveStateAction, std::string_view message) { From 6b45c4f3e2163ba57cf99b5e0aa8db552e1992a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:12:53 -0600 Subject: [PATCH 11/27] IconCache: Drop icons that fail to decode instead of keeping them forever Any 200 response went into the cache, and one that didn't decode (a captive portal's HTML, or a PNG over the 256x128 limit) was marked bad, saved to icon.cache and never downloaded again. Remove it and mark it failed, so MarkPending allows another download after the retry delay. Co-Authored-By: Claude Opus 5.5 (1M context) --- Common/UI/IconCache.cpp | 10 ++++------ Common/UI/IconCache.h | 1 - 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/Common/UI/IconCache.cpp b/Common/UI/IconCache.cpp index 0262cd9910..d071e79334 100644 --- a/Common/UI/IconCache.cpp +++ b/Common/UI/IconCache.cpp @@ -293,7 +293,7 @@ bool IconCache::InsertIcon(std::string_view key, IconFormat format, std::string } double now = time_now_d(); - cache_.emplace(key, Entry{ std::move(data), format, nullptr, now, now, false }); + cache_.emplace(key, Entry{ std::move(data), format, nullptr, now, now }); return true; } @@ -318,10 +318,6 @@ Draw::Texture *IconCache::BindIconTexture(UIContext *context, std::string_view k return entry.texture; } - if (entry.badData) { - return nullptr; - } - // OK, don't have a texture. Upload it! int width = 0; int height = 0; @@ -336,7 +332,9 @@ Draw::Texture *IconCache::BindIconTexture(UIContext *context, std::string_view k if (result != 1) { ERROR_LOG(Log::G3D, "IconCache: Failed to load png (%d bytes) for key %.*s", (int)data.size(), STR_VIEW(key)); - entry.badData = true; + // Drop it, so it isn't saved to disk, and MarkPending allows downloading it again later. + failed_[std::string(key)] = time_now_d(); + cache_.erase(iter); return nullptr; } dataFormat = Draw::DataFormat::R8G8B8A8_UNORM; diff --git a/Common/UI/IconCache.h b/Common/UI/IconCache.h index 1b22e9ba0f..a0431617d9 100644 --- a/Common/UI/IconCache.h +++ b/Common/UI/IconCache.h @@ -62,7 +62,6 @@ private: Draw::Texture *texture; double insertedTimeStamp; double usedTimeStamp; - bool badData; }; std::map> cache_; From c5ef99ad3f9d983ce4592dcaafcdd6613f294f08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:12:53 -0600 Subject: [PATCH 12/27] IconCache: Check entry lengths before allocating when loading the cache The key was resized before its length was checked, and the data length wasn't checked at all, so a corrupt icon.cache could make a multi-GB allocation and crash every launch. The cache is only saved at a clean exit, so a bad file stayed. Co-Authored-By: Claude Opus 5.5 (1M context) --- Common/UI/IconCache.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Common/UI/IconCache.cpp b/Common/UI/IconCache.cpp index d071e79334..bd098c707a 100644 --- a/Common/UI/IconCache.cpp +++ b/Common/UI/IconCache.cpp @@ -79,13 +79,14 @@ bool IconCache::LoadFromFile(FILE *file) { break; } - std::string key; - key.resize(entryHeader.keyLen, 0); - if (entryHeader.keyLen > 0x1000) { - // Let's say this is invalid, probably a corrupted file. + if (entryHeader.keyLen > 0x1000 || entryHeader.dataLen > MAX_SAVED_CACHE_SIZE) { + // Let's say this is invalid, probably a corrupted file. Check before allocating. break; } + std::string key; + key.resize(entryHeader.keyLen, 0); + if (fread(&key[0], 1, entryHeader.keyLen, file) != entryHeader.keyLen) { break; } From f87179d0c1f89c60fa493ce9120c9c2fa8218b0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:14:58 -0600 Subject: [PATCH 13/27] HTTP: Keep abandoned naett requests' sinks and POST bodies alive Join() deliberately leaks the naett objects of a request still running at shutdown and parks its sink, so a late chunk lands somewhere valid. But HTTPSShutdown, which runs right after CancelAll, deleted those sinks, and the POST body naett reads in place was owned by the request being destroyed. On Android, where NativeShutdown isn't process exit, a late WinHTTP/JNI callback could then write to or read from freed memory. Only free the list when it's empty (that still fixes the leak the delete was added for), and keep the POST body in the sink. Co-Authored-By: Claude Opus 5.5 (1M context) --- Common/Net/HTTPNaettRequest.cpp | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/Common/Net/HTTPNaettRequest.cpp b/Common/Net/HTTPNaettRequest.cpp index 8161e6d92a..b2991274f2 100644 --- a/Common/Net/HTTPNaettRequest.cpp +++ b/Common/Net/HTTPNaettRequest.cpp @@ -31,6 +31,8 @@ HTTPSRequest::~HTTPSRequest() { // still exists instead of in a destroyed object. struct NaettBodySink { Buffer buffer; + // naett reads the POST body in place while uploading, so it lives here too. + std::string postData; // Written by us, read by the transfer thread. std::atomic cancelled{false}; }; @@ -43,8 +45,11 @@ struct NaettBodySink { static std::vector> *g_abandonedSinks = nullptr; void HTTPSShutdown() { - delete g_abandonedSinks; - g_abandonedSinks = nullptr; + // Free the list only if it's empty. An abandoned sink can still be written to. + if (g_abandonedSinks && g_abandonedSinks->empty()) { + delete g_abandonedSinks; + g_abandonedSinks = nullptr; + } } int HTTPSRequest::WriteBodyThunk(const void *source, int bytes, void *userData) { @@ -73,6 +78,13 @@ void HTTPSRequest::Start() { _dbg_assert_(!req_); _dbg_assert_(!res_); + // Our own writer, so that Cancel() can actually stop a transfer rather than just relabelling + // it once it finishes. + sink_ = std::make_unique(); + // In case someone managed to cancel us between construction and here. + sink_->cancelled = cancelled_; + sink_->postData = postData_; + std::vector options; options.push_back(naettMethod(method_ == RequestMethod::GET ? "GET" : "POST")); options.push_back(naettHeader("Accept", acceptMime_)); @@ -83,18 +95,13 @@ void HTTPSRequest::Start() { if (method_ == RequestMethod::POST) { if (!postData_.empty()) { // Note: Naett does not take ownership over the body. - options.push_back(naettBody(postData_.data(), (int)postData_.size())); + options.push_back(naettBody(sink_->postData.data(), (int)sink_->postData.size())); } } else { _dbg_assert_(postData_.empty()); } // 30 s timeout - not sure what's reasonable? options.push_back(naettTimeout(30 * 1000)); // milliseconds - // Our own writer, so that Cancel() can actually stop a transfer rather than just relabelling - // it once it finishes. - sink_ = std::make_unique(); - // In case someone managed to cancel us between construction and here. - sink_->cancelled = cancelled_; options.push_back(naettBodyWriter(&HTTPSRequest::WriteBodyThunk, sink_.get())); const naettOption **opts = (const naettOption **)options.data(); From 8e92fef5279202f590c86ef856d1ca70965be1d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:14:58 -0600 Subject: [PATCH 14/27] HTTP: Publish request completion with release/acquire ordering HTTPRequest::completed_ was a plain bool set on the download thread, and naett's res->complete a plain int set on its transfer thread. On ARM, the thread polling Done() could see the flag before the result code or body written ahead of it, and report a stale code or a partial body. Co-Authored-By: Claude Opus 5.5 (1M context) --- Common/Net/HTTPClient.h | 5 ++++- ext/naett-lib/src/naett_android.c | 4 ++-- ext/naett-lib/src/naett_core.c | 2 +- ext/naett-lib/src/naett_internal.h | 22 ++++++++++++++++++++++ ext/naett-lib/src/naett_linux.c | 8 ++++---- ext/naett-lib/src/naett_osx.c | 4 ++-- ext/naett-lib/src/naett_win.c | 18 +++++++++--------- 7 files changed, 44 insertions(+), 19 deletions(-) diff --git a/Common/Net/HTTPClient.h b/Common/Net/HTTPClient.h index b678a2c7e1..65f61e0720 100644 --- a/Common/Net/HTTPClient.h +++ b/Common/Net/HTTPClient.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -124,7 +125,9 @@ private: std::string postData_; std::thread thread_; std::string postMime_; - bool completed_ = false; + // Set last on the download thread, after the result code and buffer. Atomic, so that once + // Done() sees it, those are visible too. + std::atomic completed_{false}; bool failed_ = false; net::ResolveFunc customResolve_; }; diff --git a/ext/naett-lib/src/naett_android.c b/ext/naett-lib/src/naett_android.c index 3d433200db..d2125dc4a9 100644 --- a/ext/naett-lib/src/naett_android.c +++ b/ext/naett-lib/src/naett_android.c @@ -314,7 +314,7 @@ static void* processRequest(void* data) { res->code = statusCode; finally: - res->complete = 1; + naettSetComplete(res); (*env)->PopLocalFrame(env, NULL); JavaVM* vm = getVM(); (*env)->ExceptionClear(env); @@ -332,7 +332,7 @@ static void startWorkerThread(InternalResponse* res) { LOGE("Failed to start the request worker thread"); res->workerThread = 0; res->code = naettGenericError; - res->complete = 1; + naettSetComplete(res); } else { pthread_setname_np(res->workerThread, "naett worker thread"); } diff --git a/ext/naett-lib/src/naett_core.c b/ext/naett-lib/src/naett_core.c index 7140beaebd..27788e2149 100644 --- a/ext/naett-lib/src/naett_core.c +++ b/ext/naett-lib/src/naett_core.c @@ -390,7 +390,7 @@ naettReq* naettGetRequest(naettRes* response) { int naettComplete(const naettRes* response) { assert(response != NULL); InternalResponse* res = (InternalResponse*)response; - return res->complete; + return naettLoadComplete(res); } int naettGetStatus(const naettRes* response) { diff --git a/ext/naett-lib/src/naett_internal.h b/ext/naett-lib/src/naett_internal.h index 8631b0a83d..17c7147e49 100644 --- a/ext/naett-lib/src/naett_internal.h +++ b/ext/naett-lib/src/naett_internal.h @@ -111,6 +111,28 @@ typedef struct { #endif } InternalResponse; +// PPSSPP: complete is set on a transfer thread and polled on the caller's. Publish it with release +// semantics and read it with acquire, so that the status code and body written before it are +// visible once it reads as set. With plain accesses, ARM can reorder them. +static inline void naettSetComplete(InternalResponse* res) { +#ifdef _MSC_VER + MemoryBarrier(); + *(volatile int*)&res->complete = 1; +#else + __atomic_store_n(&res->complete, 1, __ATOMIC_RELEASE); +#endif +} + +static inline int naettLoadComplete(InternalResponse* res) { +#ifdef _MSC_VER + int complete = *(volatile int*)&res->complete; + MemoryBarrier(); + return complete; +#else + return __atomic_load_n(&res->complete, __ATOMIC_ACQUIRE); +#endif +} + void naettPlatformInit(naettInitData initData); int naettPlatformInitRequest(InternalRequest* req); void naettPlatformMakeRequest(InternalResponse* res); diff --git a/ext/naett-lib/src/naett_linux.c b/ext/naett-lib/src/naett_linux.c index ad505137e6..112a8bc76f 100644 --- a/ext/naett-lib/src/naett_linux.c +++ b/ext/naett-lib/src/naett_linux.c @@ -64,7 +64,7 @@ static void* curlWorker(void* data) { long responseCode = 0; curl_easy_getinfo(handle, CURLINFO_RESPONSE_CODE, &responseCode); res->code = (int)responseCode; - res->complete = 1; + naettSetComplete(res); // PPSSPP: curl wants the easy handle out of the multi before it's cleaned up. curl_multi_remove_handle(mc, handle); curl_easy_cleanup(handle); @@ -237,14 +237,14 @@ void naettPlatformMakeRequest(InternalResponse* res) { // No libcurl, or the worker died. Complete the request as failed rather than // leaving the caller waiting forever on a request nobody is going to run. res->code = naettGenericError; - res->complete = 1; + naettSetComplete(res); return; } CURL* c = curl_easy_init(); if (c == NULL) { res->code = naettGenericError; - res->complete = 1; + naettSetComplete(res); return; } curl_easy_setopt(c, CURLOPT_URL, req->url); @@ -303,7 +303,7 @@ void naettPlatformMakeRequest(InternalResponse* res) { res->headerList = NULL; curl_easy_cleanup(c); res->code = naettGenericError; - res->complete = 1; + naettSetComplete(res); } } diff --git a/ext/naett-lib/src/naett_osx.c b/ext/naett-lib/src/naett_osx.c index a583d09057..9e936e7630 100644 --- a/ext/naett-lib/src/naett_osx.c +++ b/ext/naett-lib/src/naett_osx.c @@ -161,7 +161,7 @@ void didReceiveData(id self, SEL _sel, id session, id dataTask, id data) { int written = res->request->options.bodyWriter(bytes, (int)length, res->request->options.bodyWriterData); if (written != (int)length) { res->code = naettReadError; - res->complete = 1; + naettSetComplete(res); objc_msgSend_void(dataTask, sel("cancel")); release(p); return; @@ -183,7 +183,7 @@ static void didComplete(id self, SEL _sel, id session, id dataTask, id error) { if (error != nil) { res->code = naettConnectionError; } - res->complete = 1; + naettSetComplete(res); } } diff --git a/ext/naett-lib/src/naett_win.c b/ext/naett-lib/src/naett_win.c index 84c38340b6..d9bfcb9ae2 100644 --- a/ext/naett-lib/src/naett_win.c +++ b/ext/naett-lib/src/naett_win.c @@ -161,7 +161,7 @@ callback(HINTERNET request, DWORD_PTR context, DWORD status, LPVOID statusInform if (!WinHttpQueryDataAvailable(request, NULL)) { res->code = naettProtocolError; - res->complete = 1; + naettSetComplete(res); } } break; @@ -169,14 +169,14 @@ callback(HINTERNET request, DWORD_PTR context, DWORD status, LPVOID statusInform DWORD* available = (DWORD*)statusInformation; res->bytesLeft = *available; if (res->bytesLeft == 0) { - res->complete = 1; + naettSetComplete(res); break; } size_t bytesToRead = min(res->bytesLeft, sizeof(res->buffer)); if (!WinHttpReadData(request, res->buffer, (DWORD)bytesToRead, NULL)) { res->code = naettReadError; - res->complete = 1; + naettSetComplete(res); } } break; @@ -193,7 +193,7 @@ callback(HINTERNET request, DWORD_PTR context, DWORD status, LPVOID statusInform // which both kept the transfer running and left WinHTTP writing into a response // the caller was by then free to close. res->code = naettReadError; - res->complete = 1; + naettSetComplete(res); break; } res->totalBytesRead += (int)bytesRead; @@ -204,12 +204,12 @@ callback(HINTERNET request, DWORD_PTR context, DWORD status, LPVOID statusInform size_t bytesToRead = min(res->bytesLeft, sizeof(res->buffer)); if (!WinHttpReadData(request, res->buffer, (DWORD)bytesToRead, NULL)) { res->code = naettReadError; - res->complete = 1; + naettSetComplete(res); } } else { if (!WinHttpQueryDataAvailable(request, NULL)) { res->code = naettProtocolError; - res->complete = 1; + naettSetComplete(res); } } } break; @@ -223,7 +223,7 @@ callback(HINTERNET request, DWORD_PTR context, DWORD status, LPVOID statusInform } else { if (!WinHttpReceiveResponse(request, NULL)) { res->code = naettReadError; - res->complete = 1; + naettSetComplete(res); } } } break; @@ -247,7 +247,7 @@ callback(HINTERNET request, DWORD_PTR context, DWORD status, LPVOID statusInform res->code = naettGenericError; } - res->complete = 1; + naettSetComplete(res); } break; } } @@ -350,7 +350,7 @@ void naettPlatformMakeRequest(InternalResponse* res) { if (!WinHttpSendRequest(req->request, extraHeaders, -1, NULL, 0, 0, (DWORD_PTR)res)) { res->code = naettConnectionError; - res->complete = 1; + naettSetComplete(res); } } From efc3f5d5566526313075c27852e6cb955050abf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:25:22 -0600 Subject: [PATCH 15/27] Achievements: Stop retrying a login token the server rejected On invalid credentials the token was kept, and Idle() retried it every 10 seconds for the rest of the session, silently after the first attempt. Clear it, as was already done for an expired token, so the settings show the login form. On access denied, stop retrying. Show a rejection even when it comes from a background retry, since it won't repeat, and have LoginProblems return the server's message. Co-Authored-By: Claude Opus 5.5 (1M context) --- Core/RetroAchievements.cpp | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/Core/RetroAchievements.cpp b/Core/RetroAchievements.cpp index 8414b76238..696c925048 100644 --- a/Core/RetroAchievements.cpp +++ b/Core/RetroAchievements.cpp @@ -165,6 +165,7 @@ bool g_isIdentifying = false; bool g_isLoggingIn = false; bool g_hasRichPresence = false; int g_loginResult; +std::string g_loginError; // The server's message for g_loginResult. double g_lastLoginAttemptTime; @@ -579,26 +580,30 @@ static void login_token_callback(int result, const char *error_message, rc_clien case RC_INVALID_JSON: default: { - ERROR_LOG(Log::Achievements, "Callback: Failure logging in via token: %d, %s", result, error_message); - if (isInitialAttempt) { + ERROR_LOG(Log::Achievements, "Callback: Failure logging in via token: %d, %s", result, error_message ? error_message : ""); + // The server rejecting the token isn't retried, so say so even if this was a background retry. + const bool rejected = result == RC_INVALID_CREDENTIALS || result == RC_EXPIRED_TOKEN || result == RC_ACCESS_DENIED; + if (isInitialAttempt || rejected) { auto ac = GetI18NCategory(I18NCat::ACHIEVEMENTS); char message[512]; - snprintf(message, sizeof(message), "%d: %s", result, error_message); + snprintf(message, sizeof(message), "%d: %s", result, error_message ? error_message : ""); g_OSD.Show(OSDType::MESSAGE_WARNING, ac->T("Failed logging in to RetroAchievements"), message, g_RAImageID); } // Take some action. switch (result) { case RC_INVALID_CREDENTIALS: - g_loginResult = RC_OK; // why? - break; case RC_EXPIRED_TOKEN: - WARN_LOG(Log::Achievements, "Clearing token since it was expired"); + // The token is no good, the user has to log in with their password again. Clearing it + // stops Idle() retrying it, and with no login problem recorded, the settings show the login form. + WARN_LOG(Log::Achievements, "Clearing token since the server rejected it"); NativeClearSecret(RA_TOKEN_SECRET_NAME); - g_loginResult = RC_OK; // why? + g_loginResult = RC_OK; break; default: + // Includes RC_ACCESS_DENIED, which Idle() doesn't retry. The settings show the message, and Log out. g_loginResult = result; + g_loginError = error_message ? error_message : ""; break; } OnAchievementsLoginStateChange(); @@ -607,6 +612,7 @@ static void login_token_callback(int result, const char *error_message, rc_clien } } g_loginResult = result; + g_loginError = error_message ? error_message : ""; g_isLoggingIn = false; } @@ -786,8 +792,11 @@ bool HasToken() { } bool LoginProblems(std::string *errorString) { - // TODO: Set error string. - return g_loginResult != RC_OK; + if (g_loginResult == RC_OK) { + return false; + } + *errorString = g_loginError; + return true; } static void TryLoginByToken(bool isInitialAttempt) { @@ -949,7 +958,7 @@ void Idle() { if (g_rcClient && IsLoggedIn()) { return; // All good. } - if (g_Config.sAchievementsUserName.empty() || g_isLoggingIn || !HasToken()) { + if (g_Config.sAchievementsUserName.empty() || g_isLoggingIn || !HasToken() || g_loginResult == RC_ACCESS_DENIED) { // Didn't try to login yet or is in the process of logging in. Also OK. return; } From 19082ce1afc2e996a256e6927280253e0a480c0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:25:37 -0600 Subject: [PATCH 16/27] Achievements: Don't start a password login while a token login is pending rc_client answers a second login with RC_INVALID_STATE, synchronously, and login_password_callback then cleared g_isLoggingIn while the token login was still running. IsBlockingExecution stopped blocking, and a game started then ran without achievements. Co-Authored-By: Claude Opus 5.5 (1M context) --- Core/RetroAchievements.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Core/RetroAchievements.cpp b/Core/RetroAchievements.cpp index 696c925048..99adb177ae 100644 --- a/Core/RetroAchievements.cpp +++ b/Core/RetroAchievements.cpp @@ -853,7 +853,8 @@ static void login_password_callback(int result, const char *error_message, rc_cl bool LoginAsync(const char *username, const char *password) { auto di = GetI18NCategory(I18NCat::DIALOG); - if (IsLoggedIn() || std::strlen(username) == 0 || std::strlen(password) == 0) + // While a token login is pending, rc_client refuses another login, and the callback for that would clear g_isLoggingIn early. + if (IsLoggedIn() || g_isLoggingIn || std::strlen(username) == 0 || std::strlen(password) == 0) return false; g_OSD.SetProgressBar("cheevos_async_login", di->T("Logging in..."), 0, 0, 0, 0.0f); From 8aac115e91228af10c8a6838cc3a93791895206d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:25:37 -0600 Subject: [PATCH 17/27] Achievements: Use the sized progress (de)serializers for savestates The deprecated unsized rc_client_deserialize_progress parses with no bound, so a truncated or corrupt state could make it read past the buffer. data_size is known, so pass it. Co-Authored-By: Claude Opus 5.5 (1M context) --- Core/RetroAchievements.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Core/RetroAchievements.cpp b/Core/RetroAchievements.cpp index 99adb177ae..c0f38c17f7 100644 --- a/Core/RetroAchievements.cpp +++ b/Core/RetroAchievements.cpp @@ -1015,7 +1015,7 @@ void DoState(PointerWrap &p) { case PointerWrap::MODE_WRITE: case PointerWrap::MODE_VERIFY: { - int retval = rc_client_serialize_progress(g_rcClient, buffer); + int retval = rc_client_serialize_progress_sized(g_rcClient, buffer, data_size); if (retval != RC_OK) { ERROR_LOG(Log::Achievements, "Error %d serializing achievement data. Ignoring.", retval); } @@ -1030,7 +1030,7 @@ void DoState(PointerWrap &p) { switch (p.mode) { case PointerWrap::MODE_READ: { - int retval = rc_client_deserialize_progress(g_rcClient, buffer); + int retval = rc_client_deserialize_progress_sized(g_rcClient, buffer, data_size); if (retval != RC_OK) { // TODO: What should we really do here? ERROR_LOG(Log::Achievements, "Error %d deserializing achievement data. Ignoring.", retval); From 0b24dc7f1afabd9a44e8b9a0453eac2e09e279b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:25:55 -0600 Subject: [PATCH 18/27] naett: Complete a Windows request when WinHttpWriteData fails The return value was ignored, and after a synchronous failure no further callback arrives, so the request never completed. During achievement game identification, that blocked boot indefinitely. Co-Authored-By: Claude Opus 5.5 (1M context) --- ext/naett-lib/src/naett_win.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ext/naett-lib/src/naett_win.c b/ext/naett-lib/src/naett_win.c index d9bfcb9ae2..253f83da4e 100644 --- a/ext/naett-lib/src/naett_win.c +++ b/ext/naett-lib/src/naett_win.c @@ -219,7 +219,12 @@ callback(HINTERNET request, DWORD_PTR context, DWORD status, LPVOID statusInform int bytesRead = res->request->options.bodyReader( res->buffer, sizeof(res->buffer), res->request->options.bodyReaderData); if (bytesRead) { - WinHttpWriteData(request, res->buffer, bytesRead, NULL); + // PPSSPP: upstream ignored a failure here, and with no further callback coming, + // the request never completed. + if (!WinHttpWriteData(request, res->buffer, bytesRead, NULL)) { + res->code = naettWriteError; + naettSetComplete(res); + } } else { if (!WinHttpReceiveResponse(request, NULL)) { res->code = naettReadError; From 39797c4a440c36733e0721fb6c3785b1fed4392b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:26:52 -0600 Subject: [PATCH 19/27] Achievements: Reload RAIntegration when achievements are re-enabled With RAIntegration, Initialize leaves login to the DLL load, but WinMain only calls InitializeRAIntegration once and Shutdown unloads the DLL, so turning achievements off and on left you logged out until restart. Remember the window and load the DLL again on its thread. Co-Authored-By: Claude Opus 5.5 (1M context) --- Core/RetroAchievements.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Core/RetroAchievements.cpp b/Core/RetroAchievements.cpp index c0f38c17f7..5c1c99787c 100644 --- a/Core/RetroAchievements.cpp +++ b/Core/RetroAchievements.cpp @@ -185,6 +185,11 @@ struct TrackedClient { static std::map g_trackedClients; static uint32_t g_nextClientId = 1; constexpr double RETIRED_CLIENT_TIMEOUT = 60.0; + +#ifdef RC_CLIENT_SUPPORTS_RAINTEGRATION +// The main window, once InitializeRAIntegration has been called, so Initialize can load the DLL again. +static void *g_raIntegrationWindow; +#endif static const std::string g_RAImageID = "I_RETROACHIEVEMENTS_LOGO"; constexpr double LOGIN_ATTEMPT_INTERVAL_S = 10.0; @@ -744,6 +749,12 @@ void Initialize() { #ifdef RC_CLIENT_SUPPORTS_RAINTEGRATION if (!g_Config.bAchievementsEnableRAIntegration) { TryLoginByToken(true); + } else if (g_raIntegrationWindow) { + // Re-enabled after startup. WinMain only calls InitializeRAIntegration once, and Shutdown + // unloaded the DLL, so load it again (it logs in when loaded). It builds menus, so do it on the window's thread. + System_RunCallbackInWndProc([](void *hWnd, void *) { + InitializeRAIntegration(hWnd); + }, nullptr); } #else TryLoginByToken(true); @@ -752,6 +763,7 @@ void Initialize() { void InitializeRAIntegration(void *windowHandle) { #ifdef RC_CLIENT_SUPPORTS_RAINTEGRATION + g_raIntegrationWindow = windowHandle; if (g_rcClient && g_Config.bAchievementsEnableRAIntegration) { wchar_t szFilePath[MAX_PATH]; GetModuleFileNameW(NULL, szFilePath, MAX_PATH); From 77cc6a80713baccb5453d21f4f3ba06e90e60f82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:27:02 -0600 Subject: [PATCH 20/27] Achievements: Don't pass a null error message to %s error_message is null on success, which is when these two lines log. Co-Authored-By: Claude Opus 5.5 (1M context) --- Core/RetroAchievements.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Core/RetroAchievements.cpp b/Core/RetroAchievements.cpp index 5c1c99787c..33427fd993 100644 --- a/Core/RetroAchievements.cpp +++ b/Core/RetroAchievements.cpp @@ -1135,7 +1135,7 @@ void ShowNotLoggedInMessage() { void identify_and_load_callback(int result, const char *error_message, rc_client_t *client, void *userdata) { auto ac = GetI18NCategory(I18NCat::ACHIEVEMENTS); - NOTICE_LOG(Log::Achievements, "Load callback: %d (%s)", result, error_message); + NOTICE_LOG(Log::Achievements, "Load callback: %d (%s)", result, error_message ? error_message : ""); switch (result) { case RC_OK: @@ -1273,7 +1273,7 @@ void UnloadGame() { void change_media_callback(int result, const char *error_message, rc_client_t *client, void *userdata) { auto ac = GetI18NCategory(I18NCat::ACHIEVEMENTS); - NOTICE_LOG(Log::Achievements, "Change media callback: %d (%s)", result, error_message); + NOTICE_LOG(Log::Achievements, "Change media callback: %d (%s)", result, error_message ? error_message : ""); g_isIdentifying = false; switch (result) { From ab8fc0d1bb258e979070f7274443c62a6dda41e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:27:08 -0600 Subject: [PATCH 21/27] Achievements: Remove unused ResetRuntime Co-Authored-By: Claude Opus 5.5 (1M context) --- Core/RetroAchievements.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/Core/RetroAchievements.cpp b/Core/RetroAchievements.cpp index 33427fd993..20545c442c 100644 --- a/Core/RetroAchievements.cpp +++ b/Core/RetroAchievements.cpp @@ -936,14 +936,6 @@ bool Shutdown(bool waitForRequests) { return true; } -void ResetRuntime() { - if (!g_rcClient) - return; - INFO_LOG(Log::Achievements, "Resetting rcheevos state..."); - rc_client_reset(g_rcClient); - g_activeChallenges.clear(); -} - void FrameUpdate() { if (!g_rcClient) return; From 2d6045fa4bd686667de9780ac52660011090b654 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:28:26 -0600 Subject: [PATCH 22/27] IconCache: Bound failed_, back off failed uploads, fix the disk layout - Prune expired failed_ entries, which otherwise grew for the whole session. - When CreateTexture fails, wait 5 seconds before decoding and uploading again, instead of retrying every frame. - Bind a newly created texture. Callers draw right after BindIconTexture, and got whatever was bound before for the first frame. - Make DiskCacheEntry's padding explicit. 32-bit x86 Linux aligns double to 4, so its icon.cache didn't match other builds. The layout is unchanged elsewhere. Co-Authored-By: Claude Opus 5.5 (1M context) --- Common/UI/IconCache.cpp | 26 +++++++++++++++++++++++--- Common/UI/IconCache.h | 1 + 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/Common/UI/IconCache.cpp b/Common/UI/IconCache.cpp index bd098c707a..1abcea8030 100644 --- a/Common/UI/IconCache.cpp +++ b/Common/UI/IconCache.cpp @@ -16,6 +16,8 @@ // Seconds before MarkPending accepts a key whose download failed. constexpr double FAILED_RETRY_DELAY = 30.0; +// Seconds before BindIconTexture tries again to create a texture that failed. +constexpr double UPLOAD_RETRY_DELAY = 5.0; constexpr uint32_t ICON_CACHE_MAGIC = MK_FOURCC("pICN"); @@ -31,8 +33,10 @@ struct DiskCacheEntry { uint32_t keyLen; uint32_t dataLen; IconFormat format; + uint32_t padding; // Explicit, so that 32-bit x86 Linux (which aligns double to 4) has the same layout. double insertedTimestamp; }; +static_assert(sizeof(DiskCacheEntry) == 24, "DiskCacheEntry is written to disk as is"); void IconCache::SaveToFile(FILE *file) { std::unique_lock lock(lock_); @@ -151,6 +155,13 @@ void IconCache::FrameUpdate() { } } } + for (auto iter = failed_.begin(); iter != failed_.end(); ) { + if (now > iter->second + FAILED_RETRY_DELAY) { + iter = failed_.erase(iter); // MarkPending would accept it anyway. + } else { + ++iter; + } + } lastUpdate_ = now; } @@ -319,6 +330,10 @@ Draw::Texture *IconCache::BindIconTexture(UIContext *context, std::string_view k return entry.texture; } + if (entry.uploadFailedTime != 0.0 && time_now_d() < entry.uploadFailedTime + UPLOAD_RETRY_DELAY) { + return nullptr; + } + // OK, don't have a texture. Upload it! int width = 0; int height = 0; @@ -358,11 +373,16 @@ Draw::Texture *IconCache::BindIconTexture(UIContext *context, std::string_view k iconDesc.type = Draw::TextureType::LINEAR2D; Draw::Texture *texture = context->GetDrawContext()->CreateTexture(iconDesc); + free(buffer); + if (!texture) { + ERROR_LOG(Log::G3D, "IconCache: Failed to create a %dx%d texture for key %.*s", width, height, STR_VIEW(key)); + entry.uploadFailedTime = time_now_d(); + return nullptr; + } entry.texture = texture; entry.usedTimeStamp = time_now_d(); - - free(buffer); - + // The caller draws with it right away, same as with an existing texture. + context->GetDrawContext()->BindTexture(0, texture); return texture; } diff --git a/Common/UI/IconCache.h b/Common/UI/IconCache.h index a0431617d9..3c62379764 100644 --- a/Common/UI/IconCache.h +++ b/Common/UI/IconCache.h @@ -62,6 +62,7 @@ private: Draw::Texture *texture; double insertedTimeStamp; double usedTimeStamp; + double uploadFailedTime; // When CreateTexture last failed, or 0. }; std::map> cache_; From 36bd2e924951872e08481f0ab0ad8d52266e7ec6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:29:17 -0600 Subject: [PATCH 23/27] IconCache: Know an icon's size before its texture exists GetDimensions only answered once a texture had been created, so the first MeasureNotice for a new icon (and after the texture was released for being unused) measured without it, and the icon was then drawn over the text. Read the size from the PNG header when the icon is inserted or loaded. Also remove a width adjustment in MeasureNotice that was overwritten before use. Co-Authored-By: Claude Opus 5.5 (1M context) --- Common/UI/IconCache.cpp | 30 +++++++++++++++++++++++------- Common/UI/IconCache.h | 2 ++ Common/UI/Notice.cpp | 1 - 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/Common/UI/IconCache.cpp b/Common/UI/IconCache.cpp index 1abcea8030..467e3504cf 100644 --- a/Common/UI/IconCache.cpp +++ b/Common/UI/IconCache.cpp @@ -1,4 +1,5 @@ #include +#include #include "Common/UI/IconCache.h" #include "Common/UI/Context.h" @@ -38,6 +39,20 @@ struct DiskCacheEntry { }; static_assert(sizeof(DiskCacheEntry) == 24, "DiskCacheEntry is written to disk as is"); +// Reads the size from the header, so layout can use it before there's a texture. 0x0 if unknown. +static void PeekIconSize(const std::string &data, IconFormat format, int *width, int *height) { + *width = 0; + *height = 0; + if (format == IconFormat::PNG && data.size() >= sizeof(PNGHeaderPeek)) { + PNGHeaderPeek peek; + memcpy(&peek, data.data(), sizeof(peek)); + if (peek.IsValidPNGHeader()) { + *width = peek.Width(); + *height = peek.Height(); + } + } +} + void IconCache::SaveToFile(FILE *file) { std::unique_lock lock(lock_); @@ -115,6 +130,7 @@ bool IconCache::LoadFromFile(FILE *file) { entry.format = entryHeader.format; entry.insertedTimeStamp = entryHeader.insertedTimestamp; entry.usedTimeStamp = now; + PeekIconSize(entry.data, entry.format, &entry.width, &entry.height); cache_.emplace(key, entry); } @@ -224,14 +240,12 @@ bool IconCache::GetDimensions(std::string_view key, int *width, int *height) { } const auto &entry = iter->second; - if (entry.texture) { - // TODO: Store the width/height in the cache. - *width = entry.texture->Width(); - *height = entry.texture->Height(); - return true; - } else { + if (entry.width <= 0 || entry.height <= 0) { return false; } + *width = entry.width; + *height = entry.height; + return true; } bool IconCache::Contains(std::string_view key) { @@ -305,7 +319,9 @@ bool IconCache::InsertIcon(std::string_view key, IconFormat format, std::string } double now = time_now_d(); - cache_.emplace(key, Entry{ std::move(data), format, nullptr, now, now }); + Entry entry{ std::move(data), format, nullptr, now, now }; + PeekIconSize(entry.data, entry.format, &entry.width, &entry.height); + cache_.emplace(key, std::move(entry)); return true; } diff --git a/Common/UI/IconCache.h b/Common/UI/IconCache.h index 3c62379764..45eee95f54 100644 --- a/Common/UI/IconCache.h +++ b/Common/UI/IconCache.h @@ -63,6 +63,8 @@ private: double insertedTimeStamp; double usedTimeStamp; double uploadFailedTime; // When CreateTexture last failed, or 0. + int width; // From the image header, 0 if unknown. + int height; }; std::map> cache_; diff --git a/Common/UI/Notice.cpp b/Common/UI/Notice.cpp index 60d304b619..a5ee6c88eb 100644 --- a/Common/UI/Notice.cpp +++ b/Common/UI/Notice.cpp @@ -54,7 +54,6 @@ void MeasureNotice(const UIContext &dc, NoticeLevel level, std::string_view text // Normal entry but with a cached icon. int iconWidth, iconHeight; if (g_iconCache.GetDimensions(iconName, &iconWidth, &iconHeight)) { - *width += 5.0f + iconWidth; iconW = iconWidth; iconH = iconHeight; } From 9cb8f54a4e40e7fc38408851c0e24cdaa88ccb33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:29:43 -0600 Subject: [PATCH 24/27] IconCache: Expire saved icons after 24 hours Icons were kept in icon.cache indefinitely, so one that changes on the server (a RetroAchievements user avatar, keyed by username) never refreshed. The insertion timestamp was process-relative, so it couldn't be compared across sessions. Store wall-clock time, and skip entries older than a day when loading. Bumps the cache version. Co-Authored-By: Claude Opus 5.5 (1M context) --- Common/UI/IconCache.cpp | 12 ++++++++---- Common/UI/IconCache.h | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/Common/UI/IconCache.cpp b/Common/UI/IconCache.cpp index 467e3504cf..83a339423e 100644 --- a/Common/UI/IconCache.cpp +++ b/Common/UI/IconCache.cpp @@ -9,7 +9,8 @@ #include "Common/GPU/thin3d.h" #include "Common/File/FileUtil.h" -#define ICON_CACHE_VERSION 1 +// 2: insertedTimestamp became wall-clock time. +#define ICON_CACHE_VERSION 2 #define MK_FOURCC(str) (str[0] | ((uint8_t)str[1] << 8) | ((uint8_t)str[2] << 16) | ((uint8_t)str[3] << 24)) #define MAX_RUNTIME_CACHE_SIZE (1024 * 1024 * 4) @@ -17,6 +18,8 @@ // Seconds before MarkPending accepts a key whose download failed. constexpr double FAILED_RETRY_DELAY = 30.0; +// Seconds a saved icon is kept across sessions, so that ones that change (user avatars) get refreshed. +constexpr double MAX_SAVED_ICON_AGE = 24 * 60 * 60.0; // Seconds before BindIconTexture tries again to create a texture that failed. constexpr double UPLOAD_RETRY_DELAY = 5.0; @@ -91,6 +94,7 @@ bool IconCache::LoadFromFile(FILE *file) { } double now = time_now_d(); + double nowUnix = time_now_unix_utc(); for (uint32_t i = 0; i < header.entryCount; i++) { DiskCacheEntry entryHeader{}; @@ -110,8 +114,8 @@ bool IconCache::LoadFromFile(FILE *file) { break; } - // Check if we already have the entry somehow. - if (cache_.find(key) != cache_.end()) { + // Skip it if we already have the entry somehow, or it's too old. + if (cache_.find(key) != cache_.end() || nowUnix - entryHeader.insertedTimestamp > MAX_SAVED_ICON_AGE) { // Seek past the data and go to the next entry. File::Fseek(file, entryHeader.dataLen, SEEK_CUR); continue; @@ -319,7 +323,7 @@ bool IconCache::InsertIcon(std::string_view key, IconFormat format, std::string } double now = time_now_d(); - Entry entry{ std::move(data), format, nullptr, now, now }; + Entry entry{ std::move(data), format, nullptr, time_now_unix_utc(), now }; PeekIconSize(entry.data, entry.format, &entry.width, &entry.height); cache_.emplace(key, std::move(entry)); return true; diff --git a/Common/UI/IconCache.h b/Common/UI/IconCache.h index 45eee95f54..c3f690b689 100644 --- a/Common/UI/IconCache.h +++ b/Common/UI/IconCache.h @@ -60,7 +60,7 @@ private: std::string data; IconFormat format; Draw::Texture *texture; - double insertedTimeStamp; + double insertedTimeStamp; // Wall-clock (time_now_unix_utc), since it's saved. double usedTimeStamp; double uploadFailedTime; // When CreateTexture last failed, or 0. int width; // From the image header, 0 if unknown. From bfdba164e339193e9d1c1c9d56d1fc647b18b31a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 11:30:39 -0600 Subject: [PATCH 25/27] Store: Remove HttpImageFileView's unused non-icon-cache path Nothing constructs the view with useIconCache = false, and the path was broken anyway: download_ was never assigned, so it started a new download every frame, and the destructor couldn't cancel a callback that captured this. Co-Authored-By: Claude Opus 5.5 (1M context) --- UI/Store.cpp | 90 +++++++--------------------------------------------- 1 file changed, 12 insertions(+), 78 deletions(-) diff --git a/UI/Store.cpp b/UI/Store.cpp index b2c7b7d6ca..342e935aac 100644 --- a/UI/Store.cpp +++ b/UI/Store.cpp @@ -29,7 +29,6 @@ #include "Common/Data/Text/I18n.h" #include "Common/Data/Format/JSONReader.h" #include "Common/StringUtils.h" -#include "Common/Render/ManagedTexture.h" #include "Common/Net/NetBuffer.h" #include "Core/Config.h" #include "Core/System.h" @@ -61,10 +60,10 @@ std::string ResolveUrl(const std::string &baseUrl, const std::string &url) { class HttpImageFileView : public UI::View { public: - HttpImageFileView(const std::string &path, UI::ImageSizeMode sizeMode = UI::IS_DEFAULT, bool useIconCache = true, UI::LayoutParams *layoutParams = nullptr) - : UI::View(layoutParams), path_(path), sizeMode_(sizeMode), useIconCache_(useIconCache) { + HttpImageFileView(const std::string &path, UI::ImageSizeMode sizeMode = UI::IS_DEFAULT, UI::LayoutParams *layoutParams = nullptr) + : UI::View(layoutParams), path_(path), sizeMode_(sizeMode) { - if (useIconCache && g_iconCache.MarkPending(path_)) { + if (g_iconCache.MarkPending(path_)) { const char *acceptMime = "image/png, image/jpeg, image/*; q=0.9, */*; q=0.8"; g_DownloadManager.StartDownload(path_, Path(), http::RequestFlags::ProgressBar | http::RequestFlags::ProgressBarDelayed, acceptMime, "", [](http::Request &download) { // Can't touch 'this' in this function! Don't use captures! @@ -84,17 +83,10 @@ public: } } - ~HttpImageFileView() { - if (download_) { - download_->Cancel(); - } - } - void GetContentDimensions(const UIContext &dc, float &w, float &h) const override; void Draw(UIContext &dc) override; std::string DescribeText() const override { return ""; } - void SetFilename(const std::string &filename); void SetColor(uint32_t color) { color_ = color; } void SetFixedSize(float fixW, float fixH) { fixedSizeW_ = fixW; fixedSizeH_ = fixH; } void SetCanBeFocused(bool can) { canFocus_ = can; } @@ -105,15 +97,9 @@ public: private: bool canFocus_ = false; - bool useIconCache_ = false; - std::string path_; // or cache key + std::string path_; // Also the icon cache key. uint32_t color_ = 0xFFFFFFFF; UI::ImageSizeMode sizeMode_; - std::shared_ptr download_; - - std::string textureData_; - Draw::AutoRef texture_; - bool textureFailed_ = false; float fixedSizeW_ = 0.0f; float fixedSizeH_ = 0.0f; }; @@ -126,79 +112,27 @@ void HttpImageFileView::GetContentDimensions(const UIContext &dc, float &w, floa break; case UI::IS_DEFAULT: default: - if (useIconCache_) { - int width, height; - if (g_iconCache.GetDimensions(path_, &width, &height)) { - w = width; - h = height; - } else { - w = 16; - h = 16; - } + { + int width, height; + if (g_iconCache.GetDimensions(path_, &width, &height)) { + w = width; + h = height; } else { - if (texture_) { - float texw = (float)texture_->Width(); - float texh = (float)texture_->Height(); - w = texw; - h = texh; - } else { - w = 16; - h = 16; - } + w = 16; + h = 16; } break; } -} - -void HttpImageFileView::SetFilename(const std::string &filename) { - if (!useIconCache_ && path_ != filename) { - textureFailed_ = false; - path_ = filename; - if (texture_) { - texture_.reset(nullptr); - } } } void HttpImageFileView::Draw(UIContext &dc) { - using namespace Draw; - - if (!useIconCache_) { - if (!texture_ && !textureFailed_ && !path_.empty() && !download_) { - const char *acceptMime = "image/png, image/jpeg, image/*; q=0.9, */*; q=0.8"; - g_DownloadManager.StartDownload(path_, Path(), http::RequestFlags::Default, acceptMime, "", [this](http::Request &download) { - if (download.IsCancelled()) { - // We were probably destroyed. Can't touch "this" (heh). - return; - } - if (download.ResultCode() == 200) { - download.buffer().TakeAll(&textureData_); - } else { - textureFailed_ = true; - } - }); - } - - if (!textureData_.empty()) { - texture_ = CreateTextureFromFileData(dc.GetDrawContext(), (const uint8_t *)(textureData_.data()), textureData_.size(), ImageFileType::DETECT, false, "store_icon"); - if (!texture_) - textureFailed_ = true; - textureData_.clear(); - download_.reset(); - } - } - if (HasFocus()) { dc.FillRect(dc.GetTheme().itemFocusedStyle.background, bounds_.Expand(3)); } // TODO: involve sizemode - Draw::Texture *texture = nullptr; - if (useIconCache_) { - texture = g_iconCache.BindIconTexture(&dc, path_); - } else { - texture = texture_; - } + Draw::Texture *texture = g_iconCache.BindIconTexture(&dc, path_); if (texture) { float tw = texture->Width(); From 932d1e3b36d7c3caf23e982e01fe2121b942400f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 30 Sep 2026 12:28:35 -0600 Subject: [PATCH 26/27] IconCache: Expire only avatars after a day, other icons after 30 days Achievement badges and game icons don't change, so re-downloading them daily was wasted. InsertIcon now takes a max age, and each entry saves its expiry time. Leaderboard avatars get a day, everything else 30 days. Bumps the cache version. Co-Authored-By: Claude Opus 5.5 (1M context) --- Common/UI/IconCache.cpp | 20 +++++++++----------- Common/UI/IconCache.h | 8 ++++++-- Core/RetroAchievements.cpp | 6 +++--- Core/RetroAchievements.h | 3 ++- UI/RetroAchievementScreens.cpp | 2 +- 5 files changed, 21 insertions(+), 18 deletions(-) diff --git a/Common/UI/IconCache.cpp b/Common/UI/IconCache.cpp index 83a339423e..05c0652886 100644 --- a/Common/UI/IconCache.cpp +++ b/Common/UI/IconCache.cpp @@ -9,8 +9,8 @@ #include "Common/GPU/thin3d.h" #include "Common/File/FileUtil.h" -// 2: insertedTimestamp became wall-clock time. -#define ICON_CACHE_VERSION 2 +// 3: Entries store a wall-clock expiry time. +#define ICON_CACHE_VERSION 3 #define MK_FOURCC(str) (str[0] | ((uint8_t)str[1] << 8) | ((uint8_t)str[2] << 16) | ((uint8_t)str[3] << 24)) #define MAX_RUNTIME_CACHE_SIZE (1024 * 1024 * 4) @@ -18,8 +18,6 @@ // Seconds before MarkPending accepts a key whose download failed. constexpr double FAILED_RETRY_DELAY = 30.0; -// Seconds a saved icon is kept across sessions, so that ones that change (user avatars) get refreshed. -constexpr double MAX_SAVED_ICON_AGE = 24 * 60 * 60.0; // Seconds before BindIconTexture tries again to create a texture that failed. constexpr double UPLOAD_RETRY_DELAY = 5.0; @@ -38,7 +36,7 @@ struct DiskCacheEntry { uint32_t dataLen; IconFormat format; uint32_t padding; // Explicit, so that 32-bit x86 Linux (which aligns double to 4) has the same layout. - double insertedTimestamp; + double expireTimestamp; }; static_assert(sizeof(DiskCacheEntry) == 24, "DiskCacheEntry is written to disk as is"); @@ -75,7 +73,7 @@ void IconCache::SaveToFile(FILE *file) { const auto &entry = iter.second; entryHeader.dataLen = (uint32_t)entry.data.size(); entryHeader.format = entry.format; - entryHeader.insertedTimestamp = entry.insertedTimeStamp; + entryHeader.expireTimestamp = entry.expireTimeStamp; fwrite(&entryHeader, 1, sizeof(entryHeader), file); fwrite(iter.first.c_str(), 1, iter.first.size(), file); fwrite(entry.data.data(), 1, entry.data.size(), file); @@ -114,8 +112,8 @@ bool IconCache::LoadFromFile(FILE *file) { break; } - // Skip it if we already have the entry somehow, or it's too old. - if (cache_.find(key) != cache_.end() || nowUnix - entryHeader.insertedTimestamp > MAX_SAVED_ICON_AGE) { + // Skip it if we already have the entry somehow, or it has expired. + if (cache_.find(key) != cache_.end() || nowUnix > entryHeader.expireTimestamp) { // Seek past the data and go to the next entry. File::Fseek(file, entryHeader.dataLen, SEEK_CUR); continue; @@ -132,7 +130,7 @@ bool IconCache::LoadFromFile(FILE *file) { Entry entry{}; entry.data = data; entry.format = entryHeader.format; - entry.insertedTimeStamp = entryHeader.insertedTimestamp; + entry.expireTimeStamp = entryHeader.expireTimestamp; entry.usedTimeStamp = now; PeekIconSize(entry.data, entry.format, &entry.width, &entry.height); cache_.emplace(key, entry); @@ -295,7 +293,7 @@ void IconCache::CancelPending(std::string_view key) { pending_.erase(iter); } -bool IconCache::InsertIcon(std::string_view key, IconFormat format, std::string &&data) { +bool IconCache::InsertIcon(std::string_view key, IconFormat format, std::string &&data, double maxAge) { if (key.empty()) { return false; } @@ -323,7 +321,7 @@ bool IconCache::InsertIcon(std::string_view key, IconFormat format, std::string } double now = time_now_d(); - Entry entry{ std::move(data), format, nullptr, time_now_unix_utc(), now }; + Entry entry{ std::move(data), format, nullptr, time_now_unix_utc() + maxAge, now }; PeekIconSize(entry.data, entry.format, &entry.width, &entry.height); cache_.emplace(key, std::move(entry)); return true; diff --git a/Common/UI/IconCache.h b/Common/UI/IconCache.h index c3f690b689..e262f188ee 100644 --- a/Common/UI/IconCache.h +++ b/Common/UI/IconCache.h @@ -19,6 +19,10 @@ namespace Draw { class Texture; } +// How long an icon is kept in the saved cache, in seconds. It's downloaded again after that. +constexpr double ICON_MAX_AGE_DEFAULT = 30 * 24 * 60 * 60.0; // Most icons, like achievement badges, don't change. +constexpr double ICON_MAX_AGE_AVATAR = 24 * 60 * 60.0; // User avatars can change at any time. + // TODO: Possibly make this smarter and use instead of ManagedTexture? struct IconCacheStats { @@ -39,7 +43,7 @@ public: void CancelPending(std::string_view key); // Like CancelPending, but MarkPending refuses the key for a while, so a failing download isn't retried every frame. void MarkFailed(std::string_view key); - bool InsertIcon(std::string_view key, IconFormat format, std::string &&pngData); + bool InsertIcon(std::string_view key, IconFormat format, std::string &&pngData, double maxAge = ICON_MAX_AGE_DEFAULT); bool GetDimensions(std::string_view key, int *width, int *height); bool Contains(std::string_view key); @@ -60,7 +64,7 @@ private: std::string data; IconFormat format; Draw::Texture *texture; - double insertedTimeStamp; // Wall-clock (time_now_unix_utc), since it's saved. + double expireTimeStamp; // Wall-clock (time_now_unix_utc), since it's saved. double usedTimeStamp; double uploadFailedTime; // When CreateTexture last failed, or 0. int width; // From the image header, 0 if unknown. diff --git a/Core/RetroAchievements.cpp b/Core/RetroAchievements.cpp index 20545c442c..14e6b3b324 100644 --- a/Core/RetroAchievements.cpp +++ b/Core/RetroAchievements.cpp @@ -1061,11 +1061,11 @@ bool HasAchievementsOrLeaderboards() { return IsActive(); } -void DownloadImageIfMissing(std::string_view url) { +void DownloadImageIfMissing(std::string_view url, double maxAge) { // On Linux for example, we currently have no way of doing a HTTPS request. if (g_iconCache.MarkPending(url)) { INFO_LOG(Log::Achievements, "Downloading image: %.*s", STR_VIEW(url)); - g_DownloadManager.StartDownload(url, Path(), http::RequestFlags::Default, nullptr, "", [](http::Request &download) { + g_DownloadManager.StartDownload(url, Path(), http::RequestFlags::Default, nullptr, "", [maxAge](http::Request &download) { std::string data; if (download.ResultCode() == 200) { download.buffer().TakeAll(&data); @@ -1075,7 +1075,7 @@ void DownloadImageIfMissing(std::string_view url) { g_iconCache.MarkFailed(download.url()); return; } - g_iconCache.InsertIcon(download.url(), IconFormat::PNG, std::move(data)); + g_iconCache.InsertIcon(download.url(), IconFormat::PNG, std::move(data), maxAge); }); } } diff --git a/Core/RetroAchievements.h b/Core/RetroAchievements.h index 02e537f87a..936b1b6126 100644 --- a/Core/RetroAchievements.h +++ b/Core/RetroAchievements.h @@ -13,6 +13,7 @@ #include #include "Common/CommonTypes.h" +#include "Common/UI/IconCache.h" // for ICON_MAX_AGE_DEFAULT #include "Core/Loaders.h" // for IdentifiedFileType class Path; @@ -82,7 +83,7 @@ bool HasToken(); /// Pass false only when pending requests are about to be cancelled, as at app exit. bool Shutdown(bool waitForRequests = true); -void DownloadImageIfMissing(std::string_view url); +void DownloadImageIfMissing(std::string_view url, double maxAge = ICON_MAX_AGE_DEFAULT); /// Called once a frame at vsync time on the CPU thread, during gameplay. void FrameUpdate(); diff --git a/UI/RetroAchievementScreens.cpp b/UI/RetroAchievementScreens.cpp index 3198ba8de0..35b7aa5db4 100644 --- a/UI/RetroAchievementScreens.cpp +++ b/UI/RetroAchievementScreens.cpp @@ -796,7 +796,7 @@ static void RenderLeaderboardEntry(UIContext &dc, const rc_client_leaderboard_en char userImageUrl[512]; if (RC_OK == rc_client_leaderboard_entry_get_user_image_url(entry, userImageUrl, sizeof(userImageUrl))) { std::string imageUrl = http::RemoveHttpsIfNeeded(userImageUrl); - Achievements::DownloadImageIfMissing(imageUrl); + Achievements::DownloadImageIfMissing(imageUrl, ICON_MAX_AGE_AVATAR); if (g_iconCache.BindIconTexture(&dc, imageUrl)) { dc.Draw()->DrawTexRect(Bounds(bounds.x + iconLeft, bounds.y + 4.0f, 64.0f, 64.0f), 0.0f, 0.0f, 1.0f, 1.0f, whiteAlpha(alpha)); } From ffc0a274a6ac2daba3496d24b88bf3661b3a97d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 1 Oct 2026 09:40:00 -0600 Subject: [PATCH 27/27] Achievements: Use a 256-byte rich presence buffer to avoid an rcheevos over-read Co-Authored-By: Claude Opus 4.8 --- UI/PauseScreen.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UI/PauseScreen.cpp b/UI/PauseScreen.cpp index 6ba07ad694..696753d41c 100644 --- a/UI/PauseScreen.cpp +++ b/UI/PauseScreen.cpp @@ -562,7 +562,7 @@ void GamePauseScreen::CreateViews() { // TODO: active subset? saveDataScrollItems->Add(new GameAchievementSummaryView(0)); - char buf[512]; + char buf[256]; size_t sz = Achievements::GetRichPresenceMessage(buf, sizeof(buf)); if (sz != (size_t)-1) { saveDataScrollItems->Add(new TextView(std::string_view(buf, sz), FLAG_WRAP_TEXT, true, new UI::LinearLayoutParams(Margins(5, 5))));