Move disconnected peer checking to STKHost loop to avoid race condition

src/network/stk_host.cpp

@@ -873,7 +873,7 @@ void STKHost::mainLoop()
                             std::lock_guard<std::mutex> lock(m_enet_cmd_mutex);
                             m_enet_cmd.emplace_back(p.second->getENetPeer(),
                                 (ENetPacket*)NULL, PDI_KICK_HIGH_PING,
-                                ECT_DISCONNECT);
+                                ECT_DISCONNECT, p.first->address);
                         }
                         else if (!p.second->hasWarnedForHighPing())
                         {
@@ -955,13 +955,27 @@ void STKHost::mainLoop()
             peer_lock.unlock();
         }
 
-        std::list<std::tuple<ENetPeer*, ENetPacket*, uint32_t,
+        std::vector<std::tuple<ENetPeer*, ENetPacket*, uint32_t,
-            ENetCommandType> > copied_list;
+            ENetCommandType, ENetAddress> > copied_list;
         std::unique_lock<std::mutex> lock(m_enet_cmd_mutex);
         std::swap(copied_list, m_enet_cmd);
         lock.unlock();
         for (auto& p : copied_list)
         {
+            ENetPeer* peer = std::get<0>(p);
+            ENetAddress& ea = std::get<4>(p);
+            ENetAddress& ea_peer_now = peer->address;
+            ENetPacket* packet = std::get<1>(p);
+            // Enet will reuse a disconnected peer so we check here to avoid
+            // sending to wrong peer
+            if (peer->state != ENET_PEER_STATE_CONNECTED ||
+                (ea_peer_now.host != ea.host && ea_peer_now.port != ea.port))
+            {
+                if (packet != NULL)
+                    enet_packet_destroy(packet);
+                continue;
+            }
+
             switch (std::get<3>(p))
             {
             case ECT_SEND_PACKET:
@@ -969,24 +983,22 @@ void STKHost::mainLoop()
                 // If enet_peer_send failed, destroy the packet to
                 // prevent leaking, this can only be done if the packet
                 // is copied instead of shared sending to all peers
-                ENetPacket* packet = std::get<1>(p);
+                if (enet_peer_send(peer, (uint8_t)std::get<2>(p), packet) < 0)
-                if (enet_peer_send(
-                    std::get<0>(p), (uint8_t)std::get<2>(p), packet) < 0)
                 {
                     enet_packet_destroy(packet);
                 }
                 break;
             }
             case ECT_DISCONNECT:
-                enet_peer_disconnect(std::get<0>(p), std::get<2>(p));
+                enet_peer_disconnect(peer, std::get<2>(p));
                 break;
             case ECT_RESET:
                 // Flush enet before reset (so previous command is send)
                 enet_host_flush(host);
-                enet_peer_reset(std::get<0>(p));
+                enet_peer_reset(peer);
                 // Remove the stk peer of it
                 std::lock_guard<std::mutex> lock(m_peers_mutex);
-                m_peers.erase(std::get<0>(p));
+                m_peers.erase(peer);
                 break;
             }
         }

src/network/stk_host.hpp

@@ -98,9 +98,9 @@ private:
 
     /** Let (atm enet_peer_send and enet_peer_disconnect) run in the listening
      *  thread. */
-    std::list<std::tuple</*peer receive*/ENetPeer*,
+    std::vector<std::tuple</*peer receive*/ENetPeer*,
         /*packet to send*/ENetPacket*, /*integer data*/uint32_t,
-        ENetCommandType> > m_enet_cmd;
+        ENetCommandType, ENetAddress> > m_enet_cmd;
 
     /** Protect \ref m_enet_cmd from multiple threads usage. */
     std::mutex m_enet_cmd_mutex;
@@ -285,10 +285,10 @@ public:
     void setErrorMessage(const irr::core::stringw &message);
     // ------------------------------------------------------------------------
     void addEnetCommand(ENetPeer* peer, ENetPacket* packet, uint32_t i,
-                        ENetCommandType ect)
+                        ENetCommandType ect, ENetAddress ea)
     {
         std::lock_guard<std::mutex> lock(m_enet_cmd_mutex);
-        m_enet_cmd.emplace_back(peer, packet, i, ect);
+        m_enet_cmd.emplace_back(peer, packet, i, ect, ea);
     }
     // ------------------------------------------------------------------------
     /** Returns the last error (or "" if no error has happened). */

src/network/stk_peer.cpp

@@ -83,12 +83,9 @@ void STKPeer::disconnect()
 {
     if (m_disconnected.load())
         return;
-    if (m_enet_peer->state != ENET_PEER_STATE_CONNECTED ||
-        (m_enet_peer->address.host != m_address.host &&
-        m_enet_peer->address.port != m_address.port))
-        return;
     m_disconnected.store(true);
-    m_host->addEnetCommand(m_enet_peer, NULL, PDI_NORMAL, ECT_DISCONNECT);
+    m_host->addEnetCommand(m_enet_peer, NULL, PDI_NORMAL, ECT_DISCONNECT,
+        m_address);
 }   // disconnect
 
 //-----------------------------------------------------------------------------
@@ -98,12 +95,9 @@ void STKPeer::kick()
 {
     if (m_disconnected.load())
         return;
-    if (m_enet_peer->state != ENET_PEER_STATE_CONNECTED ||
-        (m_enet_peer->address.host != m_address.host &&
-        m_enet_peer->address.port != m_address.port))
-        return;
     m_disconnected.store(true);
-    m_host->addEnetCommand(m_enet_peer, NULL, PDI_KICK, ECT_DISCONNECT);
+    m_host->addEnetCommand(m_enet_peer, NULL, PDI_KICK, ECT_DISCONNECT,
+        m_address);
 }   // kick
 
 //-----------------------------------------------------------------------------
@@ -113,12 +107,8 @@ void STKPeer::reset()
 {
     if (m_disconnected.load())
         return;
-    if (m_enet_peer->state != ENET_PEER_STATE_CONNECTED ||
-        (m_enet_peer->address.host != m_address.host &&
-        m_enet_peer->address.port != m_address.port))
-        return;
     m_disconnected.store(true);
-    m_host->addEnetCommand(m_enet_peer, NULL, 0, ECT_RESET);
+    m_host->addEnetCommand(m_enet_peer, NULL, 0, ECT_RESET, m_address);
 }   // reset
 
 //-----------------------------------------------------------------------------
@@ -131,12 +121,6 @@ void STKPeer::sendPacket(NetworkString *data, bool reliable, bool encrypted)
 {
     if (m_disconnected.load())
         return;
-    // Enet will reuse a disconnected peer so we check here to avoid sending
-    // to wrong peer
-    if (m_enet_peer->state != ENET_PEER_STATE_CONNECTED ||
-        (m_enet_peer->address.host != m_address.host &&
-        m_enet_peer->address.port != m_address.port))
-        return;
 
     ENetPacket* packet = NULL;
     if (m_crypto && encrypted)
@@ -162,7 +146,7 @@ void STKPeer::sendPacket(NetworkString *data, bool reliable, bool encrypted)
         }
         m_host->addEnetCommand(m_enet_peer, packet,
                 encrypted ? EVENT_CHANNEL_NORMAL : EVENT_CHANNEL_UNENCRYPTED,
-                ECT_SEND_PACKET);
+                ECT_SEND_PACKET, m_address);
     }
 }   // sendPacket