From 85b0dcdbd5959e0bd5248156a6ed98d2c5c3f642 Mon Sep 17 00:00:00 2001
From: Jupeyy <jupjopjap@gmail.com>
Date: Thu, 28 Oct 2021 00:52:06 +0200
Subject: [PATCH] Fix data race in backend_sdl

---
 src/engine/client/backend_sdl.cpp | 71 +++++++++++++------------------
 src/engine/client/backend_sdl.h   | 15 ++++---
 2 files changed, 40 insertions(+), 46 deletions(-)

diff --git a/src/engine/client/backend_sdl.cpp b/src/engine/client/backend_sdl.cpp
index 2544f8da6..44f27af7e 100644
--- a/src/engine/client/backend_sdl.cpp
+++ b/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
diff --git a/src/engine/client/backend_sdl.h b/src/engine/client/backend_sdl.h
index 2888a4a6f..690bd310d 100644
--- a/src/engine/client/backend_sdl.h
+++ b/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