From 4bb18290fc903a0099511b4b4d0232ce4f70d8a4 Mon Sep 17 00:00:00 2001
From: kuwacom <kuwa.com3@gmail.com>
Date: Sat, 7 Mar 2026 18:23:34 +0900
Subject: [PATCH] fix: send DisconnectPacket on shutdown and fix Win64
 recv-thread teardown race

Before this change, server/host shutdown closed sockets directly in
ServerConnection::stop(), which bypassed the normal disconnect flow.
As a result, clients could be dropped without receiving a proper
DisconnectPacket during stop/kill/world-close paths.

Also, WinsockNetLayer::Shutdown() could destroy synchronization objects
while host-side recv threads were still exiting, causing a crash in
RecvThreadProc (access violation on world close in host mode).
---
 Minecraft.Client/ServerConnection.cpp         | 31 ++++++++++-----
 .../Windows64/Network/WinsockNetLayer.cpp     | 38 +++++++++++++++----
 2 files changed, 52 insertions(+), 17 deletions(-)

diff --git a/Minecraft.Client/ServerConnection.cpp b/Minecraft.Client/ServerConnection.cpp
index 07616aa47..d82e792ae 100644
--- a/Minecraft.Client/ServerConnection.cpp
+++ b/Minecraft.Client/ServerConnection.cpp
@@ -46,19 +46,30 @@ void ServerConnection::handleConnection(shared_ptr<PendingConnection> uc)
 
 void ServerConnection::stop()
 {
+	std::vector<shared_ptr<PendingConnection> > pendingSnapshot;
 	EnterCriticalSection(&pending_cs);
-    for (unsigned int i = 0; i < pending.size(); i++)
-	{
-        shared_ptr<PendingConnection> uc = pending[i];
-        uc->connection->close(DisconnectPacket::eDisconnect_Closed);
-    }
+	pendingSnapshot = pending;
 	LeaveCriticalSection(&pending_cs);
 
-    for (unsigned int i = 0; i < players.size(); i++)
+	for (unsigned int i = 0; i < pendingSnapshot.size(); i++)
 	{
-        shared_ptr<PlayerConnection> player = players[i];
-        player->connection->close(DisconnectPacket::eDisconnect_Closed);
-    }
+		shared_ptr<PendingConnection> uc = pendingSnapshot[i];
+		if (uc != NULL && !uc->done)
+		{
+			uc->disconnect(DisconnectPacket::eDisconnect_Closed);
+		}
+	}
+
+	// Disconnect through PlayerConnection so clients receive a proper DisconnectPacket before socket close.
+	std::vector<shared_ptr<PlayerConnection> > playerSnapshot = players;
+	for (unsigned int i = 0; i < playerSnapshot.size(); i++)
+	{
+		shared_ptr<PlayerConnection> player = playerSnapshot[i];
+		if (player != NULL && !player->done)
+		{
+			player->disconnect(DisconnectPacket::eDisconnect_Quitting);
+		}
+	}
 }
 
 void ServerConnection::tick()
@@ -202,4 +213,4 @@ void ServerConnection::handleServerSettingsChanged(shared_ptr<ServerSettingsChan
 vector< shared_ptr<PlayerConnection> >  * ServerConnection::getPlayers()
 {
 	return &players;
-}
\ No newline at end of file
+}
diff --git a/Minecraft.Client/Windows64/Network/WinsockNetLayer.cpp b/Minecraft.Client/Windows64/Network/WinsockNetLayer.cpp
index c68505743..eae056e3d 100644
--- a/Minecraft.Client/Windows64/Network/WinsockNetLayer.cpp
+++ b/Minecraft.Client/Windows64/Network/WinsockNetLayer.cpp
@@ -106,6 +106,15 @@ void WinsockNetLayer::Shutdown()
 		s_hostConnectionSocket = INVALID_SOCKET;
 	}
 
+	// Stop accept loop first so no new RecvThread can be created while shutting down.
+	if (s_acceptThread != NULL)
+	{
+		WaitForSingleObject(s_acceptThread, 2000);
+		CloseHandle(s_acceptThread);
+		s_acceptThread = NULL;
+	}
+
+	std::vector<HANDLE> recvThreads;
 	EnterCriticalSection(&s_connectionsLock);
 	for (size_t i = 0; i < s_connections.size(); i++)
 	{
@@ -113,18 +122,27 @@ void WinsockNetLayer::Shutdown()
 		if (s_connections[i].tcpSocket != INVALID_SOCKET)
 		{
 			closesocket(s_connections[i].tcpSocket);
+			s_connections[i].tcpSocket = INVALID_SOCKET;
+		}
+		if (s_connections[i].recvThread != NULL)
+		{
+			recvThreads.push_back(s_connections[i].recvThread);
+			s_connections[i].recvThread = NULL;
 		}
 	}
-	s_connections.clear();
 	LeaveCriticalSection(&s_connectionsLock);
 
-	if (s_acceptThread != NULL)
+	// Ensure all host-side receive threads have exited before destroying locks.
+	for (size_t i = 0; i < recvThreads.size(); i++)
 	{
-		WaitForSingleObject(s_acceptThread, 2000);
-		CloseHandle(s_acceptThread);
-		s_acceptThread = NULL;
+		WaitForSingleObject(recvThreads[i], 2000);
+		CloseHandle(recvThreads[i]);
 	}
 
+	EnterCriticalSection(&s_connectionsLock);
+	s_connections.clear();
+	LeaveCriticalSection(&s_connectionsLock);
+
 	if (s_clientRecvThread != NULL)
 	{
 		WaitForSingleObject(s_clientRecvThread, 2000);
@@ -134,14 +152,20 @@ void WinsockNetLayer::Shutdown()
 
 	if (s_initialized)
 	{
+		EnterCriticalSection(&s_disconnectLock);
+		s_disconnectedSmallIds.clear();
+		LeaveCriticalSection(&s_disconnectLock);
+
+		EnterCriticalSection(&s_freeSmallIdLock);
+		s_freeSmallIds.clear();
+		LeaveCriticalSection(&s_freeSmallIdLock);
+
 		DeleteCriticalSection(&s_sendLock);
 		DeleteCriticalSection(&s_connectionsLock);
 		DeleteCriticalSection(&s_advertiseLock);
 		DeleteCriticalSection(&s_discoveryLock);
 		DeleteCriticalSection(&s_disconnectLock);
-		s_disconnectedSmallIds.clear();
 		DeleteCriticalSection(&s_freeSmallIdLock);
-		s_freeSmallIds.clear();
 		WSACleanup();
 		s_initialized = false;
 	}