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

@@ -144,6 +144,20 @@ NetPlayServer::NetPlayServer(const u16 port, const bool forward_port,
   }
 }
 
+static PlayerId* PeerPlayerId(ENetPeer* peer)
+{
+  return static_cast<PlayerId*>(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;
+        // Actual client initialization is deferred to the receive event, so here
-        unsigned int error;
+        // we'll just log the new connection.
-        {
+        INFO_LOG(NETPLAY, "Peer connected from: %x:%u", netEvent.peer->address.host,
-          std::lock_guard<std::recursive_mutex> lkg(m_crit.game);
+                 netEvent.peer->address.port);
-          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);
-        }
       }
       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);
+        if (!netEvent.peer->data)
-        Client& client = it->second;
-        if (OnData(rpac, client) != 0)
         {
-          // if a bad packet is received, disconnect the client
+          // uninitialized client, we'll assume this is their initialization packet
-          std::lock_guard<std::recursive_mutex> lkg(m_crit.game);
+          unsigned int error;
-          OnDisconnect(client);
-
-          if (netEvent.peer->data)
           {
-            delete (PlayerId*)netEvent.peer->data;
+            std::lock_guard<std::recursive_mutex> lkg(m_crit.game);
-            netEvent.peer->data = nullptr;
+            error = OnConnect(netEvent.peer, rpac);
+          }
+
+          if (error)
+          {
+            sf::Packet spac;
+            spac << static_cast<MessageId>(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<std::recursive_mutex> lkg(m_crit.game);
+            OnDisconnect(client);
+
+            ClearPeerPlayerId(netEvent.peer);
           }
         }
         enet_packet_destroy(netEvent.packet);
@@ -240,17 +257,13 @@ void NetPlayServer::ThreadFunc()
         std::lock_guard<std::recursive_mutex> 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)
+          ClearPeerPlayerId(netEvent.peer);
-          {
-            delete (PlayerId*)netEvent.peer->data;
-            netEvent.peer->data = nullptr;
-          }
         }
       }
       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;
+    ClearPeerPlayerId(player_entry.second.socket);
-    player_entry.second.socket->data = nullptr;
     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<std::recursive_mutex> 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();
   }

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