Merge #4258

4258: Fix data race in backend_sdl r=def- a=Jupeyy


## 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

@@ -54,44 +54,26 @@ int putenv(const char *);
 }
 #endif
 
-/*
-	sync_barrier - creates a full hardware fence
-*/
-#if defined(__GNUC__)
-inline void sync_barrier()
-{
-	__sync_synchronize();
-}
-#elif defined(_MSC_VER)
-#define WIN32_LEAN_AND_MEAN
-#include <windows.h>
-inline void sync_barrier()
-{
-	MemoryBarrier();
-}
-#else
-#error missing atomic implementation for this compiler
-#endif
-
 // ------------ CGraphicsBackend_Threaded
 
-void CGraphicsBackend_Threaded::ThreadFunc(void *pUser)
+void CGraphicsBackend_Threaded::ThreadFunc()
 {
-	CGraphicsBackend_Threaded *pThis = (CGraphicsBackend_Threaded *)pUser;
-
-	while(!pThis->m_Shutdown)
+	std::unique_lock<std::mutex> Lock(m_BufferSwapMutex);
+	// notify, that the thread started
+	m_BufferDoneCond.notify_all();
+	while(!m_Shutdown)
 	{
-		pThis->m_Activity.Wait();
-		if(pThis->m_pBuffer)
+		m_BufferSwapCond.wait(Lock);
+		if(m_pBuffer)
 		{
 #ifdef CONF_PLATFORM_MACOS
 			CAutoreleasePool AutoreleasePool;
 #endif
-			pThis->m_pProcessor->RunBuffer(pThis->m_pBuffer);
+			m_pProcessor->RunBuffer(m_pBuffer);
 
-			sync_barrier();
-			pThis->m_pBuffer = 0x0;
-			pThis->m_BufferDone.Signal();
+			m_pBuffer = nullptr;
+			m_BufferInProcess.store(false, std::memory_order_relaxed);
+			m_BufferDoneCond.notify_all();
 		}
 #if defined(CONF_VIDEORECORDER)
 		if(IVideo::Current())
@@ -102,43 +84,50 @@ void CGraphicsBackend_Threaded::ThreadFunc(void *pUser)
 
 CGraphicsBackend_Threaded::CGraphicsBackend_Threaded()
 {
-	m_pBuffer = 0x0;
-	m_pProcessor = 0x0;
-	m_pThread = 0x0;
+	m_pBuffer = nullptr;
+	m_pProcessor = nullptr;
+	m_BufferInProcess.store(false, std::memory_order_relaxed);
 }
 
 void CGraphicsBackend_Threaded::StartProcessor(ICommandProcessor *pProcessor)
 {
 	m_Shutdown = false;
 	m_pProcessor = pProcessor;
-	m_pThread = thread_init(ThreadFunc, this, "CGraphicsBackend_Threaded");
-	m_BufferDone.Signal();
+	std::unique_lock<std::mutex> Lock(m_BufferSwapMutex);
+	m_Thread = std::thread([&]() { ThreadFunc(); });
+	// wait for the thread to start
+	m_BufferDoneCond.wait(Lock);
 }
 
 void CGraphicsBackend_Threaded::StopProcessor()
 {
 	m_Shutdown = true;
-	m_Activity.Signal();
-	if(m_pThread)
-		thread_wait(m_pThread);
+	{
+		std::unique_lock<std::mutex> Lock(m_BufferSwapMutex);
+		m_BufferSwapCond.notify_all();
+	}
+	m_Thread.join();
 }
 
 void CGraphicsBackend_Threaded::RunBuffer(CCommandBuffer *pBuffer)
 {
 	WaitForIdle();
 	m_pBuffer = pBuffer;
-	m_Activity.Signal();
+	std::unique_lock<std::mutex> Lock(m_BufferSwapMutex);
+	m_BufferInProcess.store(true, std::memory_order_relaxed);
+	m_BufferSwapCond.notify_all();
 }
 
 bool CGraphicsBackend_Threaded::IsIdle() const
 {
-	return m_pBuffer == 0x0;
+	return !m_BufferInProcess.load(std::memory_order_relaxed);
 }
 
 void CGraphicsBackend_Threaded::WaitForIdle()
 {
-	while(m_pBuffer != 0x0)
-		m_BufferDone.Wait();
+	std::unique_lock<std::mutex> Lock(m_BufferSwapMutex);
+	if(m_pBuffer != nullptr)
+		m_BufferDoneCond.wait(Lock);
 }
 
 // ------------ CCommandProcessorFragment_General

src/engine/client/backend_sdl.h

@@ -13,6 +13,9 @@
 #include <base/tl/threading.h>
 
 #include <atomic>
+#include <condition_variable>
+#include <mutex>
+#include <thread>
 #include <vector>
 
 #if defined(CONF_PLATFORM_MACOS)
@@ -64,13 +67,15 @@ protected:
 
 private:
 	ICommandProcessor *m_pProcessor;
-	CCommandBuffer *volatile m_pBuffer;
+	std::mutex m_BufferSwapMutex;
+	std::condition_variable m_BufferSwapCond;
+	std::condition_variable m_BufferDoneCond;
+	CCommandBuffer *m_pBuffer;
 	std::atomic_bool m_Shutdown;
-	CSemaphore m_Activity;
-	CSemaphore m_BufferDone;
-	void *m_pThread;
+	std::atomic_bool m_BufferInProcess;
+	std::thread m_Thread;
 
-	static void ThreadFunc(void *pUser);
+	void ThreadFunc();
 };
 
 // takes care of implementation independent operations