From bc17e212bb6161cec83e57f6bd4f46f887f0d310 Mon Sep 17 00:00:00 2001 From: phosit Date: Tue, 3 Mar 2026 19:46:40 +0100 Subject: [PATCH] Launch session at construction of Net* This way it's statically assured that the session aren't launched multiple times. --- source/network/NetClient.cpp | 22 +++- source/network/NetClient.h | 16 +-- source/network/NetServer.cpp | 102 ++++++++---------- source/network/NetServer.h | 29 ++--- .../network/scripting/JSInterface_Network.cpp | 22 +--- source/network/tests/test_Net.h | 24 ++--- 6 files changed, 86 insertions(+), 129 deletions(-) diff --git a/source/network/NetClient.cpp b/source/network/NetClient.cpp index a2cd2ece24..5f60c2c605 100644 --- a/source/network/NetClient.cpp +++ b/source/network/NetClient.cpp @@ -69,6 +69,15 @@ CNetClient *g_NetClient = NULL; CNetClient::CNetClient(CGame* game, std::string serverAddressOrHostname, std::uint16_t serverPort, const CStrW& username, const CStr& hostJID, std::string hashedPassword, std::string controllerSecret) : + CNetClient{PrivateTag{}, game, std::move(serverAddressOrHostname), serverPort, username, hostJID, + std::move(hashedPassword), std::move(controllerSecret)} +{ + SetupConnection(nullptr); +} + +CNetClient::CNetClient(PrivateTag, CGame* game, std::string serverAddressOrHostname, + std::uint16_t serverPort, const CStrW& username, const CStr& hostJID, std::string hashedPassword, + std::string controllerSecret) : m_UserName{username}, m_HostJID{hostJID}, m_Game{game}, @@ -143,7 +152,7 @@ CNetClient::CNetClient(CGame* game, std::string serverAddressOrHostname, std::ui CNetClient::CNetClient(CGame* game, const CStrW& username, const CStr& hostJID, std::string hashedPassword, IXmppClient& xmppClient) : - CNetClient{game, {}, 0, username, hostJID, std::move(hashedPassword)} + CNetClient{PrivateTag{}, game, {}, 0, username, hostJID, std::move(hashedPassword)} { xmppClient.SendIqGetConnectionData(m_HostJID, m_Password, m_UserName.ToUTF8(), false); } @@ -158,14 +167,15 @@ CNetClient::~CNetClient() } -bool CNetClient::SetupConnection(ENetHost* enetClient) +void CNetClient::SetupConnection(ENetHost* enetClient) { CNetClientSession* session = new CNetClientSession(*this); bool ok = session->Connect(m_ServerAddressOrHostname, m_ServerPort, enetClient); SetAndOwnSession(session); if (ok) m_PollingThread = std::thread(Threading::HandleExceptions::Wrapper, m_Session); - return ok; + else + throw std::runtime_error{"Failed to connect to server"}; } void CNetClient::HandleGetServerDataFailed(const CStr& error) @@ -270,7 +280,11 @@ bool CNetClient::TryToConnectWithSTUN(std::string serverAddressOrHostname, std:: StunClient::SendHolePunchingMessages(*enetClient, m_ServerAddressOrHostname, m_ServerPort); } - if (!g_NetClient->SetupConnection(enetClient)) + try + { + g_NetClient->SetupConnection(enetClient); + } + catch (...) { PushGuiMessage( "type", "netstatus", diff --git a/source/network/NetClient.h b/source/network/NetClient.h index 42e1146ac6..6eddbf1f07 100644 --- a/source/network/NetClient.h +++ b/source/network/NetClient.h @@ -100,12 +100,6 @@ public: */ CStr GetGUID() const { return m_GUID; } - /** - * Set up a connection to the remote networked server. - * @return true on success, false on connection failure - */ - bool SetupConnection(ENetHost* enetClient); - /** * Connect to the remote networked server using lobby. * Push netstatus messages on failure. @@ -259,6 +253,10 @@ public: */ void HandleGetServerDataFailed(const CStr& error); private: + struct PrivateTag {}; + CNetClient(PrivateTag, CGame* game, std::string serverAddressOrHostname, + std::uint16_t serverPont, const CStrW& username = L"anonymous", const CStr& hostJID = {}, + std::string hashedPassword = {}, std::string controllerSecret = {}); void SendAuthenticateMessage(); @@ -286,6 +284,12 @@ private: static bool OnClientPaused(CNetClient* client, CFsmEvent* event); static bool OnLoadedGame(CNetClient* client, CFsmEvent* event); + /** + * Set up a connection to the remote networked server. + * @return true on success, false on connection failure + */ + void SetupConnection(ENetHost* enetClient); + /** * Take ownership of a session object, and use it for all network communication. */ diff --git a/source/network/NetServer.cpp b/source/network/NetServer.cpp index 951d93ad2f..a2a488d099 100644 --- a/source/network/NetServer.cpp +++ b/source/network/NetServer.cpp @@ -111,29 +111,55 @@ static CStr DebugName(CNetServerSession* session) * See https://gitea.wildfiregames.com/0ad/0ad/issues/654 */ -CNetServerWorker::CNetServerWorker(const bool continueSavedGame, const bool useLobbyAuth, - std::string password, std::string controllerSecret) : +CNetServerWorker::CNetServerWorker(const bool continueSavedGame, std::uint16_t port, + const bool useLobbyAuth, std::string password, std::string controllerSecret, + std::string initAttributes) : m_ContinuesSavedGame{continueSavedGame}, m_LobbyAuth{useLobbyAuth}, m_ControllerSecret{std::move(controllerSecret)}, m_Password{std::move(password)} { + // Bind to default host + ENetAddress addr; + addr.host = ENET_HOST_ANY; + addr.port = port; + + // Create ENet server + m_Host = PS::Enet::CreateHost(&addr, MAX_CLIENTS, CHANNEL_COUNT); + if (!m_Host) + { + enet_host_destroy(m_Host); + LOGERROR("Net server: enet_host_create failed"); + throw std::runtime_error{"Failed to start server"}; + } + + m_Stats = new CNetStatsTable(); + if (CProfileViewer::IsInitialised()) + g_ProfileViewer.AddRootTable(m_Stats); + + m_State = SERVER_STATE_PREGAME; + + // Launch the worker thread + m_WorkerThread = std::thread(Threading::HandleExceptions::Wrapper, this, + std::move(initAttributes)); + +#if CONFIG2_MINIUPNPC + // Launch the UPnP thread + m_UPnPThread = std::thread(Threading::HandleExceptions::Wrapper, port); +#endif } CNetServerWorker::~CNetServerWorker() { - if (m_State != SERVER_STATE_UNCONNECTED) + // Tell the thread to shut down { - // Tell the thread to shut down - { - std::lock_guard lock(m_WorkerMutex); - m_Shutdown = true; - } - - // Wait for it to shut down cleanly - m_WorkerThread.join(); + std::lock_guard lock(m_WorkerMutex); + m_Shutdown = true; } + // Wait for it to shut down cleanly + m_WorkerThread.join(); + #if CONFIG2_MINIUPNPC if (m_UPnPThread.joinable()) m_UPnPThread.detach(); @@ -161,43 +187,6 @@ bool CNetServerWorker::CheckPassword(const std::string& password, const std::str return HashCryptographically(m_Password, salt) == password; } - -bool CNetServerWorker::SetupConnection(const u16 port, std::string initAttributes) -{ - ENSURE(m_State == SERVER_STATE_UNCONNECTED); - ENSURE(!m_Host); - - // Bind to default host - ENetAddress addr; - addr.host = ENET_HOST_ANY; - addr.port = port; - - // Create ENet server - m_Host = PS::Enet::CreateHost(&addr, MAX_CLIENTS, CHANNEL_COUNT); - if (!m_Host) - { - LOGERROR("Net server: enet_host_create failed"); - return false; - } - - m_Stats = new CNetStatsTable(); - if (CProfileViewer::IsInitialised()) - g_ProfileViewer.AddRootTable(m_Stats); - - m_State = SERVER_STATE_PREGAME; - - // Launch the worker thread - m_WorkerThread = std::thread{Threading::HandleExceptions::Wrapper, this, - std::move(initAttributes)}; - -#if CONFIG2_MINIUPNPC - // Launch the UPnP thread - m_UPnPThread = std::thread(Threading::HandleExceptions::Wrapper, port); -#endif - - return true; -} - #if CONFIG2_MINIUPNPC void CNetServerWorker::SetupUPnP(const u16 port) { @@ -742,7 +731,7 @@ void CNetServerWorker::AddPlayer(const CStr& guid, const CStrW& name) i32 playerID = -1; - if (m_State != SERVER_STATE_UNCONNECTED && m_State != SERVER_STATE_PREGAME) + if (m_State != SERVER_STATE_PREGAME) { // Try to match GUID first for (PlayerAssignmentMap::iterator it = m_PlayerAssignments.begin(); it != m_PlayerAssignments.end(); ++it) @@ -1124,7 +1113,7 @@ bool CNetServerWorker::OnAuthenticate(CNetServerSession* session, CFsmEventSetupConnection(port, std::move(initAttributes)); -} - CStr CNetServer::GetPublicIp() const { return m_PublicIp; diff --git a/source/network/NetServer.h b/source/network/NetServer.h index dd8db09d83..751d1eb86c 100644 --- a/source/network/NetServer.h +++ b/source/network/NetServer.h @@ -49,10 +49,6 @@ template class CFsmEvent; enum NetServerState { - // We haven't opened the port yet, we're just setting some stuff up. - // The worker thread has not been started. - SERVER_STATE_UNCONNECTED, - // The server is open and accepting connections. This is the screen where // rules are set up by the operator and where players join and select civs // and stuff. @@ -112,17 +108,10 @@ class CNetServer { NONCOPYABLE(CNetServer); public: - CNetServer(const bool isSavedGame, const bool useLobbyAuth = false, std::string password = {}, - std::string controllerSecret = {}); + CNetServer(const bool isSavedGame, std::uint16_t port, const bool useLobbyAuth = false, + std::string password = {}, std::string controllerSecret = {}, std::string initAttributes = {}); ~CNetServer(); - /** - * Begin listening for network connections. - * This function is synchronous (it won't return until the connection is established). - * @return true on success, false on error (e.g. port already in use) - */ - bool SetupConnection(const u16 port, std::string initAttributes = {}); - /** * Call from the GUI to asynchronously notify all clients that they should start loading the game. * SetupConnection must be called at least once. @@ -188,7 +177,7 @@ private: * by the host player's framerate - the only delay should be the network latency.) * * Thread-safety: - * - SetupConnection and constructor/destructor must be called from the main thread. + * - Constructor/destructor must be called from the main thread. * - The main thread may push commands onto the Queue members, * while holding the m_WorkerMutex lock. * - Public functions (SendMessage, Broadcast) must be called from the network @@ -220,18 +209,12 @@ public: private: friend class CNetServer; - CNetServerWorker(const bool continuesSavedGame, const bool useLobbyAuth, std::string password, - std::string controllerSecret); + CNetServerWorker(const bool continuesSavedGame, std::uint16_t port, const bool useLobbyAuth, + std::string password, std::string controllerSecret, std::string initAttributes); ~CNetServerWorker(); bool CheckPassword(const std::string& password, const std::string& salt) const; - /** - * Begin listening for network connections. - * @return true on success, false on error (e.g. port already in use) - */ - bool SetupConnection(const u16 port, std::string initAttributes); - /** * The given GUID will be (re)assigned to the given player ID. * Any player currently using that ID will be unassigned. @@ -359,7 +342,7 @@ private: CNetStatsTable* m_Stats{nullptr}; - NetServerState m_State{SERVER_STATE_UNCONNECTED}; + NetServerState m_State{SERVER_STATE_PREGAME}; CStrW m_ServerName{L"Unnamed Server"}; diff --git a/source/network/scripting/JSInterface_Network.cpp b/source/network/scripting/JSInterface_Network.cpp index 3752988ae3..e030119d70 100644 --- a/source/network/scripting/JSInterface_Network.cpp +++ b/source/network/scripting/JSInterface_Network.cpp @@ -106,13 +106,7 @@ void StartNetworkHost(const CStrW& playerName, const u16 serverPort, const CStr& HashCryptographically(password, hostJID + password + PS_SERIALIZATION_VERSION) : ""; const std::string secret = ps_generate_guid(); - g_NetServer = new CNetServer(continueSavedGame, hasLobby, hashedPassword, secret); - - if (!g_NetServer->SetupConnection(serverPort)) - { - SAFE_DELETE(g_NetServer); - throw std::runtime_error{"Failed to start server"}; - } + g_NetServer = new CNetServer(continueSavedGame, serverPort, hasLobby, hashedPassword, secret); // In lobby, we send our public ip and port on request to the players who want to connect. // Thus we need to know our public IP and use STUN to get it. @@ -127,13 +121,6 @@ void StartNetworkHost(const CStrW& playerName, const u16 serverPort, const CStr& g_Game = new CGame(storeReplay); g_NetClient = new CNetClient(g_Game, "127.0.0.1", serverPort, playerName, hostJID, hashedPassword, secret); - - if (!g_NetClient->SetupConnection(nullptr)) - { - SAFE_DELETE(g_NetClient); - SAFE_DELETE(g_Game); - throw std::runtime_error{"Failed to connect to server"}; - } } void StartNetworkJoin(const CStrW& playerName, const CStr& serverAddress, u16 serverPort, bool storeReplay) @@ -144,13 +131,6 @@ void StartNetworkJoin(const CStrW& playerName, const CStr& serverAddress, u16 se g_Game = new CGame(storeReplay); g_NetClient = new CNetClient(g_Game, serverAddress, serverPort, playerName); - - if (!g_NetClient->SetupConnection(nullptr)) - { - SAFE_DELETE(g_NetClient); - SAFE_DELETE(g_Game); - throw std::runtime_error{"Failed to connect to server"}; - } } /** diff --git a/source/network/tests/test_Net.h b/source/network/tests/test_Net.h index b593b10a55..28809a10c4 100644 --- a/source/network/tests/test_Net.h +++ b/source/network/tests/test_Net.h @@ -88,14 +88,8 @@ public: return true; } - void connect(CNetServer& server, const std::vector& clients, std::string initAttributes) + void connect(const std::vector& clients) { - TS_ASSERT(server.SetupConnection(PS_DEFAULT_PORT, std::move(initAttributes))); - for (CNetClient* client: clients) - { - TS_ASSERT(client->SetupConnection(nullptr)); - } - for (size_t i = 0; ; ++i) { // debug_printf("."); @@ -125,7 +119,7 @@ public: for (size_t j = 0; j < clients.size(); ++j) clients[j]->Poll(); - if (server.GetState() == SERVER_STATE_UNCONNECTED && clients_are_all(clients, NCS_UNCONNECTED)) + if (clients_are_all(clients, NCS_UNCONNECTED)) break; if (i > 20) @@ -169,8 +163,6 @@ public: CGame client2Game(false); CGame client3Game(false); - CNetServer server{false}; - JS::RootedValue attrs(rq.cx); Script::CreateObject( rq, @@ -180,6 +172,8 @@ public: "mapPath", "maps/scenarios/", "thing", "example"); + CNetServer server{false, PS_DEFAULT_PORT, false, {}, {}, Script::StringifyJSON(rq, &attrs, false)}; + CNetClient client1(&client1Game, "127.0.0.1", PS_DEFAULT_PORT); CNetClient client2(&client2Game, "127.0.0.1", PS_DEFAULT_PORT); CNetClient client3(&client3Game, "127.0.0.1", PS_DEFAULT_PORT); @@ -188,7 +182,7 @@ public: clients.push_back(&client2); clients.push_back(&client3); - connect(server, clients, Script::StringifyJSON(rq, &attrs, false)); + connect(clients); debug_printf("%s", client1.TestReadGuiMessages().c_str()); server.StartGame(); @@ -246,8 +240,6 @@ public: CGame client2Game(false); CGame client3Game(false); - CNetServer server{false}; - JS::RootedValue attrs(rq.cx); Script::CreateObject( rq, @@ -257,6 +249,8 @@ public: "mapPath", "maps/scenarios/", "thing", "example"); + CNetServer server{false, PS_DEFAULT_PORT, false, {}, {}, Script::StringifyJSON(rq, &attrs, false)}; + CNetClient client1(&client1Game, "127.0.0.1", PS_DEFAULT_PORT, L"alice"); CNetClient client2(&client2Game, "127.0.0.1", PS_DEFAULT_PORT, L"bob"); CNetClient client3(&client3Game, "127.0.0.1", PS_DEFAULT_PORT, L"charlie"); @@ -265,7 +259,7 @@ public: clients.push_back(&client2); clients.push_back(&client3); - connect(server, clients, Script::StringifyJSON(rq, &attrs, false)); + connect(clients); debug_printf("%s", client1.TestReadGuiMessages().c_str()); server.StartGame(); @@ -317,8 +311,6 @@ public: CNetClient client2B(&client2BGame, "127.0.0.1", PS_DEFAULT_PORT, L"bob"); clients.push_back(&client2B); - TS_ASSERT(client2B.SetupConnection(nullptr)); - for (size_t i = 0; ; ++i) { debug_printf("[%u]\n", client2B.GetCurrState());