Store: Fix race condition causing crashes if looking at another game …

…before an icon finishes downloading
hrydgard · Sep 20, 2023 · 50b7af0 · 50b7af0
1 parent 65b995a
commit 50b7af0
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 7 deletions.
diff --git a/Common/UI/IconCache.cpp b/Common/UI/IconCache.cpp
@@ -228,7 +228,7 @@ bool IconCache::MarkPending(const std::string &key) {
 	return true;
 }
 
-void IconCache::Cancel(const std::string &key) {
+void IconCache::CancelPending(const std::string &key) {
 	std::unique_lock<std::mutex> lock(lock_);
 	pending_.erase(key);
 }
@@ -266,6 +266,7 @@ Draw::Texture *IconCache::BindIconTexture(UIContext *context, const std::string
 		return nullptr;
 	}
 
+	// TODO: Cut down on how long we're holding this lock here.
 	std::unique_lock<std::mutex> lock(lock_);
 	auto iter = cache_.find(key);
 	if (iter == cache_.end()) {

diff --git a/Common/UI/IconCache.h b/Common/UI/IconCache.h
@@ -36,7 +36,7 @@ class IconCache {
 
 	// It's okay to call these from any thread.
 	bool MarkPending(const std::string &key);  // returns false if already pending or loaded
-	void Cancel(const std::string &key);
+	void CancelPending(const std::string &key);
 	bool InsertIcon(const std::string &key, IconFormat format, std::string &&pngData);
 	bool GetDimensions(const std::string &key, int *width, int *height);
 	bool Contains(const std::string &key);

diff --git a/Core/HW/SasAudio.cpp b/Core/HW/SasAudio.cpp
@@ -396,6 +396,8 @@ void SasInstance::GetDebugText(char *text, size_t bufsize) {
 					indicator = " (BAD!)";
 				}
 				break;
+			default:
+				break;
 			}
 			p += snprintf(p, sizeof(voiceBuf) - (p - voiceBuf), " %d: Pitch %04x L/R,FX: %d,%d|%d,%d VAG: %08x:%d:%08x%s Height:%d%%\n", i,
 				voices[i].pitch, voices[i].volumeLeft, voices[i].volumeRight, voices[i].effectLeft, voices[i].effectRight,

diff --git a/UI/Store.cpp b/UI/Store.cpp
@@ -63,25 +63,28 @@ class HttpImageFileView : public UI::View {
 
 		if (useIconCache && g_iconCache.MarkPending(path_)) {
 			const char *acceptMime = "image/png, image/jpeg, image/*; q=0.9, */*; q=0.8";
-			requestManager_->StartDownloadWithCallback(path_, Path(), http::ProgressBarMode::DELAYED, [&](http::Request &download) {
+			requestManager_->StartDownloadWithCallback(path_, Path(), http::ProgressBarMode::DELAYED, [](http::Request &download) {
+				// Can't touch 'this' in this function! Don't use captures!
+				std::string path = download.url();
 				if (download.ResultCode() == 200) {
 					std::string data;
 					download.buffer().TakeAll(&data);
 					if (!data.empty()) {
-						g_iconCache.InsertIcon(path_, IconFormat::PNG, std::move(data));
+						g_iconCache.InsertIcon(path, IconFormat::PNG, data);
 					} else {
-						g_iconCache.Cancel(path_);
+						g_iconCache.CancelPending(path);
 					}
 				} else {
-					g_iconCache.Cancel(path_);
+					g_iconCache.CancelPending(path);
 				}
 			}, acceptMime);
 		}
 	}
 
 	~HttpImageFileView() {
-		if (download_)
+		if (download_) {
 			download_->Cancel();
+		}
 	}
 
 	void GetContentDimensions(const UIContext &dc, float &w, float &h) const override;