From 40c6ee4cd9c71231f6967375811a6921e5ec8980 Mon Sep 17 00:00:00 2001 From: Yousef Khadadeh Date: Thu, 10 Jul 2025 15:52:01 +0300 Subject: [PATCH] Fixed segfault when some clients disconnect from server --- include/Helpers/Networking/NetServer.cpp | 30 +++++++++++------- include/Helpers/Networking/NetServer.hpp | 28 +++++++++-------- src/Server/MainServer.cpp | 40 ++++++++++++------------ 3 files changed, 53 insertions(+), 45 deletions(-) diff --git a/include/Helpers/Networking/NetServer.cpp b/include/Helpers/Networking/NetServer.cpp index 5c022a9..2c7987f 100644 --- a/include/Helpers/Networking/NetServer.cpp +++ b/include/Helpers/Networking/NetServer.cpp @@ -14,18 +14,24 @@ NetServer::NetServer(const std::string& serverLocalID) { void NetServer::update() { if(isDisconnected) return; - std::erase_if(clients, [&](auto& client) { - bool timedOut = std::chrono::steady_clock::now() - client->lastMessageTime > NetLibrary::TIMEOUT_DURATION; - if(client->setToDisconnect || timedOut) { - if(disconnectCallback) - disconnectCallback(*client); - if(timedOut) + for(auto& client : clients) { + if(client) { + bool timedOut = std::chrono::steady_clock::now() - client->lastMessageTime > NetLibrary::TIMEOUT_DURATION; + if(timedOut) { + client->setToDisconnect = true; Logger::get().log("INFO", "Connection timed out"); - return true; + } + if(client->setToDisconnect) + disconnectCallback(client); + else { + client->parse_received_messages(*this); + client->send_queued_messages(*this); + } } - client->parse_received_messages(*this); - client->send_queued_messages(*this); - return false; + } + + std::erase_if(clients, [&](auto& client) { + return !client || client->setToDisconnect.load(); }); std::scoped_lock clientAddLock(clientsToAddMutex); @@ -171,7 +177,7 @@ void NetServer::ClientData::parse_received_messages(NetServer& server) { cereal::PortableBinaryInputArchive completeArchive(completeStrm); MessageCommandType completeCommandID; completeArchive(completeCommandID); - server.recvCallbacks[completeCommandID](*this, completeArchive); + server.recvCallbacks[completeCommandID](shared_from_this(), completeArchive); spfm.partialFragmentMessage.clear(); spfm.partialFragmentMessageLoc = 0; } @@ -181,7 +187,7 @@ void NetServer::ClientData::parse_received_messages(NetServer& server) { } } else - server.recvCallbacks[commandID](*this, inArchive); + server.recvCallbacks[commandID](shared_from_this(), inArchive); receivedMessages.pop(); } diff --git a/include/Helpers/Networking/NetServer.hpp b/include/Helpers/Networking/NetServer.hpp index 2546d88..ca268e5 100644 --- a/include/Helpers/Networking/NetServer.hpp +++ b/include/Helpers/Networking/NetServer.hpp @@ -12,10 +12,10 @@ class NetServer : public std::enable_shared_from_this { public: struct ClientData; - typedef std::function NetServerRecvCallback; - typedef std::function NetServerConnectCallback; - typedef std::function NetServerDisconnectCallback; - struct ClientData { + typedef std::function, cereal::PortableBinaryInputArchive&)> NetServerRecvCallback; + typedef std::function)> NetServerConnectCallback; + typedef std::function)> NetServerDisconnectCallback; + struct ClientData : public std::enable_shared_from_this { std::unordered_map>> messages; struct PartialFragmentMessage { std::string partialFragmentMessage; @@ -53,7 +53,10 @@ class NetServer : public std::enable_shared_from_this { NetServer(const std::string& serverLocalID); ~NetServer(); void update(); - template void send_items_to_client(ClientData& client, const std::string& channel, Args&&... items) { + template void send_items_to_client(std::shared_ptr client, const std::string& channel, Args&&... items) { + if(!client) + return; + auto ss(std::make_shared()); { cereal::PortableBinaryOutputArchive m(*ss); @@ -61,7 +64,7 @@ class NetServer : public std::enable_shared_from_this { } std::vector> fragmentedMessage = fragment_message(ss->view(), NetLibrary::FRAGMENT_MESSAGE_STRIDE); - auto& messageQueue = client.messageQueues[channel]; + auto& messageQueue = client->messageQueues[channel]; if(fragmentedMessage.empty()) messageQueue.emplace(ss); else if(channel != UNRELIABLE_COMMAND_CHANNEL) { // Just drop unreliable messages that are fragmented @@ -70,27 +73,26 @@ class NetServer : public std::enable_shared_from_this { } } template void send_items_to_all_clients(const std::string& channel, Args&&... items) { - send_items_to_client_if([&](const ClientData& c) { + send_items_to_client_if([&](std::shared_ptr c) { return true; }, channel, items...); } - template void send_items_to_all_clients_except(ClientData& client, const std::string& channel, Args&&... items) { - send_items_to_client_if([&](const ClientData& c) { - return (&c) != (&client); + template void send_items_to_all_clients_except(std::shared_ptr client, const std::string& channel, Args&&... items) { + send_items_to_client_if([&](std::shared_ptr c) { + return c != client; }, channel, items...); } - template void send_items_to_client_if(std::function clientChecker, const std::string& channel, Args&&... items) { + template void send_items_to_client_if(std::function)> clientChecker, const std::string& channel, Args&&... items) { auto ss(std::make_shared()); { cereal::PortableBinaryOutputArchive m(*ss); (m(items), ...); } - std::vector> fragmentedMessage = fragment_message(ss->view(), NetLibrary::FRAGMENT_MESSAGE_STRIDE); for(auto& client : clients) { - if(clientChecker(*client)) { + if(client && clientChecker(client)) { auto& messageQueue = client->messageQueues[channel]; if(fragmentedMessage.empty()) messageQueue.emplace(ss); diff --git a/src/Server/MainServer.cpp b/src/Server/MainServer.cpp index 30f6909..d2acc83 100644 --- a/src/Server/MainServer.cpp +++ b/src/Server/MainServer.cpp @@ -70,7 +70,7 @@ MainServer::MainServer(World& initWorld, const std::string& serverLocalID): netServer = std::make_shared(serverLocalID); NetLibrary::register_server(netServer); - netServer->add_recv_callback(SERVER_INITIAL_DATA, [&](NetServer::ClientData& client, cereal::PortableBinaryInputArchive& message) { + netServer->add_recv_callback(SERVER_INITIAL_DATA, [&](std::shared_ptr client, cereal::PortableBinaryInputArchive& message) { bool isDirectConnect; ClientData newClient; Vector3f hsv; @@ -85,7 +85,7 @@ MainServer::MainServer(World& initWorld, const std::string& serverLocalID): fileDisplayName = newClient.displayName; newClient.id.first = Random::get().int_range(1, std::numeric_limits::max()); - client.customID = newClient.id.first; + client->customID = newClient.id.first; newClient.id.second = data.get_max_id(newClient.id.first); netServer->send_items_to_all_clients_except(client, RELIABLE_COMMAND_CHANNEL, CLIENT_USER_CONNECT, newClient.id.first, newClient.displayName, newClient.cursorColor); if(isDirectConnect) @@ -97,29 +97,29 @@ MainServer::MainServer(World& initWorld, const std::string& serverLocalID): } clients.emplace(newClient.id.first, newClient); }); - netServer->add_recv_callback(SERVER_MOVE_MOUSE, [&](NetServer::ClientData& client, cereal::PortableBinaryInputArchive& message) { - auto& c = clients[client.customID]; + netServer->add_recv_callback(SERVER_MOVE_MOUSE, [&](std::shared_ptr client, cereal::PortableBinaryInputArchive& message) { + auto& c = clients[client->customID]; message(c.camCoords, c.windowSize, c.cursorPos); netServer->send_items_to_all_clients_except(client, UNRELIABLE_COMMAND_CHANNEL, CLIENT_MOVE_MOUSE, c.id.first, c.camCoords, c.windowSize, c.cursorPos); }); - netServer->add_recv_callback(SERVER_PLACE_COMPONENT, [&](NetServer::ClientData& client, cereal::PortableBinaryInputArchive& message) { + netServer->add_recv_callback(SERVER_PLACE_COMPONENT, [&](std::shared_ptr client, cereal::PortableBinaryInputArchive& message) { uint64_t placement; DrawComponentType type; message(placement, type); auto newComp = DrawComponent::allocate_comp_type(type); message(newComp->coords, *newComp); - ServerClientID id = clients[client.customID].get_next_id(); + ServerClientID id = clients[client->customID].get_next_id(); placement = std::min(placement, data.components.size()); data.components.insert(data.components.begin() + placement, {id, newComp}); newComp->server_send_place(*this, id, placement); }); - netServer->add_recv_callback(SERVER_ERASE_COMPONENT, [&](NetServer::ClientData& client, cereal::PortableBinaryInputArchive& message) { + netServer->add_recv_callback(SERVER_ERASE_COMPONENT, [&](std::shared_ptr client, cereal::PortableBinaryInputArchive& message) { ServerClientID compToRemove; message(compToRemove); std::erase_if(data.components, [&](auto& c){ return c.first == compToRemove; }); DrawComponent::server_send_erase(*this, compToRemove); }); - netServer->add_recv_callback(SERVER_TRANSFORM_COMPONENT, [&](NetServer::ClientData& client, cereal::PortableBinaryInputArchive& message) { + netServer->add_recv_callback(SERVER_TRANSFORM_COMPONENT, [&](std::shared_ptr client, cereal::PortableBinaryInputArchive& message) { bool isTemp; ServerClientID idToTransform; message(isTemp, idToTransform); @@ -135,7 +135,7 @@ MainServer::MainServer(World& initWorld, const std::string& serverLocalID): comp->server_send_transform_final(*this, idToTransform); } }); - netServer->add_recv_callback(SERVER_UPDATE_COMPONENT, [&](NetServer::ClientData& client, cereal::PortableBinaryInputArchive& message) { + netServer->add_recv_callback(SERVER_UPDATE_COMPONENT, [&](std::shared_ptr client, cereal::PortableBinaryInputArchive& message) { bool isTemp; ServerClientID idToTransform; message(isTemp, idToTransform); @@ -151,8 +151,8 @@ MainServer::MainServer(World& initWorld, const std::string& serverLocalID): comp->server_send_update_final(*this, idToTransform); } }); - netServer->add_recv_callback(SERVER_RESOURCE_INIT, [&](NetServer::ClientData& client, cereal::PortableBinaryInputArchive& message) { - auto& c = clients[client.customID]; + netServer->add_recv_callback(SERVER_RESOURCE_INIT, [&](std::shared_ptr client, cereal::PortableBinaryInputArchive& message) { + auto& c = clients[client->customID]; ServerClientID newID = c.get_next_id(); if(c.pendingResourceData.empty()) c.pendingResourceID.emplace(newID); @@ -162,10 +162,10 @@ MainServer::MainServer(World& initWorld, const std::string& serverLocalID): c.pendingResourceData.pop(); } }); - netServer->add_recv_callback(SERVER_RESOURCE_DATA, [&](NetServer::ClientData& client, cereal::PortableBinaryInputArchive& message) { + netServer->add_recv_callback(SERVER_RESOURCE_DATA, [&](std::shared_ptr client, cereal::PortableBinaryInputArchive& message) { ResourceData resourceData; message(resourceData); - auto& c = clients[client.customID]; + auto& c = clients[client->customID]; if(c.pendingResourceID.empty()) { c.pendingResourceData.emplace(); c.pendingResourceData.back() = std::move(resourceData); @@ -176,31 +176,31 @@ MainServer::MainServer(World& initWorld, const std::string& serverLocalID): c.pendingResourceID.pop(); } }); - netServer->add_recv_callback(SERVER_CHAT_MESSAGE, [&](NetServer::ClientData& client, cereal::PortableBinaryInputArchive& message) { + netServer->add_recv_callback(SERVER_CHAT_MESSAGE, [&](std::shared_ptr client, cereal::PortableBinaryInputArchive& message) { std::string chatMessage; Vector4f c{1.0f, 1.0f, 1.0f, 1.0f}; message(chatMessage); netServer->send_items_to_all_clients_except(client, RELIABLE_COMMAND_CHANNEL, CLIENT_CHAT_MESSAGE, chatMessage, c); }); - netServer->add_recv_callback(SERVER_NEW_BOOKMARK, [&](NetServer::ClientData& client, cereal::PortableBinaryInputArchive& message) { + netServer->add_recv_callback(SERVER_NEW_BOOKMARK, [&](std::shared_ptr client, cereal::PortableBinaryInputArchive& message) { std::string name; message(name); Bookmark& b = data.bookmarks[name]; message(b); netServer->send_items_to_all_clients(RELIABLE_COMMAND_CHANNEL, CLIENT_NEW_BOOKMARK, name, b); }); - netServer->add_recv_callback(SERVER_REMOVE_BOOKMARK, [&](NetServer::ClientData& client, cereal::PortableBinaryInputArchive& message) { + netServer->add_recv_callback(SERVER_REMOVE_BOOKMARK, [&](std::shared_ptr client, cereal::PortableBinaryInputArchive& message) { std::string name; message(name); data.bookmarks.erase(name); netServer->send_items_to_all_clients(RELIABLE_COMMAND_CHANNEL, CLIENT_REMOVE_BOOKMARK, name); }); - netServer->add_recv_callback(SERVER_KEEP_ALIVE, [&](NetServer::ClientData& client, cereal::PortableBinaryInputArchive& message) { + netServer->add_recv_callback(SERVER_KEEP_ALIVE, [&](std::shared_ptr client, cereal::PortableBinaryInputArchive& message) { }); - netServer->add_disconnect_callback([&](NetServer::ClientData& client) { - auto it = clients.find(client.customID); + netServer->add_disconnect_callback([&](std::shared_ptr client) { + auto it = clients.find(client->customID); if(it != clients.end()) { - netServer->send_items_to_all_clients_except(client, RELIABLE_COMMAND_CHANNEL, CLIENT_USER_DISCONNECT, client.customID); + netServer->send_items_to_all_clients_except(client, RELIABLE_COMMAND_CHANNEL, CLIENT_USER_DISCONNECT, client->customID); clients.erase(it); } });