From 7597740bff711d78e3f005bdfad214154281559a Mon Sep 17 00:00:00 2001 From: Rubidium Date: Sun, 11 Apr 2021 11:30:44 +0200 Subject: Fix: split the UDP blocking of sockets to only the socket involved, and when another thread is busy do not attempt to process the packets of that socket --- src/network/network_udp.cpp | 96 ++++++++++++++++++++++++++++----------------- 1 file changed, 59 insertions(+), 37 deletions(-) diff --git a/src/network/network_udp.cpp b/src/network/network_udp.cpp index e4276c58f..6d7e6ca69 100644 --- a/src/network/network_udp.cpp +++ b/src/network/network_udp.cpp @@ -33,9 +33,6 @@ #include "../safeguards.h" -/** Mutex for all out threaded udp resolution and such. */ -static std::mutex _network_udp_mutex; - /** Session key to register ourselves to the master server */ static uint64 _session_key = 0; @@ -43,14 +40,46 @@ static const std::chrono::minutes ADVERTISE_NORMAL_INTERVAL(15); ///< interval b static const std::chrono::seconds ADVERTISE_RETRY_INTERVAL(10); ///< re-advertise when no response after this amount of time. static const uint32 ADVERTISE_RETRY_TIMES = 3; ///< give up re-advertising after this much failed retries -NetworkUDPSocketHandler *_udp_client_socket = nullptr; ///< udp client socket -NetworkUDPSocketHandler *_udp_server_socket = nullptr; ///< udp server socket -NetworkUDPSocketHandler *_udp_master_socket = nullptr; ///< udp master socket - static bool _network_udp_server; ///< Is the UDP server started? static uint16 _network_udp_broadcast; ///< Timeout for the UDP broadcasts. static uint8 _network_advertise_retries; ///< The number of advertisement retries we did. +/** Some information about a socket, which exists before the actual socket has been created to provide locking and the likes. */ +struct UDPSocket { + const std::string name; ///< The name of the socket. + std::mutex mutex; ///< Mutex for everything that (indirectly) touches the sockets within the handler. + NetworkUDPSocketHandler *socket; ///< The actual socket, which may be nullptr when not initialized yet. + std::atomic receive_iterations_locked; ///< The number of receive iterations the mutex was locked. + + UDPSocket(const std::string &name_) : name(name_), socket(nullptr) {} + + void Close() + { + std::lock_guard lock(mutex); + socket->Close(); + delete socket; + socket = nullptr; + } + + void ReceivePackets() + { + std::unique_lock lock(mutex, std::defer_lock); + if (!lock.try_lock()) { + if (++receive_iterations_locked % 32 == 0) { + DEBUG(net, 0, "[udp] %s background UDP loop processing appears to be blocked. Your OS may be low on UDP send buffers.", name.c_str()); + } + return; + } + + receive_iterations_locked.store(0); + socket->ReceivePackets(); + } +}; + +static UDPSocket _udp_client("Client"); ///< udp client socket +static UDPSocket _udp_server("Server"); ///< udp server socket +static UDPSocket _udp_master("Master"); ///< udp master socket + /** * Helper function doing the actual work for querying the server. * @param address The address of the server. @@ -67,11 +96,11 @@ static void DoNetworkUDPQueryServer(NetworkAddress &address, bool needs_mutex, b item->manually = manually; NetworkGameListAddItemDelayed(item); - std::unique_lock lock(_network_udp_mutex, std::defer_lock); + std::unique_lock lock(_udp_client.mutex, std::defer_lock); if (needs_mutex) lock.lock(); /* Init the packet */ Packet p(PACKET_UDP_CLIENT_FIND_SERVER); - if (_udp_client_socket != nullptr) _udp_client_socket->SendPacket(&p, &address); + if (_udp_client.socket != nullptr) _udp_client.socket->SendPacket(&p, &address); } /** @@ -479,7 +508,8 @@ void NetworkUDPQueryMasterServer() p.Send_uint8(NETWORK_MASTER_SERVER_VERSION); p.Send_uint8(SLT_AUTODETECT); - _udp_client_socket->SendPacket(&p, &out_addr, true); + std::lock_guard lock(_udp_client.mutex); + _udp_client.socket->SendPacket(&p, &out_addr, true); DEBUG(net, 2, "[udp] master server queried at %s", out_addr.GetAddressAsString().c_str()); } @@ -492,7 +522,7 @@ void NetworkUDPSearchGame() DEBUG(net, 0, "[udp] searching server"); - NetworkUDPBroadCast(_udp_client_socket); + NetworkUDPBroadCast(_udp_client.socket); _network_udp_broadcast = 300; // Stay searching for 300 ticks } @@ -512,8 +542,8 @@ static void NetworkUDPRemoveAdvertiseThread() p.Send_uint8 (NETWORK_MASTER_SERVER_VERSION); p.Send_uint16(_settings_client.network.server_port); - std::lock_guard lock(_network_udp_mutex); - if (_udp_master_socket != nullptr) _udp_master_socket->SendPacket(&p, &out_addr, true); + std::lock_guard lock(_udp_master.mutex); + if (_udp_master.socket != nullptr) _udp_master.socket->SendPacket(&p, &out_addr, true); } /** @@ -564,8 +594,8 @@ static void NetworkUDPAdvertiseThread() p.Send_uint16(_settings_client.network.server_port); p.Send_uint64(_session_key); - std::lock_guard lock(_network_udp_mutex); - if (_udp_master_socket != nullptr) _udp_master_socket->SendPacket(&p, &out_addr, true); + std::lock_guard lock(_udp_master.mutex); + if (_udp_master.socket != nullptr) _udp_master.socket->SendPacket(&p, &out_addr, true); } /** @@ -607,22 +637,22 @@ void NetworkUDPAdvertise() void NetworkUDPInitialize() { /* If not closed, then do it. */ - if (_udp_server_socket != nullptr) NetworkUDPClose(); + if (_udp_server.socket != nullptr) NetworkUDPClose(); DEBUG(net, 1, "[udp] initializing listeners"); - assert(_udp_client_socket == nullptr && _udp_server_socket == nullptr && _udp_master_socket == nullptr); + assert(_udp_client.socket == nullptr && _udp_server.socket == nullptr && _udp_master.socket == nullptr); - std::lock_guard lock(_network_udp_mutex); + std::scoped_lock lock(_udp_client.mutex, _udp_server.mutex, _udp_master.mutex); - _udp_client_socket = new ClientNetworkUDPSocketHandler(); + _udp_client.socket = new ClientNetworkUDPSocketHandler(); NetworkAddressList server; GetBindAddresses(&server, _settings_client.network.server_port); - _udp_server_socket = new ServerNetworkUDPSocketHandler(&server); + _udp_server.socket = new ServerNetworkUDPSocketHandler(&server); server.clear(); GetBindAddresses(&server, 0); - _udp_master_socket = new MasterNetworkUDPSocketHandler(&server); + _udp_master.socket = new MasterNetworkUDPSocketHandler(&server); _network_udp_server = false; _network_udp_broadcast = 0; @@ -632,22 +662,16 @@ void NetworkUDPInitialize() /** Start the listening of the UDP server component. */ void NetworkUDPServerListen() { - _network_udp_server = _udp_server_socket->Listen(); + std::lock_guard lock(_udp_server.mutex); + _network_udp_server = _udp_server.socket->Listen(); } /** Close all UDP related stuff. */ void NetworkUDPClose() { - std::lock_guard lock(_network_udp_mutex); - _udp_server_socket->Close(); - _udp_master_socket->Close(); - _udp_client_socket->Close(); - delete _udp_client_socket; - delete _udp_server_socket; - delete _udp_master_socket; - _udp_client_socket = nullptr; - _udp_server_socket = nullptr; - _udp_master_socket = nullptr; + _udp_client.Close(); + _udp_server.Close(); + _udp_master.Close(); _network_udp_server = false; _network_udp_broadcast = 0; @@ -657,13 +681,11 @@ void NetworkUDPClose() /** Receive the UDP packets. */ void NetworkBackgroundUDPLoop() { - std::lock_guard lock(_network_udp_mutex); - if (_network_udp_server) { - _udp_server_socket->ReceivePackets(); - _udp_master_socket->ReceivePackets(); + _udp_server.ReceivePackets(); + _udp_master.ReceivePackets(); } else { - _udp_client_socket->ReceivePackets(); + _udp_client.ReceivePackets(); if (_network_udp_broadcast > 0) _network_udp_broadcast--; } } -- cgit v1.2.3-70-g09d2