From 4cdca80c4b8ec7db1d208ebc3ddb5f808a9ae146 Mon Sep 17 00:00:00 2001
From: Daniel Collins <solemnwarning@solemnwarning.net>
Date: Sun, 18 Jun 2017 02:47:51 +0100
Subject: [PATCH] Don't do any cleanup in DllMain() during process termination.

Subtle crashes are fun. I found this to cause a deadlock, but only
after adding lots of extra debug logging to diagnose a DIFFERENT bug
and only when running it on a particular machine.

But once both of those criteria are met? It crashes pretty reliably.
---
 src/directplay.c | 16 +++++++++++++---
 src/ipxwrapper.c | 16 +++++++++++++---
 src/stubdll.c    | 16 +++++++++++++---
 3 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/src/directplay.c b/src/directplay.c
index 6a092f9..5f95df3 100644
--- a/src/directplay.c
+++ b/src/directplay.c
@@ -722,15 +722,25 @@ HRESULT WINAPI SPInit(LPSPINITDATA data) {
 	return DPERR_UNAVAILABLE;
 }
 
-BOOL WINAPI DllMain(HINSTANCE me, DWORD why, LPVOID res) {
-	if(why == DLL_PROCESS_ATTACH)
+BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved) {
+	if(fdwReason == DLL_PROCESS_ATTACH)
 	{
 		log_open("ipxwrapper.log");
 		
 		min_log_level = get_main_config().log_level;
 	}
-	else if(why == DLL_PROCESS_DETACH)
+	else if(fdwReason == DLL_PROCESS_DETACH)
 	{
+		/* When the "lpvReserved" parameter is non-NULL, the process is terminating rather
+		 * than the DLL being unloaded dynamically and any threads will have been terminated
+		 * at unknown points, meaning any global data may be in an inconsistent state and we
+		 * cannot (safely) clean up. MSDN states we should do nothing.
+		*/
+		if(lpvReserved != NULL)
+		{
+			return TRUE;
+		}
+		
 		unload_dlls();
 		log_close();
 	}
diff --git a/src/ipxwrapper.c b/src/ipxwrapper.c
index 25ae94d..48c0d90 100644
--- a/src/ipxwrapper.c
+++ b/src/ipxwrapper.c
@@ -59,9 +59,9 @@ static void init_cs(CRITICAL_SECTION *cs)
 	}
 }
 
-BOOL WINAPI DllMain(HINSTANCE me, DWORD why, LPVOID res)
+BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved)
 {
-	if(why == DLL_PROCESS_ATTACH)
+	if(fdwReason == DLL_PROCESS_ATTACH)
 	{
 		log_open("ipxwrapper.log");
 		
@@ -105,8 +105,18 @@ BOOL WINAPI DllMain(HINSTANCE me, DWORD why, LPVOID res)
 		
 		router_init();
 	}
-	else if(why == DLL_PROCESS_DETACH)
+	else if(fdwReason == DLL_PROCESS_DETACH)
 	{
+		/* When the "lpvReserved" parameter is non-NULL, the process is terminating rather
+		 * than the DLL being unloaded dynamically and any threads will have been terminated
+		 * at unknown points, meaning any global data may be in an inconsistent state and we
+		 * cannot (safely) clean up. MSDN states we should do nothing.
+		*/
+		if(lpvReserved != NULL)
+		{
+			return TRUE;
+		}
+		
 		router_cleanup();
 		
 		WSACleanup();
diff --git a/src/stubdll.c b/src/stubdll.c
index b7324d0..19074a5 100644
--- a/src/stubdll.c
+++ b/src/stubdll.c
@@ -22,15 +22,25 @@
 #include "common.h"
 #include "config.h"
 
-BOOL WINAPI DllMain(HINSTANCE me, DWORD why, LPVOID res) {
-	if(why == DLL_PROCESS_ATTACH)
+BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved) {
+	if(fdwReason == DLL_PROCESS_ATTACH)
 	{
 		log_open("ipxwrapper.log");
 		
 		min_log_level = get_main_config().log_level;
 	}
-	else if(why == DLL_PROCESS_DETACH)
+	else if(fdwReason == DLL_PROCESS_DETACH)
 	{
+		/* When the "lpvReserved" parameter is non-NULL, the process is terminating rather
+		 * than the DLL being unloaded dynamically and any threads will have been terminated
+		 * at unknown points, meaning any global data may be in an inconsistent state and we
+		 * cannot (safely) clean up. MSDN states we should do nothing.
+		*/
+		if(lpvReserved != NULL)
+		{
+			return TRUE;
+		}
+		
 		unload_dlls();
 		log_close();
 	}