From 36e22f3a7bc0f86c650a293e2f624ac5ddfffa84 Mon Sep 17 00:00:00 2001 From: Patric Stout Date: Tue, 11 May 2021 12:26:16 +0200 Subject: Fix: [Network] clients leaving because of broken connections was not broadcasted (#9238) The code mixed up "client has quit but we already told everyone" with "client lost connection, handle this". Split up those two signals: - CLIENT_QUIT means we told everyone and the connection is now dead - CONNECTION_LIST means we should tell everyone we lost a client --- src/network/core/core.h | 21 +++++++++++---------- src/network/core/tcp_admin.cpp | 2 +- src/network/core/tcp_game.cpp | 4 ++-- src/network/network.cpp | 4 ++-- src/network/network_client.cpp | 2 +- src/network/network_server.cpp | 16 ++++++++-------- 6 files changed, 25 insertions(+), 24 deletions(-) (limited to 'src') diff --git a/src/network/core/core.h b/src/network/core/core.h index aac36080f..37948ad52 100644 --- a/src/network/core/core.h +++ b/src/network/core/core.h @@ -20,16 +20,17 @@ void NetworkCoreShutdown(); /** Status of a network client; reasons why a client has quit */ enum NetworkRecvStatus { - NETWORK_RECV_STATUS_OKAY, ///< Everything is okay - NETWORK_RECV_STATUS_DESYNC, ///< A desync did occur - NETWORK_RECV_STATUS_NEWGRF_MISMATCH, ///< We did not have the required NewGRFs - NETWORK_RECV_STATUS_SAVEGAME, ///< Something went wrong (down)loading the savegame - NETWORK_RECV_STATUS_CONN_LOST, ///< The connection is 'just' lost - NETWORK_RECV_STATUS_MALFORMED_PACKET, ///< We apparently send a malformed packet - NETWORK_RECV_STATUS_SERVER_ERROR, ///< The server told us we made an error - NETWORK_RECV_STATUS_SERVER_FULL, ///< The server is full - NETWORK_RECV_STATUS_SERVER_BANNED, ///< The server has banned us - NETWORK_RECV_STATUS_CLOSE_QUERY, ///< Done querying the server + NETWORK_RECV_STATUS_OKAY, ///< Everything is okay. + NETWORK_RECV_STATUS_DESYNC, ///< A desync did occur. + NETWORK_RECV_STATUS_NEWGRF_MISMATCH, ///< We did not have the required NewGRFs. + NETWORK_RECV_STATUS_SAVEGAME, ///< Something went wrong (down)loading the savegame. + NETWORK_RECV_STATUS_CLIENT_QUIT, ///< The connection is lost gracefully. Other clients are already informed of this leaving client. + NETWORK_RECV_STATUS_MALFORMED_PACKET, ///< We apparently send a malformed packet. + NETWORK_RECV_STATUS_SERVER_ERROR, ///< The server told us we made an error. + NETWORK_RECV_STATUS_SERVER_FULL, ///< The server is full. + NETWORK_RECV_STATUS_SERVER_BANNED, ///< The server has banned us. + NETWORK_RECV_STATUS_CLOSE_QUERY, ///< Done querying the server. + NETWORK_RECV_STATUS_CONNECTION_LOST, ///< The connection is lost unexpectedly. }; /** Forward declaration due to circular dependencies */ diff --git a/src/network/core/tcp_admin.cpp b/src/network/core/tcp_admin.cpp index c72583f55..36daae4a1 100644 --- a/src/network/core/tcp_admin.cpp +++ b/src/network/core/tcp_admin.cpp @@ -41,7 +41,7 @@ NetworkAdminSocketHandler::~NetworkAdminSocketHandler() NetworkRecvStatus NetworkAdminSocketHandler::CloseConnection(bool error) { delete this; - return NETWORK_RECV_STATUS_CONN_LOST; + return NETWORK_RECV_STATUS_CLIENT_QUIT; } /** diff --git a/src/network/core/tcp_game.cpp b/src/network/core/tcp_game.cpp index eb53db5ac..771ea37b1 100644 --- a/src/network/core/tcp_game.cpp +++ b/src/network/core/tcp_game.cpp @@ -48,10 +48,10 @@ NetworkRecvStatus NetworkGameSocketHandler::CloseConnection(bool error) _networking = false; ShowErrorMessage(STR_NETWORK_ERROR_LOSTCONNECTION, INVALID_STRING_ID, WL_CRITICAL); - return NETWORK_RECV_STATUS_CONN_LOST; + return NETWORK_RECV_STATUS_CLIENT_QUIT; } - return this->CloseConnection(error ? NETWORK_RECV_STATUS_SERVER_ERROR : NETWORK_RECV_STATUS_CONN_LOST); + return this->CloseConnection(NETWORK_RECV_STATUS_CONNECTION_LOST); } diff --git a/src/network/network.cpp b/src/network/network.cpp index 4fdbdab73..2186cb8a1 100644 --- a/src/network/network.cpp +++ b/src/network/network.cpp @@ -585,13 +585,13 @@ void NetworkClose(bool close_admins) } for (NetworkClientSocket *cs : NetworkClientSocket::Iterate()) { - cs->CloseConnection(NETWORK_RECV_STATUS_CONN_LOST); + cs->CloseConnection(NETWORK_RECV_STATUS_CLIENT_QUIT); } ServerNetworkGameSocketHandler::CloseListeners(); ServerNetworkAdminSocketHandler::CloseListeners(); } else if (MyClient::my_client != nullptr) { MyClient::SendQuit(); - MyClient::my_client->CloseConnection(NETWORK_RECV_STATUS_CONN_LOST); + MyClient::my_client->CloseConnection(NETWORK_RECV_STATUS_CLIENT_QUIT); } TCPConnecter::KillAll(); diff --git a/src/network/network_client.cpp b/src/network/network_client.cpp index f827c5735..501449705 100644 --- a/src/network/network_client.cpp +++ b/src/network/network_client.cpp @@ -656,7 +656,7 @@ NetworkRecvStatus ClientNetworkGameSocketHandler::Receive_SERVER_CLIENT_INFO(Pac p->Recv_string(name, sizeof(name)); if (this->status < STATUS_AUTHORIZED) return NETWORK_RECV_STATUS_MALFORMED_PACKET; - if (this->HasClientQuit()) return NETWORK_RECV_STATUS_CONN_LOST; + if (this->HasClientQuit()) return NETWORK_RECV_STATUS_CLIENT_QUIT; /* The server validates the name when receiving it from clients, so when it is wrong * here something went really wrong. In the best case the packet got malformed on its * way too us, in the worst case the server is broken or compromised. */ diff --git a/src/network/network_server.cpp b/src/network/network_server.cpp index e8266a84f..eccf80f78 100644 --- a/src/network/network_server.cpp +++ b/src/network/network_server.cpp @@ -256,7 +256,7 @@ NetworkRecvStatus ServerNetworkGameSocketHandler::CloseConnection(NetworkRecvSta */ if (this->sock == INVALID_SOCKET) return status; - if (status != NETWORK_RECV_STATUS_CONN_LOST && status != NETWORK_RECV_STATUS_SERVER_ERROR && !this->HasClientQuit() && this->status >= STATUS_AUTHORIZED) { + if (status != NETWORK_RECV_STATUS_CLIENT_QUIT && status != NETWORK_RECV_STATUS_SERVER_ERROR && !this->HasClientQuit() && this->status >= STATUS_AUTHORIZED) { /* We did not receive a leave message from this client... */ char client_name[NETWORK_CLIENT_NAME_LENGTH]; @@ -893,7 +893,7 @@ NetworkRecvStatus ServerNetworkGameSocketHandler::Receive_CLIENT_JOIN(Packet *p) p->Recv_string(name, sizeof(name)); playas = (Owner)p->Recv_uint8(); - if (this->HasClientQuit()) return NETWORK_RECV_STATUS_CONN_LOST; + if (this->HasClientQuit()) return NETWORK_RECV_STATUS_CLIENT_QUIT; /* join another company does not affect these values */ switch (playas) { @@ -1077,7 +1077,7 @@ NetworkRecvStatus ServerNetworkGameSocketHandler::Receive_CLIENT_COMMAND(Packet CommandPacket cp; const char *err = this->ReceiveCommand(p, &cp); - if (this->HasClientQuit()) return NETWORK_RECV_STATUS_CONN_LOST; + if (this->HasClientQuit()) return NETWORK_RECV_STATUS_CLIENT_QUIT; NetworkClientInfo *ci = this->GetInfo(); @@ -1136,7 +1136,7 @@ NetworkRecvStatus ServerNetworkGameSocketHandler::Receive_CLIENT_ERROR(Packet *p /* The client was never joined.. thank the client for the packet, but ignore it */ if (this->status < STATUS_DONE_MAP || this->HasClientQuit()) { - return this->CloseConnection(NETWORK_RECV_STATUS_CONN_LOST); + return this->CloseConnection(NETWORK_RECV_STATUS_CLIENT_QUIT); } this->GetClientName(client_name, lastof(client_name)); @@ -1156,7 +1156,7 @@ NetworkRecvStatus ServerNetworkGameSocketHandler::Receive_CLIENT_ERROR(Packet *p NetworkAdminClientError(this->client_id, errorno); - return this->CloseConnection(NETWORK_RECV_STATUS_CONN_LOST); + return this->CloseConnection(NETWORK_RECV_STATUS_CLIENT_QUIT); } NetworkRecvStatus ServerNetworkGameSocketHandler::Receive_CLIENT_QUIT(Packet *p) @@ -1167,7 +1167,7 @@ NetworkRecvStatus ServerNetworkGameSocketHandler::Receive_CLIENT_QUIT(Packet *p) /* The client was never joined.. thank the client for the packet, but ignore it */ if (this->status < STATUS_DONE_MAP || this->HasClientQuit()) { - return this->CloseConnection(NETWORK_RECV_STATUS_CONN_LOST); + return this->CloseConnection(NETWORK_RECV_STATUS_CLIENT_QUIT); } this->GetClientName(client_name, lastof(client_name)); @@ -1182,7 +1182,7 @@ NetworkRecvStatus ServerNetworkGameSocketHandler::Receive_CLIENT_QUIT(Packet *p) NetworkAdminClientQuit(this->client_id); - return this->CloseConnection(NETWORK_RECV_STATUS_CONN_LOST); + return this->CloseConnection(NETWORK_RECV_STATUS_CLIENT_QUIT); } NetworkRecvStatus ServerNetworkGameSocketHandler::Receive_CLIENT_ACK(Packet *p) @@ -1412,7 +1412,7 @@ NetworkRecvStatus ServerNetworkGameSocketHandler::Receive_CLIENT_SET_NAME(Packet p->Recv_string(client_name, sizeof(client_name)); ci = this->GetInfo(); - if (this->HasClientQuit()) return NETWORK_RECV_STATUS_CONN_LOST; + if (this->HasClientQuit()) return NETWORK_RECV_STATUS_CLIENT_QUIT; if (ci != nullptr) { if (!NetworkIsValidClientName(client_name)) { -- cgit v1.2.3-54-g00ecf