Merge pull request #22398 from hrydgard/achievements-fixes

Claude code review: RetroAchievements

Common/Net/HTTPClient.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <atomic>
 #include <functional>
 #include <memory>
 #include <thread>
@@ -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<bool> completed_{false};
 	bool failed_ = false;
 	net::ResolveFunc customResolve_;
 };

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<bool> cancelled{false};
 };
@@ -43,8 +45,11 @@ struct NaettBodySink {
 static std::vector<std::unique_ptr<NaettBodySink>> *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<NaettBodySink>();
+	// In case someone managed to cancel us between construction and here.
+	sink_->cancelled = cancelled_;
+	sink_->postData = postData_;
+
 	std::vector<naettOption *> 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<NaettBodySink>();
-	// 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();

Common/UI/IconCache.cpp

@@ -1,4 +1,5 @@
 #include <algorithm>
+#include <cstring>
 
 #include "Common/UI/IconCache.h"
 #include "Common/UI/Context.h"
@@ -8,12 +9,18 @@
 #include "Common/GPU/thin3d.h"
 #include "Common/File/FileUtil.h"
 
-#define ICON_CACHE_VERSION 1
+// 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)
 #define MAX_SAVED_CACHE_SIZE (1024 * 1024 * 1)
 
+// 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");
 
 IconCache g_iconCache;
@@ -28,8 +35,24 @@ struct DiskCacheEntry {
 	uint32_t keyLen;
 	uint32_t dataLen;
 	IconFormat format;
-	double insertedTimestamp;
+	uint32_t padding;  // Explicit, so that 32-bit x86 Linux (which aligns double to 4) has the same layout.
+	double expireTimestamp;
 };
+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<std::mutex> lock(lock_);
@@ -50,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);
@@ -69,6 +92,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{};
@@ -76,19 +100,20 @@ 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;
 		}
 
-		// 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 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;
@@ -105,8 +130,9 @@ 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);
 	}
 
@@ -127,6 +153,7 @@ void IconCache::ClearData() {
 	ClearTextures();
 	std::unique_lock<std::mutex> lock(lock_);
 	cache_.clear();
+	failed_.clear();
 }
 
 void IconCache::FrameUpdate() {
@@ -146,6 +173,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;
 	}
 
@@ -208,14 +242,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) {
@@ -231,10 +263,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<std::mutex> 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<std::mutex> lock(lock_);
 	auto iter = pending_.find(key);
@@ -245,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;
 	}
@@ -273,7 +321,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, false });
+	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;
 }
 
@@ -298,7 +348,7 @@ Draw::Texture *IconCache::BindIconTexture(UIContext *context, std::string_view k
 		return entry.texture;
 	}
 
-	if (entry.badData) {
+	if (entry.uploadFailedTime != 0.0 && time_now_d() < entry.uploadFailedTime + UPLOAD_RETRY_DELAY) {
 		return nullptr;
 	}
 
@@ -316,7 +366,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;
@@ -339,11 +391,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;
 }
 

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 {
@@ -37,7 +41,9 @@ 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);
-	bool InsertIcon(std::string_view key, IconFormat format, std::string &&pngData);
+	// 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, double maxAge = ICON_MAX_AGE_DEFAULT);
 	bool GetDimensions(std::string_view key, int *width, int *height);
 	bool Contains(std::string_view key);
 
@@ -58,13 +64,16 @@ private:
 		std::string data;
 		IconFormat format;
 		Draw::Texture *texture;
-		double insertedTimeStamp;
+		double expireTimeStamp;  // Wall-clock (time_now_unix_utc), since it's saved.
 		double usedTimeStamp;
-		bool badData;
+		double uploadFailedTime;  // When CreateTexture last failed, or 0.
+		int width;  // From the image header, 0 if unknown.
+		int height;
 	};
 
 	std::map<std::string, Entry, std::less<>> cache_;
 	std::set<std::string, std::less<>> pending_;
+	std::map<std::string, double, std::less<>> failed_;  // key -> time of failure
 
 	std::mutex lock_;
 

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;
 		}

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");
 	}

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 <algorithm>
+#include <map>
 #include <set>
 #include <string>
 #include <vector>
@@ -164,11 +165,31 @@ 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;
 
 // 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<uint32_t, TrackedClient> 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;
 
@@ -223,7 +244,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) {
@@ -290,6 +316,47 @@ static uint32_t read_memory_callback(uint32_t address, uint8_t *buffer, uint32_t
 	return num_bytes;
 }
 
+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{};
+	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);
+
+	// 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
 // needs to talk to the server, it will call this function.
 static void server_call_callback(const rc_api_request_t *request,
@@ -298,28 +365,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<http::Request> 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);
+			[clientId, callback, callback_data](http::Request &download) {
+			complete_server_call(clientId, download, callback, callback_data);
 		}, ac->T("Contacting RetroAchievements server..."));
 	} else {
 		std::shared_ptr<http::Request> 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);
+			[clientId, callback, callback_data](http::Request &download) {
+			complete_server_call(clientId, download, callback, callback_data);
 		});
 	}
 }
@@ -351,11 +417,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);
@@ -519,26 +585,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();
@@ -547,6 +617,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;
 }
 
@@ -643,13 +714,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) {
@@ -662,6 +727,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);
@@ -683,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);
@@ -691,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);
@@ -731,8 +804,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) {
@@ -789,7 +865,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);
@@ -801,6 +878,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");
@@ -822,27 +902,40 @@ 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;
 	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;
@@ -853,6 +946,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.
@@ -861,7 +963,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;
 		}
@@ -917,7 +1019,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);
 			}
@@ -932,7 +1034,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);
@@ -959,16 +1061,21 @@ 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) {
-			if (download.ResultCode() != 200)
-				return;
+		g_DownloadManager.StartDownload(url, Path(), http::RequestFlags::Default, nullptr, "", [maxAge](http::Request &download) {
 			std::string data;
-			download.buffer().TakeAll(&data);
-			g_iconCache.InsertIcon(download.url(), IconFormat::PNG, std::move(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), maxAge);
 		});
 	}
 }
@@ -1020,7 +1127,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:
@@ -1152,11 +1259,13 @@ 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) {
 	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) {

Core/RetroAchievements.h

@@ -13,6 +13,7 @@
 #include <set>
 
 #include "Common/CommonTypes.h"
+#include "Common/UI/IconCache.h"  // for ICON_MAX_AGE_DEFAULT
 #include "Core/Loaders.h"  // for IdentifiedFileType
 
 class Path;
@@ -78,9 +79,11 @@ 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);
+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();

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");

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))));

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 {
@@ -788,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));
 		}

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<http::Request> download_;
-
-	std::string textureData_;
-	Draw::AutoRef<Draw::Texture> 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();

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:

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.
+
 `<platform>` is `ARM64` or `x64` - whichever the machine actually is, so look it up rather than
 picking a default. The output directory follows it (`Windows\<platform>\<configuration>\`), which
 makes building one and running the other an easy mistake. It is easiest to make on Windows-on-ARM,
@@ -69,6 +77,15 @@ 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=<platform> /m
+```
+
 ## Android assets
 
 The APK's `assets/` directory is the repo-root `assets/` directory, wired up by

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");
     }

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) {

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);

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);
     }
 }
 

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);
     }
 }
 

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;
@@ -219,11 +219,16 @@ 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;
-                    res->complete = 1;
+                    naettSetComplete(res);
                 }
             }
         } break;
@@ -247,7 +252,7 @@ callback(HINTERNET request, DWORD_PTR context, DWORD status, LPVOID statusInform
                     res->code = naettGenericError;
             }
 
-            res->complete = 1;
+            naettSetComplete(res);
         } break;
     }
 }
@@ -350,7 +355,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);
     }
 }