From 5c81285b910affc5eaf494c4dbf0e89a75d95c29 Mon Sep 17 00:00:00 2001 From: Techjar Date: Wed, 12 Sep 2018 06:41:46 -0400 Subject: [PATCH] NetPlay: Fix server peer initialization hang The implementation of peer initialization would hang if the initial packet was never received. This fixes that issue by deferring the initialization to the packet receive loop. --- Source/Core/Core/NetPlayServer.cpp | 101 +++++++++++++++-------------- Source/Core/Core/NetPlayServer.h | 2 +- 2 files changed, 53 insertions(+), 50 deletions(-) diff --git a/Source/Core/Core/NetPlayServer.cpp b/Source/Core/Core/NetPlayServer.cpp index e79c396319..b765a8c70a 100644 --- a/Source/Core/Core/NetPlayServer.cpp +++ b/Source/Core/Core/NetPlayServer.cpp @@ -144,6 +144,20 @@ NetPlayServer::NetPlayServer(const u16 port, const bool forward_port, } } +static PlayerId* PeerPlayerId(ENetPeer* peer) +{ + return static_cast(peer->data); +} + +static void ClearPeerPlayerId(ENetPeer* peer) +{ + if (peer->data) + { + delete PeerPlayerId(peer); + peer->data = nullptr; + } +} + // called from ---NETPLAY--- thread void NetPlayServer::ThreadFunc() { @@ -191,26 +205,10 @@ void NetPlayServer::ThreadFunc() { case ENET_EVENT_TYPE_CONNECT: { - ENetPeer* accept_peer = netEvent.peer; - unsigned int error; - { - std::lock_guard lkg(m_crit.game); - error = OnConnect(accept_peer); - } - - if (error) - { - sf::Packet spac; - spac << (MessageId)error; - // don't need to lock, this client isn't in the client map - Send(accept_peer, spac); - if (netEvent.peer->data) - { - delete (PlayerId*)netEvent.peer->data; - netEvent.peer->data = nullptr; - } - enet_peer_disconnect_later(accept_peer, 0); - } + // Actual client initialization is deferred to the receive event, so here + // we'll just log the new connection. + INFO_LOG(NETPLAY, "Peer connected from: %x:%u", netEvent.peer->address.host, + netEvent.peer->address.port); } break; case ENET_EVENT_TYPE_RECEIVE: @@ -218,18 +216,37 @@ void NetPlayServer::ThreadFunc() sf::Packet rpac; rpac.append(netEvent.packet->data, netEvent.packet->dataLength); - auto it = m_players.find(*(PlayerId*)netEvent.peer->data); - Client& client = it->second; - if (OnData(rpac, client) != 0) + if (!netEvent.peer->data) { - // if a bad packet is received, disconnect the client - std::lock_guard lkg(m_crit.game); - OnDisconnect(client); - - if (netEvent.peer->data) + // uninitialized client, we'll assume this is their initialization packet + unsigned int error; { - delete (PlayerId*)netEvent.peer->data; - netEvent.peer->data = nullptr; + std::lock_guard lkg(m_crit.game); + error = OnConnect(netEvent.peer, rpac); + } + + if (error) + { + sf::Packet spac; + spac << static_cast(error); + // don't need to lock, this client isn't in the client map + Send(netEvent.peer, spac); + + ClearPeerPlayerId(netEvent.peer); + enet_peer_disconnect_later(netEvent.peer, 0); + } + } + else + { + auto it = m_players.find(*PeerPlayerId(netEvent.peer)); + Client& client = it->second; + if (OnData(rpac, client) != 0) + { + // if a bad packet is received, disconnect the client + std::lock_guard lkg(m_crit.game); + OnDisconnect(client); + + ClearPeerPlayerId(netEvent.peer); } } enet_packet_destroy(netEvent.packet); @@ -240,17 +257,13 @@ void NetPlayServer::ThreadFunc() std::lock_guard lkg(m_crit.game); if (!netEvent.peer->data) break; - auto it = m_players.find(*(PlayerId*)netEvent.peer->data); + auto it = m_players.find(*PeerPlayerId(netEvent.peer)); if (it != m_players.end()) { Client& client = it->second; OnDisconnect(client); - if (netEvent.peer->data) - { - delete (PlayerId*)netEvent.peer->data; - netEvent.peer->data = nullptr; - } + ClearPeerPlayerId(netEvent.peer); } } break; @@ -263,23 +276,14 @@ void NetPlayServer::ThreadFunc() // close listening socket and client sockets for (auto& player_entry : m_players) { - delete (PlayerId*)player_entry.second.socket->data; - player_entry.second.socket->data = nullptr; + ClearPeerPlayerId(player_entry.second.socket); enet_peer_disconnect(player_entry.second.socket, 0); } } // called from ---NETPLAY--- thread -unsigned int NetPlayServer::OnConnect(ENetPeer* socket) +unsigned int NetPlayServer::OnConnect(ENetPeer* socket, sf::Packet& rpac) { - sf::Packet rpac; - ENetPacket* epack; - do - { - epack = enet_peer_receive(socket, nullptr); - } while (epack == nullptr); - rpac.append(epack->data, epack->dataLength); - // give new client first available id PlayerId pid = 1; for (auto i = m_players.begin(); i != m_players.end(); ++i) @@ -316,7 +320,6 @@ unsigned int NetPlayServer::OnConnect(ENetPeer* socket) rpac >> player.revision; rpac >> player.name; - enet_packet_destroy(epack); // try to automatically assign new user a pad for (PadMapping& mapping : m_pad_map) { @@ -398,7 +401,7 @@ unsigned int NetPlayServer::OnConnect(ENetPeer* socket) // add client to the player list { std::lock_guard lkp(m_crit.players); - m_players.emplace(*(PlayerId*)player.socket->data, std::move(player)); + m_players.emplace(*PeerPlayerId(player.socket), std::move(player)); UpdatePadMapping(); // sync pad mappings with everyone UpdateWiimoteMapping(); } diff --git a/Source/Core/Core/NetPlayServer.h b/Source/Core/Core/NetPlayServer.h index e16d7c1107..951941b10b 100644 --- a/Source/Core/Core/NetPlayServer.h +++ b/Source/Core/Core/NetPlayServer.h @@ -125,7 +125,7 @@ private: void SendToClients(const sf::Packet& packet, PlayerId skip_pid = 0, u8 channel_id = DEFAULT_CHANNEL); void Send(ENetPeer* socket, const sf::Packet& packet, u8 channel_id = DEFAULT_CHANNEL); - unsigned int OnConnect(ENetPeer* socket); + unsigned int OnConnect(ENetPeer* socket, sf::Packet& rpac); unsigned int OnDisconnect(const Client& player); unsigned int OnData(sf::Packet& packet, Client& player);