Made access to the public address thread safe, and simplified setting

of the public address.

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)

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 <vector>
 
@@ -98,12 +99,30 @@ class NetworkManager : public AbstractSingleton<NetworkManager>
         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<STKPeer*> 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<NetworkManager>
         STKHost* m_localhost;
         bool m_playing_online;
         GameSetup* m_game_setup;
-
+        /** This computer's public IP address. With lock since it can
-        TransportAddress m_public_address;
+         *  be updated from a separate thread. */
+        Synchronised<TransportAddress> m_public_address;
         PlayerLogin m_player_login;
 };
 

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())
+                if (m_server_address.getIP() == 
-                    Log::info("ConnectToServer", "Server appears to be in the same LAN.");
+                    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();

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;

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)
+GetPublicAddress::GetPublicAddress()
-                : Protocol(callback_object, PROTOCOL_SILENT)
+                : 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));
+            TransportAddress address(datas.getUInt32(pos + 8), 
-            address.setIP(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<TransportAddress*>(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);

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

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.");

src/network/protocols/server_lobby_room_protocol.hpp

@@ -37,7 +37,6 @@ class ServerLobbyRoomProtocol : public LobbyRoomProtocol
         std::vector<TransportAddress> m_peers;
         std::vector<uint32_t> m_incoming_peers_ids;
         uint32_t m_current_protocol_id;
-        TransportAddress m_public_address;
         bool m_selection_enabled;
         bool m_in_race;
 

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;