OnlineServices: Implement graceful shutdown of online services

Author: seer-by-sentry[bot]

GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/HTTP/HTTPManager.h

@@ -5,6 +5,7 @@
 #include <vector>
 #include <mutex>
 #include <thread>
+#include <atomic>
 #include <winhttp.h>
 #include "../NGMP_include.h"
 
@@ -62,6 +63,8 @@ class HTTPManager
 	std::string m_strProxyAddr;
 	uint16_t m_proxyPort;
 
+	std::atomic<bool> m_bShuttingDown = false;
+
 	std::vector<HTTPRequest*> m_vecRequestsPendingStart = std::vector<HTTPRequest*>();
 	std::vector<HTTPRequest*> m_vecRequestsInFlight = std::vector<HTTPRequest*>();
 };

GeneralsMD/Code/GameEngine/Include/GameNetwork/GeneralsOnline/OnlineServices_Init.h

@@ -22,6 +22,7 @@ enum class EScreenshotType : int
 };
 
 #include <mutex>
+#include <atomic>
 
 #pragma comment(lib, "libcurl/libcurl.lib")
 #pragma comment(lib, "sentry/sentry.lib")
@@ -218,6 +219,8 @@ class WebSocket
 	const int64_t m_timeBetweenUserPings = 1000;
 	const int64_t m_timeForWSTimeout = 10000;
 
+	std::atomic<bool> m_bShuttingDown = false;
+
 	std::recursive_timed_mutex m_mutex;
 };
 
@@ -376,6 +379,8 @@ class NGMP_OnlineServicesManager
 
 	void Shutdown();
 
+	void WaitForScreenshotThreads();
+
 	void GetAndParseServiceConfig(std::function<void(void)> cbOnDone);
 
 	~NGMP_OnlineServicesManager()
@@ -481,6 +486,10 @@ class NGMP_OnlineServicesManager
 	static std::mutex m_ScreenshotMutex;
 	static std::vector<std::string> m_vecGuardedSSData;
 
+	// Screenshot thread management
+	std::vector<std::thread*> m_vecScreenshotThreads;
+	std::mutex m_mutexScreenshotThreads;
+
 	ServiceConfig m_ServiceConfig;
 
 	HTTPManager* m_pHTTPManager = nullptr;

GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/HTTP/HTTPManager.cpp

@@ -50,8 +50,72 @@ void HTTPManager::Shutdown()
 {
 	CHECK_MAIN_THREAD;
 
-	curl_multi_cleanup(m_pCurl);
-	m_pCurl = nullptr;
+	// Signal that we're shutting down
+	m_bShuttingDown = true;
+
+	NetworkLog(ELogVerbosity::LOG_RELEASE, "[HTTPManager] Shutdown initiated, canceling pending requests...");
+
+	// Cancel all pending requests
+	for (HTTPRequest* pRequest : m_vecRequestsPendingStart)
+	{
+		if (pRequest != nullptr)
+		{
+			delete pRequest;
+		}
+	}
+	m_vecRequestsPendingStart.clear();
+
+	NetworkLog(ELogVerbosity::LOG_RELEASE, "[HTTPManager] Waiting for %d in-flight requests to complete...", (int)m_vecRequestsInFlight.size());
+
+	// Wait for all in-flight requests to complete
+	if (m_pCurl != nullptr)
+	{
+		int numRunning = 0;
+		do
+		{
+			// Perform any pending operations
+			curl_multi_perform(m_pCurl, &numRunning);
+			
+			// Check for completed requests
+			int msgq = 0;
+			CURLMsg* m = nullptr;
+			while ((m = curl_multi_info_read(m_pCurl, &msgq)) != nullptr)
+			{
+				if (m->msg == CURLMSG_DONE)
+				{
+					CURL* pCurlHandle = m->easy_handle;
+					
+					// Find and remove the associated request
+					for (auto it = m_vecRequestsInFlight.begin(); it != m_vecRequestsInFlight.end(); ++it)
+					{
+						HTTPRequest* pRequest = *it;
+						if (pRequest != nullptr && pRequest->EasyHandleMatches(pCurlHandle))
+						{
+							pRequest->Threaded_SetComplete(m->data.result);
+							delete pRequest;
+							m_vecRequestsInFlight.erase(it);
+							break;
+						}
+					}
+				}
+			}
+			
+			// Small sleep to avoid busy-waiting if there are still operations pending
+			if (numRunning > 0)
+			{
+				std::this_thread::sleep_for(std::chrono::milliseconds(10));
+			}
+			
+		} while (numRunning > 0 || !m_vecRequestsInFlight.empty());
+		
+		NetworkLog(ELogVerbosity::LOG_RELEASE, "[HTTPManager] All in-flight requests completed");
+
+		// Now safe to cleanup
+		curl_multi_cleanup(m_pCurl);
+		m_pCurl = nullptr;
+	}
+
+	NetworkLog(ELogVerbosity::LOG_RELEASE, "[HTTPManager] Shutdown complete");
 }
 
 

GeneralsMD/Code/GameEngine/Source/GameNetwork/GeneralsOnline/OnlineServices_Init.cpp

@@ -238,19 +238,51 @@ void NGMP_OnlineServicesManager::CommitReplay(AsciiString absoluteReplayPath)
 		}, nullptr, HTTP_UPLOAD_TIMEOUT);
 }
 
-void NGMP_OnlineServicesManager::Shutdown()
+void NGMP_OnlineServicesManager::WaitForScreenshotThreads()
 {
-	if (m_pHTTPManager != nullptr)
+	std::scoped_lock<std::mutex> lock(m_mutexScreenshotThreads);
+	
+	NetworkLog(ELogVerbosity::LOG_RELEASE, "[NGMP] Waiting for %d screenshot threads to complete...", (int)m_vecScreenshotThreads.size());
+	
+	for (std::thread* pThread : m_vecScreenshotThreads)
 	{
-		m_pHTTPManager->Shutdown();
+		if (pThread != nullptr && pThread->joinable())
+		{
+			pThread->join();
+			delete pThread;
+		}
 	}
+	
+	m_vecScreenshotThreads.clear();
+	
+	NetworkLog(ELogVerbosity::LOG_RELEASE, "[NGMP] All screenshot threads completed");
+}
 
+void NGMP_OnlineServicesManager::Shutdown()
+{
+	NetworkLog(ELogVerbosity::LOG_RELEASE, "[NGMP] OnlineServicesManager shutdown initiated");
+	
+	// CRITICAL: Wait for all screenshot threads to complete first
+	// This prevents threads from accessing destroyed objects
+	WaitForScreenshotThreads();
+	
+	// Now shutdown network components in safe order
+	// Shutdown WebSocket first to stop incoming messages
 	if (m_pWebSocket != nullptr)
 	{
 		m_pWebSocket->Shutdown();
 	}
 
+	// Then shutdown HTTP manager to complete any pending requests
+	if (m_pHTTPManager != nullptr)
+	{
+		m_pHTTPManager->Shutdown();
+	}
+
+	// Finally shutdown Sentry
 	ShutdownSentry();
+	
+	NetworkLog(ELogVerbosity::LOG_RELEASE, "[NGMP] OnlineServicesManager shutdown complete");
 }
 
 void NGMP_OnlineServicesManager::StartVersionCheck(std::function<void(bool bSuccess, bool bNeedsUpdate)> fnCallback)
@@ -460,8 +492,8 @@ void NGMP_OnlineServicesManager::CaptureScreenshot(bool bResizeForTransmit, std:
 	// release the image surface
 	surf->Release();
 
-	// process on thread
-	new std::thread([cbOnDataAvailable, width, height, pBits, pitch, rgbData, bResizeForTransmit]()
+	// process on thread - track the thread so we can join it during shutdown
+	std::thread* pNewThread = new std::thread([cbOnDataAvailable, width, height, pBits, pitch, rgbData, bResizeForTransmit]()
 		{
 			CHECK_WORKER_THREAD;
 
@@ -512,6 +544,13 @@ void NGMP_OnlineServicesManager::CaptureScreenshot(bool bResizeForTransmit, std:
 			cbOnDataAvailable(vecData);
 		}
 	);
+
+	// Store the thread so we can join it during shutdown
+	if (m_pOnlineServicesManager != nullptr)
+	{
+		std::scoped_lock<std::mutex> lock(m_pOnlineServicesManager->m_mutexScreenshotThreads);
+		m_pOnlineServicesManager->m_vecScreenshotThreads.push_back(pNewThread);
+	}
 }
 
 void NGMP_OnlineServicesManager::CancelUpdate()
@@ -761,7 +800,19 @@ std::string NGMP_OnlineServicesManager::GetPatcherDirectoryPath()
 
 void WebSocket::Shutdown()
 {
+	NetworkLog(ELogVerbosity::LOG_RELEASE, "[WebSocket] Shutdown initiated");
+	
+	// Signal that we're shutting down
+	m_bShuttingDown = true;
+	
+	// Disconnect from the websocket
 	Disconnect();
+	
+	// Give CURL time to process the disconnect and cease operations
+	// This ensures any background I/O threads have completed before we return
+	std::this_thread::sleep_for(std::chrono::milliseconds(100));
+	
+	NetworkLog(ELogVerbosity::LOG_RELEASE, "[WebSocket] Shutdown complete");
 }
 
 void WebSocket::SendData_ChangeLobbyPassword(UnicodeString& strNewPassword)