Handle forwarding of netplay state demotions correctly.

Netplay state demotions, i.e. changes from playing to spectating or
disconnected states, could cause chain disconnections of all other
clients. This was due to a bug in when MODE change messages were sent.
Clients rely on the server sending all messages in its own order, and as
a consequence, the server typically holds messages for retransmission
until they can be retransmitted at the correct time. MODE messages were
not held, so could be sent early. When they were sent early, this caused
other clients to panic and disconnect.

A smaller but much stupider secondary bug was also fixed, in which the
first connection could be dropped due simply to writing connections[0]
instead of connections[i] somewhere.

network/netplay/netplay_frontend.c

@@ -165,6 +165,10 @@ static bool get_self_input_state(netplay_t *netplay)
          netplay_send_cur_input(netplay, &netplay->connections[i]);
    }
 
+   /* Handle any delayed state changes */
+   if (netplay->is_server)
+      netplay_delayed_state_change(netplay);
+
    return true;
 }
 
@@ -847,7 +851,7 @@ void netplay_post_frame(netplay_t *netplay)
       if (connection->active &&
           !netplay_send_flush(&connection->send_packet_buffer, connection->fd,
             false))
-         netplay_hangup(netplay, &netplay->connections[0]);
+         netplay_hangup(netplay, connection);
    }
 }
 

network/netplay/netplay_io.c

@@ -130,21 +130,19 @@ void netplay_hangup(netplay_t *netplay, struct netplay_connection *connection)
    }
    else
    {
-      /* Remove this player */
+      /* Mark the player for removal */
       if (connection->mode == NETPLAY_CONNECTION_PLAYING ||
           connection->mode == NETPLAY_CONNECTION_SLAVE)
       {
+         /* This special mode keeps the connection object alive long enough to
+          * send the disconnection message at the correct time */
+         connection->mode = NETPLAY_CONNECTION_DELAYED_DISCONNECT;
+         connection->delay_frame = netplay->read_frame_count[connection->player];
+
+         /* Mark them as not playing anymore */
          netplay->connected_players &= ~(1<<connection->player);
          netplay->connected_slaves  &= ~(1<<connection->player);
 
-         /* FIXME: Duplication */
-         if (netplay->is_server)
-         {
-            uint32_t payload[2];
-            payload[0] = htonl(netplay->read_frame_count[connection->player]);
-            payload[1] = htonl(connection->player);
-            netplay_send_raw_cmd_all(netplay, connection, NETPLAY_CMD_MODE, payload, sizeof(payload));
-         }
       }
 
    }
@@ -154,6 +152,42 @@ void netplay_hangup(netplay_t *netplay, struct netplay_connection *connection)
       remote_unpaused(netplay, connection);
 }
 
+/**
+ * netplay_delayed_state_change:
+ *
+ * Handle any pending state changes which are ready as of the beginning of the
+ * current frame.
+ */
+void netplay_delayed_state_change(netplay_t *netplay)
+{
+   struct netplay_connection *connection;
+   size_t i;
+
+   for (i = 0; i < netplay->connections_size; i++)
+   {
+      connection = &netplay->connections[i];
+      if ((connection->active || connection->mode == NETPLAY_CONNECTION_DELAYED_DISCONNECT) &&
+          connection->delay_frame &&
+          connection->delay_frame <= netplay->self_frame_count)
+      {
+         /* Something was delayed! Prepare the MODE command */
+         uint32_t payload[2];
+         payload[0] = htonl(connection->delay_frame);
+         payload[1] = htonl(connection->player);
+
+         /* Remove the connection entirely if relevant */
+         if (connection->mode == NETPLAY_CONNECTION_DELAYED_DISCONNECT)
+            connection->mode = NETPLAY_CONNECTION_NONE;
+
+         /* Then send the mode change packet */
+         netplay_send_raw_cmd_all(netplay, connection, NETPLAY_CMD_MODE, payload, sizeof(payload));
+
+         /* And forget the delay frame */
+         connection->delay_frame = 0;
+      }
+   }
+}
+
 /* Send the specified input data */
 static bool send_input_frame(netplay_t *netplay,
    struct netplay_connection *only, struct netplay_connection *except,
@@ -663,17 +697,13 @@ static bool netplay_get_cmd(netplay_t *netplay,
              connection->mode == NETPLAY_CONNECTION_SLAVE)
          {
             /* The frame we haven't received is their end frame */
-            payload[0] = htonl(netplay->read_frame_count[connection->player]);
+            connection->delay_frame = netplay->read_frame_count[connection->player];
 
             /* Mark them as not playing anymore */
             connection->mode = NETPLAY_CONNECTION_SPECTATING;
             netplay->connected_players &= ~(1<<connection->player);
             netplay->connected_slaves  &= ~(1<<connection->player);
 
-            /* Tell everyone */
-            payload[1] = htonl(connection->player);
-            netplay_send_raw_cmd_all(netplay, connection, NETPLAY_CMD_MODE, payload, sizeof(payload));
-
             /* Announce it */
             msg[sizeof(msg)-1] = '\0';
             snprintf(msg, sizeof(msg)-1, "Player %d has left", connection->player+1);
@@ -731,6 +761,14 @@ static bool netplay_get_cmd(netplay_t *netplay,
             return netplay_cmd_nak(netplay, connection);
          }
 
+         if (connection->delay_frame)
+         {
+            /* Can't switch modes while a mode switch is already in progress. */
+            payload[0] = htonl(NETPLAY_CMD_MODE_REFUSED_REASON_TOO_FAST);
+            netplay_send_raw_cmd(netplay, connection, NETPLAY_CMD_MODE_REFUSED, payload, sizeof(uint32_t));
+            break;
+         }
+
          if (!connection->can_play)
          {
             /* Not allowed to play */

network/netplay/netplay_private.h

@@ -192,13 +192,19 @@ enum netplay_cmd_mode_reasons
    NETPLAY_CMD_MODE_REFUSED_REASON_UNPRIVILEGED,
 
    /* There are no free player slots */
-   NETPLAY_CMD_MODE_REFUSED_REASON_NO_SLOTS
+   NETPLAY_CMD_MODE_REFUSED_REASON_NO_SLOTS,
+
+   /* You're changing modes too fast */
+   NETPLAY_CMD_MODE_REFUSED_REASON_TOO_FAST
 };
 
 enum rarch_netplay_connection_mode
 {
    NETPLAY_CONNECTION_NONE = 0,
 
+   NETPLAY_CONNECTION_DELAYED_DISCONNECT, /* The connection is dead, but data
+                                             is still waiting to be forwarded */
+
    /* Initialization: */
    NETPLAY_CONNECTION_INIT, /* Waiting for header */
    NETPLAY_CONNECTION_PRE_NICK, /* Waiting for nick */
@@ -298,6 +304,11 @@ struct netplay_connection
    /* Mode of the connection */
    enum rarch_netplay_connection_mode mode;
 
+   /* If the mode is a DELAYED_DISCONNECT or SPECTATOR, the transmission of the
+    * mode change may have to wait for data to be forwarded. This is the frame
+    * to wait for, or 0 if no delay is active. */
+   uint32_t delay_frame;
+
    /* Player # of connected player */
    uint32_t player;
 
@@ -720,6 +731,14 @@ void netplay_free(netplay_t *netplay);
  */
 void netplay_hangup(netplay_t *netplay, struct netplay_connection *connection);
 
+/**
+ * netplay_delayed_state_change:
+ *
+ * Handle any pending state changes which are ready as of the beginning of the
+ * current frame.
+ */
+void netplay_delayed_state_change(netplay_t *netplay);
+
 /**
  * netplay_send_cur_input
  *

network/netplay/netplay_sync.c

@@ -298,7 +298,8 @@ bool netplay_sync_pre_frame(netplay_t *netplay)
 
          /* Allocate a connection */
          for (connection_num = 0; connection_num < netplay->connections_size; connection_num++)
-            if (!netplay->connections[connection_num].active) break;
+            if (!netplay->connections[connection_num].active &&
+                netplay->connections[connection_num].mode != NETPLAY_CONNECTION_DELAYED_DISCONNECT) break;
          if (connection_num == netplay->connections_size)
          {
             if (connection_num == 0)