Fixed GDI interworking deadlocks

Because both the DirectDraw and GDI thread locks are held by the thread that
initially calls beginGdiRendering, a deadlock can occur if a complex rendering
operation uses synchronized worker threads for some subtasks and they also need
access to the resources locked by the initial thread.

To resolve this, the GDI thread lock is no longer held after beginGdiRendering
returns, and the DirectDraw thread lock is only taken during the initial entry.

However, this introduces another problem, because now the final endGdiRendering
might not be called by the same thread that initially called beginGdiRendering.
If this happens, a deadlock will occur because the initial thread is still
holding the DirectDraw thread lock while the other thread is trying to acquire
it to unlock the primary surface.

To resolve this, the initial thread will always be the one to release the lock
on the primary surface, waiting for other threads to finish using GDI DCs if
necessary. This also means that other threads won't be able to create new
cached DCs (as they would need the DD thread lock), so to prevent yet another
deadlock, the initial thread always preallocates a number of DCs in the cache,
and only the initial thread is allowed to extend the cache.

DDrawCompat/CompatGdi.cpp

@@ -10,7 +10,12 @@
 
 namespace
 {
-	DWORD g_renderingDepth = 0;
+	DWORD g_renderingRefCount = 0;
+	DWORD g_ddLockThreadRenderingRefCount = 0;
+	DWORD g_ddLockThreadId = 0;
+	HANDLE g_ddUnlockBeginEvent = nullptr;
+	HANDLE g_ddUnlockEndEvent = nullptr;
+	bool g_isDelayedUnlockPending = false;
 
 	FARPROC getProcAddress(HMODULE module, const char* procName)
 	{
@@ -60,6 +65,35 @@ namespace
 		}
 		return TRUE;
 	}
+
+	bool lockPrimarySurface()
+	{
+		Compat::origProcs.AcquireDDThreadLock();
+
+		DDSURFACEDESC2 desc = {};
+		desc.dwSize = sizeof(desc);
+		if (FAILED(CompatDirectDrawSurface<IDirectDrawSurface7>::s_origVtable.Lock(
+			CompatPrimarySurface::surface, nullptr, &desc, DDLOCK_WAIT, nullptr)))
+		{
+			Compat::origProcs.ReleaseDDThreadLock();
+			return false;
+		}
+
+		g_ddLockThreadId = GetCurrentThreadId();
+		CompatGdiDcCache::setDdLockThreadId(g_ddLockThreadId);
+		CompatGdiDcCache::setSurfaceMemory(desc.lpSurface, desc.lPitch);
+		return true;
+	}
+
+	void unlockPrimarySurface()
+	{
+		GdiFlush();
+		CompatDirectDrawSurface<IDirectDrawSurface7>::s_origVtable.Unlock(
+			CompatPrimarySurface::surface, nullptr);
+		RealPrimarySurface::update();
+
+		Compat::origProcs.ReleaseDDThreadLock();
+	}
 }
 
 namespace CompatGdi
@@ -67,14 +101,23 @@ namespace CompatGdi
 	CRITICAL_SECTION g_gdiCriticalSection;
 	std::unordered_map<void*, const char*> g_funcNames;
 
-	GdiScopedThreadLock::GdiScopedThreadLock()
+	GdiScopedThreadLock::GdiScopedThreadLock() : m_isLocked(true)
 	{
 		EnterCriticalSection(&g_gdiCriticalSection);
 	}
 
 	GdiScopedThreadLock::~GdiScopedThreadLock()
 	{
-		LeaveCriticalSection(&g_gdiCriticalSection);
+		unlock();
+	}
+
+	void GdiScopedThreadLock::unlock()
+	{
+		if (m_isLocked)
+		{
+			LeaveCriticalSection(&g_gdiCriticalSection);
+			m_isLocked = false;
+		}
 	}
 
 	bool beginGdiRendering()
@@ -84,40 +127,63 @@ namespace CompatGdi
 			return false;
 		}
 
-		Compat::origProcs.AcquireDDThreadLock();
-		EnterCriticalSection(&g_gdiCriticalSection);
+		GdiScopedThreadLock gdiLock;
 
-		if (0 == g_renderingDepth)
+		if (0 == g_renderingRefCount)
 		{
-			DDSURFACEDESC2 desc = {};
-			desc.dwSize = sizeof(desc);
-			if (FAILED(CompatDirectDrawSurface<IDirectDrawSurface7>::s_origVtable.Lock(
-				CompatPrimarySurface::surface, nullptr, &desc, DDLOCK_WAIT, nullptr)))
+			if (!lockPrimarySurface())
 			{
-				LeaveCriticalSection(&g_gdiCriticalSection);
-				Compat::origProcs.ReleaseDDThreadLock();
 				return false;
 			}
-			CompatGdiDcCache::setSurfaceMemory(desc.lpSurface, desc.lPitch);
+			++g_ddLockThreadRenderingRefCount;
+		}
+		else if (GetCurrentThreadId() == g_ddLockThreadId)
+		{
+			++g_ddLockThreadRenderingRefCount;
 		}
 
-		++g_renderingDepth;
+		++g_renderingRefCount;
 		return true;
 	}
 
 	void endGdiRendering()
 	{
-		--g_renderingDepth;
-		if (0 == g_renderingDepth)
-		{
-			GdiFlush();
-			CompatDirectDrawSurface<IDirectDrawSurface7>::s_origVtable.Unlock(
-				CompatPrimarySurface::surface, nullptr);
-			RealPrimarySurface::update();
-		}
+		CompatGdi::GdiScopedThreadLock gdiLock;
 
-		LeaveCriticalSection(&g_gdiCriticalSection);
-		Compat::origProcs.ReleaseDDThreadLock();
+		if (GetCurrentThreadId() == g_ddLockThreadId)
+		{
+			if (1 == g_renderingRefCount)
+			{
+				unlockPrimarySurface();
+				g_ddLockThreadRenderingRefCount = 0;
+				g_renderingRefCount = 0;
+			}
+			else if (1 == g_ddLockThreadRenderingRefCount)
+			{
+				g_isDelayedUnlockPending = true;
+				gdiLock.unlock();
+				WaitForSingleObject(g_ddUnlockBeginEvent, INFINITE);
+				unlockPrimarySurface();
+				g_ddLockThreadRenderingRefCount = 0;
+				g_renderingRefCount = 0;
+				SetEvent(g_ddUnlockEndEvent);
+			}
+			else
+			{
+				--g_ddLockThreadRenderingRefCount;
+				--g_renderingRefCount;
+			}
+		}
+		else
+		{
+			--g_renderingRefCount;
+			if (1 == g_renderingRefCount && g_isDelayedUnlockPending)
+			{
+				SetEvent(g_ddUnlockBeginEvent);
+				WaitForSingleObject(g_ddUnlockEndEvent, INFINITE);
+				g_isDelayedUnlockPending = false;
+			}
+		}
 	}
 
 	void hookGdiFunction(const char* moduleName, const char* funcName, void*& origFuncPtr, void* newFuncPtr)
@@ -146,6 +212,14 @@ namespace CompatGdi
 		InitializeCriticalSection(&g_gdiCriticalSection);
 		if (CompatGdiDcCache::init())
 		{
+			g_ddUnlockBeginEvent = CreateEvent(nullptr, FALSE, FALSE, nullptr);
+			g_ddUnlockEndEvent = CreateEvent(nullptr, FALSE, FALSE, nullptr);
+			if (!g_ddUnlockBeginEvent || !g_ddUnlockEndEvent)
+			{
+				Compat::Log() << "Failed to create the unlock events for GDI";
+				return;
+			}
+
 			CompatGdiFunctions::installHooks();
 			CompatGdiWinProc::installHooks();
 			CompatGdiCaret::installHooks();

DDrawCompat/CompatGdi.h

@@ -17,6 +17,9 @@ namespace CompatGdi
 	public:
 		GdiScopedThreadLock();
 		~GdiScopedThreadLock();
+		void unlock();
+	private:
+		bool m_isLocked;
 	};
 
 	extern CRITICAL_SECTION g_gdiCriticalSection;

DDrawCompat/CompatGdiDc.cpp

@@ -1,6 +1,7 @@
 #include <algorithm>
 #include <unordered_map>
 
+#include "CompatGdi.h"
 #include "CompatGdiDc.h"
 #include "CompatGdiDcCache.h"
 #include "DDrawLog.h"
@@ -110,8 +111,14 @@ namespace CompatGdiDc
 {
 	HDC getDc(HDC origDc)
 	{
-		if (!origDc || OBJ_DC != GetObjectType(origDc) || DT_RASDISPLAY != GetDeviceCaps(origDc, TECHNOLOGY) ||
-			g_origDcToCompatDc.end() != std::find_if(g_origDcToCompatDc.begin(), g_origDcToCompatDc.end(),
+		if (!origDc || OBJ_DC != GetObjectType(origDc) || DT_RASDISPLAY != GetDeviceCaps(origDc, TECHNOLOGY))
+		{
+			return nullptr;
+		}
+
+		CompatGdi::GdiScopedThreadLock gdiLock;
+
+		if (g_origDcToCompatDc.end() != std::find_if(g_origDcToCompatDc.begin(), g_origDcToCompatDc.end(),
 				[=](const CompatDcMap::value_type& compatDc) { return compatDc.second.dc == origDc; }))
 		{
 			return nullptr;
@@ -145,6 +152,8 @@ namespace CompatGdiDc
 
 	void releaseDc(HDC origDc)
 	{
+		CompatGdi::GdiScopedThreadLock gdiLock;
+
 		auto it = g_origDcToCompatDc.find(origDc);
 		if (it == g_origDcToCompatDc.end())
 		{

DDrawCompat/CompatGdiDcCache.cpp

@@ -13,6 +13,9 @@ namespace
 	using CompatGdiDcCache::CachedDc;
 	
 	std::vector<CachedDc> g_cache;
+	DWORD g_cacheSize = 0;
+	DWORD g_maxUsedCacheSize = 0;
+	DWORD g_ddLockThreadId = 0;
 
 	IDirectDraw7* g_directDraw = nullptr;
 	void* g_surfaceMemory = nullptr;
@@ -97,6 +100,30 @@ namespace
 		return surface;
 	}
 
+	void extendCache()
+	{
+		if (g_cacheSize >= Config::preallocatedGdiDcCount)
+		{
+			LOG_ONCE("Warning: Preallocated GDI DC count is insufficient. This may lead to graphical issues.");
+		}
+
+		if (GetCurrentThreadId() != g_ddLockThreadId)
+		{
+			return;
+		}
+
+		for (DWORD i = 0; i < Config::preallocatedGdiDcCount; ++i)
+		{
+			CachedDc cachedDc = createCachedDc();
+			if (!cachedDc.dc)
+			{
+				return;
+			}
+			g_cache.push_back(cachedDc);
+			++g_cacheSize;
+		}
+	}
+
 	void releaseCachedDc(CachedDc cachedDc)
 	{
 		// Reacquire DD critical section that was temporarily released after IDirectDrawSurface7::GetDC
@@ -122,6 +149,7 @@ namespace CompatGdiDcCache
 			releaseCachedDc(cachedDc);
 		}
 		g_cache.clear();
+		g_cacheSize = 0;
 	}
 
 	CachedDc getDc()
@@ -134,12 +162,20 @@ namespace CompatGdiDcCache
 
 		if (g_cache.empty())
 		{
-			cachedDc = createCachedDc();
+			extendCache();
 		}
-		else
+
+		if (!g_cache.empty())
 		{
 			cachedDc = g_cache.back();
 			g_cache.pop_back();
+
+			const DWORD usedCacheSize = g_cacheSize - g_cache.size();
+			if (usedCacheSize > g_maxUsedCacheSize)
+			{
+				g_maxUsedCacheSize = usedCacheSize;
+				Compat::Log() << "GDI used DC cache size: " << g_maxUsedCacheSize;
+			}
 		}
 
 		return cachedDc;
@@ -156,6 +192,11 @@ namespace CompatGdiDcCache
 		g_cache.push_back(cachedDc);
 	}
 
+	void setDdLockThreadId(DWORD ddLockThreadId)
+	{
+		g_ddLockThreadId = ddLockThreadId;
+	}
+
 	void setSurfaceMemory(void* surfaceMemory, LONG pitch)
 	{
 		if (g_surfaceMemory == surfaceMemory && g_pitch == pitch)

DDrawCompat/CompatGdiDcCache.h

@@ -18,5 +18,6 @@ namespace CompatGdiDcCache
 	CachedDc getDc();
 	bool init();
 	void releaseDc(const CachedDc& cachedDc);
+	void setDdLockThreadId(DWORD ddLockThreadId);
 	void setSurfaceMemory(void* surfaceMemory, LONG pitch);
 }

DDrawCompat/Config.h

@@ -6,4 +6,5 @@ namespace Config
 {
 	const DWORD minRefreshInterval = 1000 / 60;
 	const DWORD minRefreshIntervalAfterFlip = 1000 / 10;
+	const DWORD preallocatedGdiDcCount = 4;
 }