From 0321f6a8a795dcbdd75d27470fff92d8326a0ef0 Mon Sep 17 00:00:00 2001 From: josue Date: Fri, 12 Jun 2026 13:10:22 +0200 Subject: [PATCH] Don't block on hole punching when a client joins CNetServer::SendHolePunchingMessage is called on the main thread (from the lobby's XMPP handler) whenever a lobby client requests to connect. It called StunClient::SendHolePunchingMessages, which sleeps for fw_punch.delay (default 200 ms) after each of the fw_punch.num_msg (default 3) messages. Freezing the main thread for ~600 ms freezes the hosting player's game, which in turn delays the lockstep turns of every player in the match. Instead, queue the request to the network server worker thread (like lobby auths) and pace the messages from CNetServerWorker::RunStep without sleeping. As the worker now owns the whole exchange, this also removes the concurrent use of the server's ENetHost from two threads. Punching stops as soon as the peer connects, which also gives num_msg = -1 (send indefinitely) a sane meaning. Fixes: #7957 Co-Authored-By: Claude Fable 5 --- source/network/NetServer.cpp | 58 ++++++++++++++++++++++++++++++++--- source/network/NetServer.h | 24 ++++++++++++++- source/network/StunClient.cpp | 7 ++++- source/network/StunClient.h | 10 +++++- 4 files changed, 92 insertions(+), 7 deletions(-) diff --git a/source/network/NetServer.cpp b/source/network/NetServer.cpp index 378bd08304..49be00f5aa 100644 --- a/source/network/NetServer.cpp +++ b/source/network/NetServer.cpp @@ -52,6 +52,7 @@ #include "simulation2/system/TurnManager.h" #include +#include #include #include #include @@ -414,6 +415,7 @@ bool CNetServerWorker::RunStep() std::vector newStartGame; std::vector> newLobbyAuths; std::vector newTurnLength; + std::vector> newHolePunchRequests; { std::lock_guard lock(m_WorkerMutex); @@ -424,6 +426,7 @@ bool CNetServerWorker::RunStep() newStartGame.swap(m_StartGameQueue); newLobbyAuths.swap(m_LobbyAuthQueue); newTurnLength.swap(m_TurnLengthQueue); + newHolePunchRequests.swap(m_HolePunchQueue); } if (!newTurnLength.empty()) @@ -442,6 +445,8 @@ bool CNetServerWorker::RunStep() CheckClientConnections(); + ProcessHolePunching(std::move(newHolePunchRequests)); + // Process network events: ENetEvent event; @@ -470,6 +475,13 @@ bool CNetServerWorker::RunStep() enet_address_get_host_ip(&event.peer->address, hostname, ARRAY_SIZE(hostname)); LOGMESSAGE("Net server: Received connection from %s:%u", hostname, (unsigned int)event.peer->address.port); + // The peer connected, so the hole punching for it succeeded or wasn't needed. + std::erase_if(m_HolePunchTargets, [&](const HolePunchTarget& target) + { + return target.address.host == event.peer->address.host && + target.address.port == event.peer->address.port; + }); + // Set up a session object for this peer const std::unique_ptr& session{m_Sessions.emplace_back( @@ -1644,10 +1656,47 @@ CStrW CNetServerWorker::DeduplicatePlayerName(const CStrW& original) } } -void CNetServerWorker::SendHolePunchingMessage(const CStr& ipStr, u16 port) +void CNetServerWorker::ProcessHolePunching(std::vector>&& newRequests) { - if (m_Host) - StunClient::SendHolePunchingMessages(*m_Host, ipStr, port); + if (!m_Host) + return; + + const std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now(); + + for (const std::pair& request : newRequests) + { + // A negative number of messages means punching until the peer connects. + const int numMessages{g_ConfigDB.Get("lobby.fw_punch.num_msg", 3)}; + if (numMessages == 0) + continue; + + ENetAddress address; + address.port = request.second; + if (enet_address_set_host(&address, request.first.c_str()) != 0) + { + LOGWARNING("Net server: Failed to resolve hole punching target %s", request.first.c_str()); + continue; + } + m_HolePunchTargets.push_back({address, numMessages, now}); + } + + if (m_HolePunchTargets.empty()) + return; + + const std::chrono::milliseconds delay{g_ConfigDB.Get("lobby.fw_punch.delay", 200)}; + for (HolePunchTarget& target : m_HolePunchTargets) + { + if (now < target.nextSendTime) + continue; + + StunClient::SendHolePunchingMessage(*m_Host, target.address); + if (target.remainingMessages > 0) + --target.remainingMessages; + target.nextSendTime = now + delay; + } + + std::erase_if(m_HolePunchTargets, + [](const HolePunchTarget& target) { return target.remainingMessages == 0; }); } @@ -1735,5 +1784,6 @@ void CNetServer::SetTurnLength(u32 msecs) void CNetServer::SendHolePunchingMessage(const CStr& ip, u16 port) { - m_Worker.SendHolePunchingMessage(ip, port); + std::lock_guard lock(m_Worker.m_WorkerMutex); + m_Worker.m_HolePunchQueue.emplace_back(ip, port); } diff --git a/source/network/NetServer.h b/source/network/NetServer.h index 50404f707c..5e78a50711 100644 --- a/source/network/NetServer.h +++ b/source/network/NetServer.h @@ -24,6 +24,7 @@ #include "network/NetHost.h" #include "ps/CStr.h" +#include #include #include #include @@ -234,7 +235,11 @@ private: */ void CheckClientConnections(); - void SendHolePunchingMessage(const CStr& ip, u16 port); + /** + * Turn new hole punching requests from the game thread into targets and + * send the due hole punching messages without blocking. + */ + void ProcessHolePunching(std::vector>&& newRequests); /** * Internal script context for (de)serializing script messages, @@ -319,6 +324,22 @@ private: */ std::time_t m_LastConnectionCheck{0}; + /** + * A peer which should receive hole punching messages until it connects. + */ + struct HolePunchTarget + { + ENetAddress address; + /// A negative number means sending until the peer connects. + int remainingMessages; + std::chrono::steady_clock::time_point nextSendTime; + }; + + /** + * The peers currently being sent hole punching messages. + */ + std::vector m_HolePunchTargets; + private: // Thread-related stuff: @@ -344,6 +365,7 @@ private: std::vector m_StartGameQueue; std::vector> m_LobbyAuthQueue; std::vector m_TurnLengthQueue; + std::vector> m_HolePunchQueue; }; /** diff --git a/source/network/StunClient.cpp b/source/network/StunClient.cpp index c2cc98d8ca..dbcc2c9fc6 100644 --- a/source/network/StunClient.cpp +++ b/source/network/StunClient.cpp @@ -1,4 +1,4 @@ -/* Copyright (C) 2025 Wildfire Games. +/* Copyright (C) 2026 Wildfire Games. * Copyright (C) 2013-2016 SuperTuxKart-Team. * This file is part of 0 A.D. * @@ -371,6 +371,11 @@ void SendHolePunchingMessages(ENetHost& enetClient, const std::string& serverAdd } } +void SendHolePunchingMessage(ENetHost& enetClient, const ENetAddress& addr) +{ + SendStunRequest(enetClient, addr); +} + bool FindLocalIP(CStr& ip) { // Open an UDP socket. diff --git a/source/network/StunClient.h b/source/network/StunClient.h index 5caef4bdbf..90f165ef8c 100644 --- a/source/network/StunClient.h +++ b/source/network/StunClient.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2025 Wildfire Games. +/* Copyright (C) 2026 Wildfire Games. * Copyright (C) 2013-2016 SuperTuxKart-Team. * This file is part of 0 A.D. * @@ -46,6 +46,14 @@ bool FindPublicIP(ENetHost& enetClient, CStr8& ip, u16& port); */ void SendHolePunchingMessages(ENetHost& enetClient, const std::string& serverAddress, u16 serverPort); +/** + * Send a single hole punching message to the target address. + * Unlike SendHolePunchingMessages this doesn't block, so the caller + * is responsible for repeating the message at a sensible interval. + * @see SendHolePunchingMessages + */ +void SendHolePunchingMessage(ENetHost& enetClient, const ENetAddress& addr); + /** * Return the local IP. * Technically not a STUN method, but convenient to define here.