From 22ee875ffc2fa5d6789ca512bd5b9208b919818d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= <hrydgard@gmail.com>
Date: Fri, 27 Sep 2019 23:53:31 +0200
Subject: [PATCH] Fix nasty race condition with menu background audio. Fixes
 #12365

---
 UI/BackgroundAudio.cpp | 60 +++++++++++++++++++++++-------------------
 UI/BackgroundAudio.h   |  1 +
 UI/GameInfoCache.cpp   |  1 +
 UI/NativeApp.cpp       |  6 +++++
 4 files changed, 41 insertions(+), 27 deletions(-)

diff --git a/UI/BackgroundAudio.cpp b/UI/BackgroundAudio.cpp
index fb4ec84478..f5bdd81da4 100644
--- a/UI/BackgroundAudio.cpp
+++ b/UI/BackgroundAudio.cpp
@@ -223,7 +223,7 @@ private:
 	SimpleAudio *decoder_ = nullptr;
 };
 
-static std::mutex bgMutex;
+static std::mutex g_bgMutex;
 static std::string bgGamePath;
 static int playbackOffset;
 static AT3PlusReader *at3Reader;
@@ -251,7 +251,7 @@ static void ClearBackgroundAudio(bool hard) {
 void SetBackgroundAudioGame(const std::string &path) {
 	time_update();
 
-	std::lock_guard<std::mutex> lock(bgMutex);
+	std::lock_guard<std::mutex> lock(g_bgMutex);
 	if (path == bgGamePath) {
 		// Do nothing
 		return;
@@ -272,7 +272,7 @@ void SetBackgroundAudioGame(const std::string &path) {
 int PlayBackgroundAudio() {
 	time_update();
 
-	std::lock_guard<std::mutex> lock(bgMutex);
+	std::lock_guard<std::mutex> lock(g_bgMutex);
 
 	// Immediately stop the sound if it is turned off while playing.
 	if (!g_Config.bEnableSound) {
@@ -281,30 +281,6 @@ int PlayBackgroundAudio() {
 		return 0;
 	}
 
-	// If there's a game, and some time has passed since the selected game
-	// last changed... (to prevent crazy amount of reads when skipping through a list)
-	if (!at3Reader && bgGamePath.size() && (time_now_d() - gameLastChanged > 0.5)) {
-		// Grab some audio from the current game and play it.
-		if (!g_gameInfoCache)
-			return 0;  // race condition?
-
-		// This is very unsafe! TODO: Get rid of this somehow or make threadsafe!
-		std::shared_ptr<GameInfo> gameInfo = g_gameInfoCache->GetInfo(NULL, bgGamePath, GAMEINFO_WANTSND);
-		if (!gameInfo)
-			return 0;
-
-		if (gameInfo->pending) {
-			// Should try again shortly..
-			return 0;
-		}
-
-		if (gameInfo->sndFileData.size()) {
-			const std::string &data = gameInfo->sndFileData;
-			at3Reader = new AT3PlusReader(data);
-			lastPlaybackTime = 0.0;
-		}
-	}
-
 	double now = time_now();
 	if (at3Reader) {
 		int sz = lastPlaybackTime <= 0.0 ? 44100 / 60 : (int)((now - lastPlaybackTime) * 44100);
@@ -336,3 +312,33 @@ int PlayBackgroundAudio() {
 
 	return 0;
 }
+
+// Stuff that should be on the UI thread only, like anything to do with
+// g_gameInfoCache.
+void UpdateBackgroundAudio() {
+	// If there's a game, and some time has passed since the selected game
+	// last changed... (to prevent crazy amount of reads when skipping through a list)
+	if (bgGamePath.size() && (time_now_d() - gameLastChanged > 0.5)) {
+		std::lock_guard<std::mutex> lock(g_bgMutex);
+		if (!at3Reader) {
+			// Grab some audio from the current game and play it.
+			if (!g_gameInfoCache)
+				return;
+
+			std::shared_ptr<GameInfo> gameInfo = g_gameInfoCache->GetInfo(NULL, bgGamePath, GAMEINFO_WANTSND);
+			if (!gameInfo)
+				return;
+
+			if (gameInfo->pending) {
+				// Should try again shortly..
+				return;
+			}
+
+			if (gameInfo->sndFileData.size()) {
+				const std::string &data = gameInfo->sndFileData;
+				at3Reader = new AT3PlusReader(data);
+				lastPlaybackTime = 0.0;
+			}
+		}
+	}
+}
diff --git a/UI/BackgroundAudio.h b/UI/BackgroundAudio.h
index 34b5dbd04a..5d9ae1e040 100644
--- a/UI/BackgroundAudio.h
+++ b/UI/BackgroundAudio.h
@@ -4,3 +4,4 @@
 
 void SetBackgroundAudioGame(const std::string &path);
 int PlayBackgroundAudio();
+void UpdateBackgroundAudio();
diff --git a/UI/GameInfoCache.cpp b/UI/GameInfoCache.cpp
index 2ccdd7f6ed..8402eea6ee 100644
--- a/UI/GameInfoCache.cpp
+++ b/UI/GameInfoCache.cpp
@@ -734,6 +734,7 @@ void GameInfoCache::WaitUntilDone(std::shared_ptr<GameInfo> &info) {
 
 
 // Runs on the main thread. Only call from render() and similar, not update()!
+// Can also be called from the audio thread for menu background music.
 std::shared_ptr<GameInfo> GameInfoCache::GetInfo(Draw::DrawContext *draw, const std::string &gamePath, int wantFlags) {
 	std::shared_ptr<GameInfo> info;
 
diff --git a/UI/NativeApp.cpp b/UI/NativeApp.cpp
index da9d928766..03c920bee9 100644
--- a/UI/NativeApp.cpp
+++ b/UI/NativeApp.cpp
@@ -959,6 +959,12 @@ void RenderOverlays(UIContext *dc, void *userdata) {
 void NativeRender(GraphicsContext *graphicsContext) {
 	g_GameManager.Update();
 
+	if (GetUIState() != UISTATE_INGAME) {
+		// Note: We do this from NativeRender so that the graphics context is
+		// guaranteed valid, to be safe - g_gameInfoCache messes around with textures.
+		UpdateBackgroundAudio();
+	}
+
 	float xres = dp_xres;
 	float yres = dp_yres;