Merge #4261

4261: Fix various tsan warnings r=def- a=Jupeyy

fixes #4259

Didn't find any further issues for now, maybe with other configs or faster/slower processor more are detected

## Checklist

- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)


Co-authored-by: Jupeyy <jupjopjap@gmail.com>

src/engine/client/backend_sdl.cpp

@@ -112,8 +112,8 @@ void CGraphicsBackend_Threaded::StopProcessor()
 void CGraphicsBackend_Threaded::RunBuffer(CCommandBuffer *pBuffer)
 {
 	WaitForIdle();
-	m_pBuffer = pBuffer;
 	std::unique_lock<std::mutex> Lock(m_BufferSwapMutex);
+	m_pBuffer = pBuffer;
 	m_BufferInProcess.store(true, std::memory_order_relaxed);
 	m_BufferSwapCond.notify_all();
 }

src/engine/client/http.cpp

@@ -183,9 +183,9 @@ size_t CRequest::WriteCallback(char *pData, size_t Size, size_t Number, void *pU
 int CRequest::ProgressCallback(void *pUser, double DlTotal, double DlCurr, double UlTotal, double UlCurr)
 {
 	CGetFile *pTask = (CGetFile *)pUser;
-	pTask->m_Current = DlCurr;
-	pTask->m_Size = DlTotal;
-	pTask->m_Progress = (100 * DlCurr) / (DlTotal ? DlTotal : 1);
+	pTask->m_Current.store(DlCurr, std::memory_order_relaxed);
+	pTask->m_Size.store(DlTotal, std::memory_order_relaxed);
+	pTask->m_Progress.store((100 * DlCurr) / (DlTotal ? DlTotal : 1), std::memory_order_relaxed);
 	pTask->OnProgress();
 	return pTask->m_Abort ? -1 : 0;
 }

src/engine/client/http.h

@@ -1,6 +1,7 @@
 #ifndef ENGINE_CLIENT_HTTP_H
 #define ENGINE_CLIENT_HTTP_H
 
+#include <atomic>
 #include <engine/kernel.h>
 #include <engine/shared/jobs.h>
 #include <engine/storage.h>
@@ -46,9 +47,9 @@ class CRequest : public IJob
 
 	CTimeout m_Timeout;
 
-	double m_Size;
-	double m_Current;
-	int m_Progress;
+	std::atomic<double> m_Size;
+	std::atomic<double> m_Current;
+	std::atomic<int> m_Progress;
 	HTTPLOG m_LogProgress;
 
 	std::atomic<int> m_State;
@@ -66,9 +67,9 @@ protected:
 public:
 	CRequest(const char *pUrl, CTimeout Timeout, HTTPLOG LogProgress = HTTPLOG::ALL);
 
-	double Current() const { return m_Current; }
-	double Size() const { return m_Size; }
-	int Progress() const { return m_Progress; }
+	double Current() const { return m_Current.load(std::memory_order_relaxed); }
+	double Size() const { return m_Size.load(std::memory_order_relaxed); }
+	int Progress() const { return m_Progress.load(std::memory_order_relaxed); }
 	int State() const { return m_State; }
 	void Abort() { m_Abort = true; }
 };

src/engine/client/sound.cpp

@@ -8,6 +8,7 @@
 #include <engine/storage.h>
 
 #include <engine/shared/config.h>
+#include <mutex>
 
 #include "SDL.h"
 
@@ -69,15 +70,15 @@ static CSample m_aSamples[NUM_SAMPLES] = {{0}};
 static CVoice m_aVoices[NUM_VOICES] = {{0}};
 static CChannel m_aChannels[NUM_CHANNELS] = {{255, 0}};
 
-static LOCK m_SoundLock = 0;
+static std::mutex m_SoundLock;
 
-static int m_CenterX = 0;
-static int m_CenterY = 0;
+static std::atomic<int> m_CenterX{0};
+static std::atomic<int> m_CenterY{0};
 
 static int m_MixingRate = 48000;
 static std::atomic<int> m_SoundVolume{100};
 
-static int m_NextVoice GUARDED_BY(m_SoundLock) = 0;
+static int m_NextVoice = 0;
 static int *m_pMixBuffer = 0; // buffer only used by the thread callback function
 static unsigned m_MaxFrames = 0;
 
@@ -112,7 +113,7 @@ static void Mix(short *pFinalOut, unsigned Frames)
 	Frames = minimum(Frames, m_MaxFrames);
 
 	// acquire lock while we are mixing
-	lock_wait(m_SoundLock);
+	m_SoundLock.lock();
 
 	MasterVol = m_SoundVolume;
 
@@ -144,8 +145,8 @@ static void Mix(short *pFinalOut, unsigned Frames)
 			if(Voice.m_Flags & ISound::FLAG_POS && Voice.m_pChannel->m_Pan)
 			{
 				// TODO: we should respect the channel panning value
-				int dx = Voice.m_X - m_CenterX;
-				int dy = Voice.m_Y - m_CenterY;
+				int dx = Voice.m_X - m_CenterX.load(std::memory_order_relaxed);
+				int dy = Voice.m_Y - m_CenterY.load(std::memory_order_relaxed);
 				//
 				int p = IntAbs(dx);
 				float FalloffX = 0.0f;
@@ -255,7 +256,7 @@ static void Mix(short *pFinalOut, unsigned Frames)
 	}
 
 	// release the lock
-	lock_unlock(m_SoundLock);
+	m_SoundLock.unlock();
 
 	{
 		// clamp accumulated values
@@ -299,8 +300,6 @@ int CSound::Init()
 
 	SDL_AudioSpec Format, FormatOut;
 
-	m_SoundLock = lock_create();
-
 	if(!g_Config.m_SndEnable)
 		return 0;
 
@@ -351,9 +350,8 @@ int CSound::Update()
 
 	if(WantedVolume != m_SoundVolume)
 	{
-		lock_wait(m_SoundLock);
+		std::unique_lock<std::mutex> Lock(m_SoundLock);
 		m_SoundVolume = WantedVolume;
-		lock_unlock(m_SoundLock);
 	}
 	//#if defined(CONF_VIDEORECORDER)
 	//	if(IVideo::Current() && g_Config.m_ClVideoSndEnable)
@@ -371,7 +369,6 @@ int CSound::Shutdown()
 
 	SDL_CloseAudioDevice(m_Device);
 	SDL_QuitSubSystem(SDL_INIT_AUDIO);
-	lock_destroy(m_SoundLock);
 	free(m_pMixBuffer);
 	m_pMixBuffer = 0;
 	return 0;
@@ -761,8 +758,8 @@ float CSound::GetSampleDuration(int SampleID)
 
 void CSound::SetListenerPos(float x, float y)
 {
-	m_CenterX = (int)x;
-	m_CenterY = (int)y;
+	m_CenterX.store((int)x, std::memory_order_relaxed);
+	m_CenterY.store((int)y, std::memory_order_relaxed);
 }
 
 void CSound::SetVoiceVolume(CVoiceHandle Voice, float Volume)
@@ -772,6 +769,7 @@ void CSound::SetVoiceVolume(CVoiceHandle Voice, float Volume)
 
 	int VoiceID = Voice.Id();
 
+	std::unique_lock<std::mutex> Lock(m_SoundLock);
 	if(m_aVoices[VoiceID].m_Age != Voice.Age())
 		return;
 
@@ -786,6 +784,7 @@ void CSound::SetVoiceFalloff(CVoiceHandle Voice, float Falloff)
 
 	int VoiceID = Voice.Id();
 
+	std::unique_lock<std::mutex> Lock(m_SoundLock);
 	if(m_aVoices[VoiceID].m_Age != Voice.Age())
 		return;
 
@@ -800,6 +799,7 @@ void CSound::SetVoiceLocation(CVoiceHandle Voice, float x, float y)
 
 	int VoiceID = Voice.Id();
 
+	std::unique_lock<std::mutex> Lock(m_SoundLock);
 	if(m_aVoices[VoiceID].m_Age != Voice.Age())
 		return;
 
@@ -814,10 +814,10 @@ void CSound::SetVoiceTimeOffset(CVoiceHandle Voice, float offset)
 
 	int VoiceID = Voice.Id();
 
+	std::unique_lock<std::mutex> Lock(m_SoundLock);
 	if(m_aVoices[VoiceID].m_Age != Voice.Age())
 		return;
 
-	lock_wait(m_SoundLock);
 	{
 		if(m_aVoices[VoiceID].m_pSample)
 		{
@@ -841,7 +841,6 @@ void CSound::SetVoiceTimeOffset(CVoiceHandle Voice, float offset)
 			}
 		}
 	}
-	lock_unlock(m_SoundLock);
 }
 
 void CSound::SetVoiceCircle(CVoiceHandle Voice, float Radius)
@@ -851,6 +850,7 @@ void CSound::SetVoiceCircle(CVoiceHandle Voice, float Radius)
 
 	int VoiceID = Voice.Id();
 
+	std::unique_lock<std::mutex> Lock(m_SoundLock);
 	if(m_aVoices[VoiceID].m_Age != Voice.Age())
 		return;
 
@@ -865,6 +865,7 @@ void CSound::SetVoiceRectangle(CVoiceHandle Voice, float Width, float Height)
 
 	int VoiceID = Voice.Id();
 
+	std::unique_lock<std::mutex> Lock(m_SoundLock);
 	if(m_aVoices[VoiceID].m_Age != Voice.Age())
 		return;
 
@@ -885,7 +886,7 @@ ISound::CVoiceHandle CSound::Play(int ChannelID, int SampleID, int Flags, float
 	int Age = -1;
 	int i;
 
-	lock_wait(m_SoundLock);
+	m_SoundLock.lock();
 
 	// search for voice
 	for(i = 0; i < NUM_VOICES; i++)
@@ -918,7 +919,7 @@ ISound::CVoiceHandle CSound::Play(int ChannelID, int SampleID, int Flags, float
 		Age = m_aVoices[VoiceID].m_Age;
 	}
 
-	lock_unlock(m_SoundLock);
+	m_SoundLock.unlock();
 	return CreateVoiceHandle(VoiceID, Age);
 }
 
@@ -935,7 +936,7 @@ ISound::CVoiceHandle CSound::Play(int ChannelID, int SampleID, int Flags)
 void CSound::Stop(int SampleID)
 {
 	// TODO: a nice fade out
-	lock_wait(m_SoundLock);
+	std::unique_lock<std::mutex> Lock(m_SoundLock);
 	CSample *pSample = &m_aSamples[SampleID];
 	for(auto &Voice : m_aVoices)
 	{
@@ -948,13 +949,12 @@ void CSound::Stop(int SampleID)
 			Voice.m_pSample = 0;
 		}
 	}
-	lock_unlock(m_SoundLock);
 }
 
 void CSound::StopAll()
 {
 	// TODO: a nice fade out
-	lock_wait(m_SoundLock);
+	std::unique_lock<std::mutex> Lock(m_SoundLock);
 	for(auto &Voice : m_aVoices)
 	{
 		if(Voice.m_pSample)
@@ -966,7 +966,6 @@ void CSound::StopAll()
 		}
 		Voice.m_pSample = 0;
 	}
-	lock_unlock(m_SoundLock);
 }
 
 void CSound::StopVoice(CVoiceHandle Voice)
@@ -976,15 +975,14 @@ void CSound::StopVoice(CVoiceHandle Voice)
 
 	int VoiceID = Voice.Id();
 
+	std::unique_lock<std::mutex> Lock(m_SoundLock);
 	if(m_aVoices[VoiceID].m_Age != Voice.Age())
 		return;
 
-	lock_wait(m_SoundLock);
 	{
 		m_aVoices[VoiceID].m_pSample = 0;
 		m_aVoices[VoiceID].m_Age++;
 	}
-	lock_unlock(m_SoundLock);
 }
 
 IEngineSound *CreateEngineSound() { return new CSound; }