From faf697ef5d21962e4df53c27425cbe65382db545 Mon Sep 17 00:00:00 2001 From: hiker Date: Mon, 19 Oct 2015 08:02:44 +1100 Subject: [PATCH] Made access to the public address thread safe, and simplified setting of the public address. --- src/network/network_manager.cpp | 13 +++++++--- src/network/network_manager.hpp | 26 ++++++++++++++++--- src/network/protocols/connect_to_server.cpp | 16 +++++++----- src/network/protocols/connect_to_server.hpp | 1 - src/network/protocols/get_public_address.cpp | 22 +++++++--------- src/network/protocols/get_public_address.hpp | 2 +- .../protocols/server_lobby_room_protocol.cpp | 4 +-- .../protocols/server_lobby_room_protocol.hpp | 1 - src/network/types.hpp | 13 +++++++++- 9 files changed, 65 insertions(+), 33 deletions(-) diff --git a/src/network/network_manager.cpp b/src/network/network_manager.cpp index 015a8a497..51d403795 100644 --- a/src/network/network_manager.cpp +++ b/src/network/network_manager.cpp @@ -35,10 +35,12 @@ NetworkManager::NetworkManager() { - m_public_address.clear(); + m_public_address.lock(); + m_public_address.getData().clear(); + m_public_address.unlock(); m_localhost = NULL; m_game_setup = NULL; -} +} // NetworkManager //----------------------------------------------------------------------------- @@ -195,8 +197,11 @@ void NetworkManager::setLogin(std::string username, std::string password) void NetworkManager::setPublicAddress(const TransportAddress& addr) { - m_public_address.copy(addr); -} + m_public_address.lock(); + m_public_address.getData().copy(addr); + m_public_address.unlock(); +} // setPublicAddress + //----------------------------------------------------------------------------- void NetworkManager::removePeer(STKPeer* peer) diff --git a/src/network/network_manager.hpp b/src/network/network_manager.hpp index c943b8630..9424a6af7 100644 --- a/src/network/network_manager.hpp +++ b/src/network/network_manager.hpp @@ -31,6 +31,7 @@ #include "network/event.hpp" #include "network/game_setup.hpp" #include "utils/singleton.hpp" +#include "utils/synchronised.hpp" #include @@ -98,12 +99,30 @@ class NetworkManager : public AbstractSingleton virtual bool isConnectedTo(const TransportAddress& peer); virtual bool isServer() = 0; + // -------------------------------------------------------------------- inline bool isClient() { return !isServer(); } + // -------------------------------------------------------------------- bool isPlayingOnline() { return m_playing_online; } + // -------------------------------------------------------------------- STKHost* getHost() { return m_localhost; } + // -------------------------------------------------------------------- std::vector getPeers() { return m_peers; } + // -------------------------------------------------------------------- unsigned int getPeerCount() { return (int)m_peers.size(); } - const TransportAddress& getPublicAddress() { return m_public_address; } + // -------------------------------------------------------------------- + /** Returns the public IP address (thread safe). The network manager + * is a friend of TransportAddress and so has access to the copy + * constructor, which is otherwise declared private. */ + const TransportAddress getPublicAddress() + { + m_public_address.lock(); + TransportAddress a; + a.copy(m_public_address.getData()); + m_public_address.unlock(); + return a; + } // getPublicAddress + + // -------------------------------------------------------------------- GameSetup* getGameSetup() { return m_game_setup; } protected: @@ -115,8 +134,9 @@ class NetworkManager : public AbstractSingleton STKHost* m_localhost; bool m_playing_online; GameSetup* m_game_setup; - - TransportAddress m_public_address; + /** This computer's public IP address. With lock since it can + * be updated from a separate thread. */ + Synchronised m_public_address; PlayerLogin m_player_login; }; diff --git a/src/network/protocols/connect_to_server.cpp b/src/network/protocols/connect_to_server.cpp index 75a401ee8..27b0deb9a 100644 --- a/src/network/protocols/connect_to_server.cpp +++ b/src/network/protocols/connect_to_server.cpp @@ -82,7 +82,6 @@ void ConnectToServer::setup() { Log::info("ConnectToServer", "SETUPP"); m_state = NONE; - m_public_address.clear(); m_server_address.clear(); m_current_protocol_id = 0; } @@ -96,7 +95,7 @@ void ConnectToServer::asynchronousUpdate() case NONE: { Log::info("ConnectToServer", "Protocol starting"); - m_current_protocol_id = m_listener->requestStart(new GetPublicAddress(&m_public_address)); + m_current_protocol_id = m_listener->requestStart(new GetPublicAddress()); m_state = GETTING_SELF_ADDRESS; break; } @@ -105,7 +104,6 @@ void ConnectToServer::asynchronousUpdate() == PROTOCOL_STATE_TERMINATED) // now we know the public addr { m_state = SHOWING_SELF_ADDRESS; - NetworkManager::getInstance()->setPublicAddress(m_public_address); // set our public address m_current_protocol_id = m_listener->requestStart(new ShowPublicAddress()); Log::info("ConnectToServer", "Public address known"); /* @@ -138,10 +136,13 @@ void ConnectToServer::asynchronousUpdate() { Log::info("ConnectToServer", "Server's address known"); // we're in the same lan (same public ip address) !! - if (m_server_address.getIP() == m_public_address.getIP()) - Log::info("ConnectToServer", "Server appears to be in the same LAN."); + if (m_server_address.getIP() == + NetworkManager::getInstance()->getPublicAddress().getIP()) + Log::info("ConnectToServer", + "Server appears to be in the same LAN."); m_state = REQUESTING_CONNECTION; - m_current_protocol_id = m_listener->requestStart(new RequestConnection(m_server_id)); + m_current_protocol_id = + m_listener->requestStart(new RequestConnection(m_server_id)); } break; case REQUESTING_CONNECTION: @@ -159,7 +160,8 @@ void ConnectToServer::asynchronousUpdate() return; } // we're in the same lan (same public ip address) !! - if (m_server_address.getIP() == m_public_address.getIP()) + if (m_server_address.getIP() == + NetworkManager::getInstance()->getPublicAddress().getIP()) { // just send a broadcast packet, the client will know our ip address and will connect STKHost* host = NetworkManager::getInstance()->getHost(); diff --git a/src/network/protocols/connect_to_server.hpp b/src/network/protocols/connect_to_server.hpp index 093a21819..d41c8147e 100644 --- a/src/network/protocols/connect_to_server.hpp +++ b/src/network/protocols/connect_to_server.hpp @@ -37,7 +37,6 @@ class ConnectToServer : public Protocol, public CallbackObject protected: TransportAddress m_server_address; - TransportAddress m_public_address; uint32_t m_server_id; uint32_t m_host_id; uint32_t m_current_protocol_id; diff --git a/src/network/protocols/get_public_address.cpp b/src/network/protocols/get_public_address.cpp index 76e55710c..c033f8e69 100644 --- a/src/network/protocols/get_public_address.cpp +++ b/src/network/protocols/get_public_address.cpp @@ -45,8 +45,8 @@ // make the linker happy const uint32_t GetPublicAddress::m_stun_magic_cookie = 0x2112A442; -GetPublicAddress::GetPublicAddress(CallbackObject* callback_object) - : Protocol(callback_object, PROTOCOL_SILENT) +GetPublicAddress::GetPublicAddress() + : Protocol(NULL, PROTOCOL_SILENT) { m_state = NOTHING_DONE; } // GetPublicAddress @@ -154,9 +154,8 @@ std::string GetPublicAddress::parseStunResponse() if (message_size < 4) // cannot even read the size return "STUN response is too short."; - // Those are the port and the address to be detected - TransportAddress address; + int pos = 20; while (true) { @@ -166,8 +165,13 @@ std::string GetPublicAddress::parseStunResponse() { assert(size == 8); assert(datas.getUInt8(pos+5) == 0x01); // Family IPv4 only - address.setPort(datas.getUInt16(pos + 6)); - address.setIP(datas.getUInt32(pos + 8)); + TransportAddress address(datas.getUInt32(pos + 8), + datas.getUInt16(pos + 6)); + // finished parsing, we know our public transport address + Log::debug("GetPublicAddress", + "The public address has been found: %s", + address.toString().c_str()); + NetworkManager::getInstance()->setPublicAddress(address); break; } // type = 0 or 1 pos += 4 + size; @@ -178,12 +182,6 @@ std::string GetPublicAddress::parseStunResponse() return "STUN response is invalid."; } // while true - // finished parsing, we know our public transport address - Log::debug("GetPublicAddress", "The public address has been found: %s", - address.toString().c_str()); - TransportAddress* addr = static_cast(m_callback_object); - addr->copy(address); - // The address and the port are known, so the connection can be closed m_state = EXITING; m_listener->requestTerminate(this); diff --git a/src/network/protocols/get_public_address.hpp b/src/network/protocols/get_public_address.hpp index c2ff1b7ef..4eb92a85e 100644 --- a/src/network/protocols/get_public_address.hpp +++ b/src/network/protocols/get_public_address.hpp @@ -26,7 +26,7 @@ class GetPublicAddress : public Protocol { public: - GetPublicAddress(CallbackObject* callback_object); + GetPublicAddress(); virtual ~GetPublicAddress() {} virtual bool notifyEvent(Event* event) { return true; } diff --git a/src/network/protocols/server_lobby_room_protocol.cpp b/src/network/protocols/server_lobby_room_protocol.cpp index 16448ee49..6a58cd18b 100644 --- a/src/network/protocols/server_lobby_room_protocol.cpp +++ b/src/network/protocols/server_lobby_room_protocol.cpp @@ -53,7 +53,6 @@ void ServerLobbyRoomProtocol::setup() m_setup->getRaceConfig()->setPlayerCount(16); //FIXME : this has to be moved to when logging into the server m_next_id = 0; m_state = NONE; - m_public_address.clear(); m_selection_enabled = false; m_in_race = false; Log::info("ServerLobbyRoomProtocol", "Starting the protocol."); @@ -106,13 +105,12 @@ void ServerLobbyRoomProtocol::update() switch (m_state) { case NONE: - m_current_protocol_id = m_listener->requestStart(new GetPublicAddress(&m_public_address)); + m_current_protocol_id = m_listener->requestStart(new GetPublicAddress()); m_state = GETTING_PUBLIC_ADDRESS; break; case GETTING_PUBLIC_ADDRESS: if (m_listener->getProtocolState(m_current_protocol_id) == PROTOCOL_STATE_TERMINATED) { - NetworkManager::getInstance()->setPublicAddress(m_public_address); m_current_protocol_id = m_listener->requestStart(new StartServer()); m_state = LAUNCHING_SERVER; Log::debug("ServerLobbyRoomProtocol", "Public address known."); diff --git a/src/network/protocols/server_lobby_room_protocol.hpp b/src/network/protocols/server_lobby_room_protocol.hpp index 82d2691f0..22788afc2 100644 --- a/src/network/protocols/server_lobby_room_protocol.hpp +++ b/src/network/protocols/server_lobby_room_protocol.hpp @@ -37,7 +37,6 @@ class ServerLobbyRoomProtocol : public LobbyRoomProtocol std::vector m_peers; std::vector m_incoming_peers_ids; uint32_t m_current_protocol_id; - TransportAddress m_public_address; bool m_selection_enabled; bool m_in_race; diff --git a/src/network/types.hpp b/src/network/types.hpp index 3ed6ddc2a..808895bc5 100644 --- a/src/network/types.hpp +++ b/src/network/types.hpp @@ -72,8 +72,19 @@ public: // ------------------------------------------------------------------------ ~TransportAddress() {} // ------------------------------------------------------------------------ +private: + friend class NetworkManager; + /** The copy constructor is private, so that the friend class + * NetworkManager can access it to create a copy, but no other + * class can. */ + TransportAddress(const TransportAddress &other) + { + copy(other); + } // TransportAddress(const TransportAddress&) +public: + // ------------------------------------------------------------------------ /** A copy function (to replace the copy constructor which is disabled - * using NoCopy). */ + * using NoCopy): it copies the data from the argument into this object.*/ void copy(const TransportAddress &other) { m_ip = other.m_ip;