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).
This commit is contained in:
kuwacom 2026-03-07 18:23:34 +09:00
parent fa09e28572
commit 4bb18290fc
2 changed files with 52 additions and 17 deletions

View file

@ -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;
}
}

View file

@ -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;
}