From 508d8a6f008e3323da73c03035ab4c9b7df5e996 Mon Sep 17 00:00:00 2001 From: Jocke Date: Thu, 25 Feb 2016 14:24:38 +0100 Subject: [PATCH] Fixed memory leak in TCPServer::AcceptNewConnections caused by acceptor->async_accept(). Fixed some formating and comments. --- include/Engine/Network/TCPServer.h | 5 ++-- src/Engine/Network/Client.cpp | 24 +++++++++------- src/Engine/Network/Server.cpp | 4 +-- src/Engine/Network/TCPServer.cpp | 45 +++++++++++++----------------- 4 files changed, 37 insertions(+), 41 deletions(-) diff --git a/include/Engine/Network/TCPServer.h b/include/Engine/Network/TCPServer.h index 9cc7646a..61184470 100644 --- a/include/Engine/Network/TCPServer.h +++ b/include/Engine/Network/TCPServer.h @@ -22,10 +22,9 @@ private: std::unique_ptr acceptor; boost::shared_ptr lastReceivedSocket; - void handle_accept(boost::shared_ptr socket, - int& nextPlayerID, std::map& connectedPlayers, - const boost::system::error_code& error); int readBuffer(char* data, PlayerDefinition& playerDefinition); + PlayerID getPlayerIDFromEndpoint(const std::map& connectedPlayers, + boost::asio::ip::address address, unsigned short port); }; #endif \ No newline at end of file diff --git a/src/Engine/Network/Client.cpp b/src/Engine/Network/Client.cpp index ad72571c..c11f86f5 100644 --- a/src/Engine/Network/Client.cpp +++ b/src/Engine/Network/Client.cpp @@ -1,7 +1,7 @@ #include "Network/Client.h" using namespace boost::asio::ip; -Client::Client(World* world, EventBroker* eventBroker) +Client::Client(World* world, EventBroker* eventBroker) : Network(world, eventBroker) { // Asumes root node is EntityID_Invalid @@ -194,12 +194,12 @@ void Client::parseSpawnEvents() } e.Player = EntityWrapper(m_World, m_ServerIDToClientID.at(m_PlayerSpawnEvents.at(i).Player.ID)); //e.Spawner = EntityWrapper(m_World, m_ServerIDToClientID.at(m_PlayerSpawnEvents.at(i).Spawner.ID)); - e.PlayerID = -1; + e.PlayerID = -1; e.PlayerName = m_PlayerSpawnEvents.at(i).PlayerName; m_EventBroker->Publish(e); } m_PlayerSpawnEvents = tempSpawn; - // m_PlayerSpawnEvents.clear(); + // m_PlayerSpawnEvents.clear(); } void Client::parsePlayersSpawned(Packet& packet) @@ -243,14 +243,14 @@ void Client::parseComponentDeletion(Packet & packet) } void Client::parseDoubleJump(Packet & packet) -{ +{ EntityID serverID = packet.ReadPrimitive(); if (!serverClientMapsHasEntity(serverID)) { return; } Events::DoubleJump e; e.entityID = m_ServerIDToClientID.at(serverID); - // If player is local player to publish to prevent infinite feedback loop + // If player is local player do not publish to prevent infinite feedback loop if (e.entityID != m_LocalPlayer.ID) { m_EventBroker->Publish(e); } @@ -330,9 +330,10 @@ void Client::parseSnapshot(Packet& packet) if (serverClientMapsHasEntity(serverEntityID)) { EntityID localEntityID = m_ServerIDToClientID.at(serverEntityID); EntityWrapper localEntity(m_World, localEntityID); - + // Update entity if (m_World->HasComponent(localEntityID, componentType)) { + // TODO Fix memory leak here SharedComponentWrapper newComponent = createSharedComponent(packet, localEntityID, componentInfo); bool shouldApply = true; // Apply potential filter function @@ -343,6 +344,7 @@ void Client::parseSnapshot(Packet& packet) ComponentWrapper currentComponent = m_World->GetComponent(localEntityID, componentType); memcpy(currentComponent.Data, newComponent.Data, componentInfo.Stride); } + //if (localEntity != m_LocalPlayer && !localEntity.IsChildOf(m_LocalPlayer)) { // updateFields(packet, componentInfo, localEntityID); //} else { @@ -371,7 +373,9 @@ void Client::parseSnapshot(Packet& packet) // This should be enough beacause we know that the entities arives in pre-order (there will always be a parent) if (serverParentID != EntityID_Invalid) { EntityID localEntityID = m_ServerIDToClientID.at(serverEntityID); - m_World->SetParent(localEntityID, m_ServerIDToClientID.at(serverParentID)); + if (m_World->GetParent(localEntityID) != m_ServerIDToClientID.at(serverParentID)) { + m_World->SetParent(localEntityID, m_ServerIDToClientID.at(serverParentID)); + } } } parseSpawnEvents(); @@ -461,7 +465,7 @@ void Client::parsePlayerDamage(Packet& packet) Events::PlayerDamage e; PlayerID victimID = packet.ReadPrimitive(); PlayerID inflictorID = packet.ReadPrimitive(); - if(!serverClientMapsHasEntity(victimID) || !serverClientMapsHasEntity(inflictorID)){ + if (!serverClientMapsHasEntity(victimID) || !serverClientMapsHasEntity(inflictorID)) { return; } e.Inflictor = EntityWrapper(m_World, m_ServerIDToClientID.at(victimID)); @@ -501,7 +505,7 @@ void Client::sendLocalPlayerTransform() packet.WritePrimitive(orientation.x); packet.WritePrimitive(orientation.y); packet.WritePrimitive(orientation.z); - + bool hasAssaultWeapon = m_LocalPlayer.HasComponent("AssaultWeapon"); packet.WritePrimitive(hasAssaultWeapon); if (hasAssaultWeapon) { @@ -509,7 +513,7 @@ void Client::sendLocalPlayerTransform() packet.WritePrimitive((int)cAssaultWeapon["MagazineAmmo"]); packet.WritePrimitive((int)cAssaultWeapon["Ammo"]); } - + m_Unreliable.Send(packet); } diff --git a/src/Engine/Network/Server.cpp b/src/Engine/Network/Server.cpp index 1a063067..547e1641 100644 --- a/src/Engine/Network/Server.cpp +++ b/src/Engine/Network/Server.cpp @@ -28,9 +28,8 @@ Server::~Server() void Server::Update() { - PlayerDefinition pd; - m_Reliable.AcceptNewConnections(m_NextPlayerID, m_ConnectedPlayers); + for (auto& kv : m_ConnectedPlayers) { while (kv.second.TCPSocket->available()) { // Packet will get real data in receive @@ -46,6 +45,7 @@ void Server::Update() } } + PlayerDefinition pd; while (m_Unreliable.IsSocketAvailable()) { // Packet will get real data in receive Packet packet(MessageType::Invalid); diff --git a/src/Engine/Network/TCPServer.cpp b/src/Engine/Network/TCPServer.cpp index a449e684..6fdc28ce 100644 --- a/src/Engine/Network/TCPServer.cpp +++ b/src/Engine/Network/TCPServer.cpp @@ -4,22 +4,33 @@ using namespace boost::asio::ip; TCPServer::TCPServer() { acceptor = std::unique_ptr(new tcp::acceptor(m_IOService, tcp::endpoint(tcp::v4(), 27666))); + // Make the acceptor non-blocking so we wont get stuck in AcceptNewConnections(). + acceptor->non_blocking(true); } TCPServer::~TCPServer() -{ -} +{ } void TCPServer::AcceptNewConnections(int& nextPlayerID, std::map& connectedPlayers) { + boost::system::error_code error; boost::shared_ptr newSocket = boost::shared_ptr(new tcp::socket(m_IOService)); - m_IOService.poll(); - acceptor->async_accept(*newSocket, - boost::bind(&TCPServer::handle_accept, this, newSocket, boost::ref(nextPlayerID), boost::ref(connectedPlayers), - boost::asio::placeholders::error)); + acceptor->accept(*newSocket, error); + // If no error occured add new tcp connection + if (!error) { + // Add tcp socket to connections + boost::asio::ip::tcp::no_delay option(true); + newSocket->set_option(option); + PlayerDefinition pd; + pd.StopTime = std::clock(); + pd.TCPSocket = newSocket; + pd.TCPAddress = newSocket.get()->remote_endpoint().address(); + pd.TCPPort = newSocket.get()->remote_endpoint().port(); + connectedPlayers[nextPlayerID++] = pd; + } } -PlayerID GetPlayerIDFromEndpoint(const std::map& connectedPlayers, +PlayerID TCPServer::getPlayerIDFromEndpoint(const std::map& connectedPlayers, boost::asio::ip::address address, unsigned short port) { for (auto& kv : connectedPlayers) { @@ -31,24 +42,6 @@ PlayerID GetPlayerIDFromEndpoint(const std::map& con return -1; } -void TCPServer::handle_accept(boost::shared_ptr socket, - int& nextPlayerID, std::map& connectedPlayers, - const boost::system::error_code& error) -{ - if (!error && GetPlayerIDFromEndpoint(connectedPlayers, socket->remote_endpoint().address(), - socket->remote_endpoint().port()) == -1) { - // Add tcp socket to connections - boost::asio::ip::tcp::no_delay option(true); - socket->set_option(option); - PlayerDefinition pd; - pd.StopTime = std::clock(); - pd.TCPSocket = socket; - pd.TCPAddress = socket.get()->remote_endpoint().address(); - pd.TCPPort = socket.get()->remote_endpoint().port(); - connectedPlayers[nextPlayerID++] = pd; - } -} - void TCPServer::Send(Packet & packet, PlayerDefinition & playerDefinition) { try { @@ -73,7 +66,7 @@ void TCPServer::Send(Packet & packet) } void TCPServer::Disconnect() -{ +{ }