Merge pull request #134 from Prime-Codes-GmbH/fix/viaversion-play-state

fix: guard ViaVersion packet sends by connection state

Author: vytskalt

implementation/src/main/java/net/megavex/scoreboardlibrary/implementation/packetAdapter/modern/team/LegacyTeamDisplayPacketAdapter.java

@@ -10,10 +10,12 @@
 import com.viaversion.viaversion.api.type.Types;
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
+import io.netty.channel.Channel;
 import net.megavex.scoreboardlibrary.implementation.commons.LegacyFormatUtil;
 import net.megavex.scoreboardlibrary.implementation.packetAdapter.ImmutableTeamProperties;
 import net.megavex.scoreboardlibrary.implementation.packetAdapter.PropertiesPacketType;
 import net.megavex.scoreboardlibrary.implementation.packetAdapter.modern.PacketAdapterProviderImpl;
+import net.megavex.scoreboardlibrary.implementation.packetAdapter.modern.util.ViaConnectionGuard;
 import net.megavex.scoreboardlibrary.implementation.packetAdapter.team.EntriesPacketType;
 import net.megavex.scoreboardlibrary.implementation.packetAdapter.team.TeamConstants;
 import net.megavex.scoreboardlibrary.implementation.packetAdapter.team.TeamDisplayPacketAdapter;
@@ -45,14 +47,14 @@ public void removeTeam(@NotNull Iterable<Player> players) {
     assert via != null;
     for (final Player player : players) {
       final UserConnection conn = via.getConnection(player.getUniqueId());
-      if (conn == null) continue;
+      if (conn == null || !ViaConnectionGuard.isCurrentPlayConnection(via, player, conn)) continue;
 
       final ByteBuf buf = Unpooled.buffer(128);
       Types.VAR_INT.writePrimitive(buf, teamsPacketId(player, conn));
       Types.STRING.write(buf, teamName);
       Types.BYTE.writePrimitive(buf, (byte) TeamConstants.MODE_REMOVE);
 
-      via.sendRawPacket(player.getUniqueId(), buf);
+      sendRawPacket(via, player, conn, buf);
     }
   }
 
@@ -62,7 +64,7 @@ public void sendEntries(@NotNull EntriesPacketType packetType, @NotNull Collecti
     assert via != null;
     for (final Player player : players) {
       final UserConnection conn = via.getConnection(player.getUniqueId());
-      if (conn == null) continue;
+      if (conn == null || !ViaConnectionGuard.isCurrentPlayConnection(via, player, conn)) continue;
 
       final ByteBuf buf = Unpooled.buffer(128);
       Types.VAR_INT.writePrimitive(buf, teamsPacketId(player, conn));
@@ -81,7 +83,7 @@ public void sendEntries(@NotNull EntriesPacketType packetType, @NotNull Collecti
         Types.STRING.write(buf, entry);
       }
 
-      via.sendRawPacket(player.getUniqueId(), buf);
+      sendRawPacket(via, player, conn, buf);
     }
   }
 
@@ -91,7 +93,7 @@ public void sendProperties(@NotNull PropertiesPacketType packetType, @NotNull Co
     assert via != null;
     for (final Player player : players) {
       final UserConnection conn = via.getConnection(player.getUniqueId());
-      if (conn == null) continue;
+      if (conn == null || !ViaConnectionGuard.isCurrentPlayConnection(via, player, conn)) continue;
 
       final ByteBuf buf = Unpooled.buffer(128);
       Types.VAR_INT.writePrimitive(buf, teamsPacketId(player, conn));
@@ -134,7 +136,28 @@ public void sendProperties(@NotNull PropertiesPacketType packetType, @NotNull Co
         }
       }
 
-      via.sendRawPacket(player.getUniqueId(), buf);
+      sendRawPacket(via, player, conn, buf);
+    }
+  }
+
+  private static void sendRawPacket(ViaAPI<Player> via, Player player, UserConnection connection, ByteBuf packet) {
+    final Channel channel = connection.getChannel();
+    if (!ViaConnectionGuard.isCurrentPlayConnection(via, player, connection, channel)) {
+      packet.release();
+      return;
+    }
+
+    try {
+      channel.eventLoop().execute(() -> {
+        if (ViaConnectionGuard.isCurrentPlayConnection(via, player, connection, channel)) {
+          connection.sendRawPacket(packet);
+        } else {
+          packet.release();
+        }
+      });
+    } catch (RuntimeException | Error exception) {
+      packet.release();
+      throw exception;
     }
   }
 

implementation/src/main/java/net/megavex/scoreboardlibrary/implementation/packetAdapter/modern/util/ModernPacketSender.java

@@ -82,16 +82,25 @@ public void sendPacket(Player player, Object packet) {
       final UserConnection conn = this.via.getConnection(player.getUniqueId());
       if (conn != null) {
         final Channel channel = conn.getChannel();
-        if (channel != null) {
-          // Paper has some "network optimization" patch that makes the NMS sendPacket method
-          // not always send the packet immediately to the player's netty channel,
-          // and this is a problem when we send some packets using ViaVersion API because it always sends
-          // the packet to the channel immediately, so the packet order between NMS and ViaVersion API can
-          // get messed up. So for ViaVersion players we send the packet directly to the player's channel
-          // bypassing Paper logic to work around this. Fortunately ViaVersion provides easy access to the player's channel.
-          channel.eventLoop().execute(() -> channel.writeAndFlush(packet));
+        if (!ViaConnectionGuard.isCurrentPlayConnection(this.via, player, conn, channel)) {
           return;
         }
+
+        // Paper has some "network optimization" patch that makes the NMS sendPacket method
+        // not always send the packet immediately to the player's netty channel,
+        // and this is a problem when we send some packets using ViaVersion API because it always sends
+        // the packet to the channel immediately, so the packet order between NMS and ViaVersion API can
+        // get messed up. So for ViaVersion players we send the packet directly to the player's channel
+        // bypassing Paper logic to work around this. Fortunately ViaVersion provides easy access to the player's channel.
+        channel.eventLoop().execute(() -> {
+          // The player can disconnect or enter configuration while this write is queued. The
+          // channel remains open in that state, but its encoder no longer accepts game packets.
+          // Also guard against a reconnect replacing ViaVersion's UUID mapping in the meantime.
+          if (ViaConnectionGuard.isCurrentPlayConnection(this.via, player, conn, channel)) {
+            channel.writeAndFlush(packet);
+          }
+        });
+        return;
       }
     }
 

implementation/src/main/java/net/megavex/scoreboardlibrary/implementation/packetAdapter/modern/util/ViaConnectionGuard.java

@@ -0,0 +1,38 @@
+package net.megavex.scoreboardlibrary.implementation.packetAdapter.modern.util;
+
+import com.viaversion.viaversion.api.ViaAPI;
+import com.viaversion.viaversion.api.connection.ProtocolInfo;
+import com.viaversion.viaversion.api.connection.UserConnection;
+import com.viaversion.viaversion.api.protocol.packet.State;
+import io.netty.channel.Channel;
+import org.bukkit.entity.Player;
+
+public final class ViaConnectionGuard {
+  private ViaConnectionGuard() {
+  }
+
+  public static boolean isCurrentPlayConnection(
+    ViaAPI<Player> via,
+    Player player,
+    UserConnection connection
+  ) {
+    return isCurrentPlayConnection(via, player, connection, connection.getChannel());
+  }
+
+  public static boolean isCurrentPlayConnection(
+    ViaAPI<Player> via,
+    Player player,
+    UserConnection connection,
+    Channel channel
+  ) {
+    final ProtocolInfo protocolInfo = connection.getProtocolInfo();
+    return channel != null
+      && player.isOnline()
+      && channel.isActive()
+      && !connection.isPendingDisconnect()
+      && protocolInfo != null
+      && protocolInfo.getServerState() == State.PLAY
+      && protocolInfo.getClientState() == State.PLAY
+      && via.getConnection(player.getUniqueId()) == connection;
+  }
+}