From 64b437fa89c0d02646395836e91f8043094e69a8 Mon Sep 17 00:00:00 2001
From: Jonathan G Rennison <j.g.rennison@gmail.com>
Date: Wed, 9 Nov 2022 23:12:30 +0000
Subject: [PATCH] Fix: Data race with mixer thread performance measurements

---
 src/framerate_gui.cpp | 41 +++++++++++++++++++++++++++++++++++++++++
 src/window.cpp        |  3 +++
 2 files changed, 44 insertions(+)

diff --git a/src/framerate_gui.cpp b/src/framerate_gui.cpp
index 903086caa5..6c9cc541ed 100644
--- a/src/framerate_gui.cpp
+++ b/src/framerate_gui.cpp
@@ -25,8 +25,16 @@
 #include "game/game_instance.hpp"
 
 #include "widgets/framerate_widget.h"
+
+#include <atomic>
+#include <mutex>
+#include <vector>
+
 #include "safeguards.h"
 
+static std::mutex _sound_perf_lock;
+static std::atomic<bool> _sound_perf_pending;
+static std::vector<TimingMeasurement> _sound_perf_measurements;
 
 /**
  * Private declarations for performance measurement implementation
@@ -251,6 +259,20 @@ PerformanceMeasurer::~PerformanceMeasurer()
 			return;
 		}
 	}
+	if (this->elem == PFE_SOUND) {
+		/* PFE_SOUND measurements are made from the mixer thread.
+		 * _pf_data cannot be concurrently accessed from the mixer thread
+		 * and the main thread, so store the measurement results in a
+		 * mutex-protected queue which is drained by the main thread.
+		 * See: ProcessPendingPerformanceMeasurements() */
+		TimingMeasurement end = GetPerformanceTimer();
+		std::lock_guard lk(_sound_perf_lock);
+		if (_sound_perf_measurements.size() >= NUM_FRAMERATE_POINTS * 2) return;
+		_sound_perf_measurements.push_back(this->start_time);
+		_sound_perf_measurements.push_back(end);
+		_sound_perf_pending.store(true, std::memory_order_release);
+		return;
+	}
 	_pf_data[this->elem].Add(this->start_time, GetPerformanceTimer());
 }
 
@@ -1079,3 +1101,22 @@ void ConPrintFramerate()
 		IConsolePrint(CC_ERROR, "No performance measurements have been taken yet.");
 	}
 }
+
+/**
+ * This drains the PFE_SOUND measurement data queue into _pf_data.
+ * PFE_SOUND measurements are made by the mixer thread and so cannot be stored
+ * into _pf_data directly, because this would not be thread safe and would violate
+ * the invariants of the FPS and frame graph windows.
+ * @see PerformanceMeasurement::~PerformanceMeasurement()
+ */
+void ProcessPendingPerformanceMeasurements()
+{
+	if (_sound_perf_pending.load(std::memory_order_acquire)) {
+		std::lock_guard lk(_sound_perf_lock);
+		for (size_t i = 0; i < _sound_perf_measurements.size(); i += 2) {
+			_pf_data[PFE_SOUND].Add(_sound_perf_measurements[i], _sound_perf_measurements[i + 1]);
+		}
+		_sound_perf_measurements.clear();
+		_sound_perf_pending.store(false, std::memory_order_relaxed);
+	}
+}
diff --git a/src/window.cpp b/src/window.cpp
index 6d94dacfb3..3cda423f37 100644
--- a/src/window.cpp
+++ b/src/window.cpp
@@ -3067,6 +3067,9 @@ void UpdateWindows()
 	PerformanceMeasurer framerate(PFE_DRAWING);
 	PerformanceAccumulator::Reset(PFE_DRAWWORLD);
 
+	extern void ProcessPendingPerformanceMeasurements();
+	ProcessPendingPerformanceMeasurements();
+
 	CallWindowRealtimeTickEvent(delta_ms);
 
 	static GUITimer network_message_timer = GUITimer(1);