From 230eb630f333f4d66968f6f267b64c5f335f187c Mon Sep 17 00:00:00 2001 From: Florian Stober Date: Sat, 5 Aug 2023 11:30:34 +0200 Subject: [PATCH 1/7] remove synchronized from data access; fixes (maybe) #724 --- .../bridge/AbstractBridge.java | 137 +++++++++--------- 1 file changed, 67 insertions(+), 70 deletions(-) diff --git a/bridge/src/main/java/de/codecrafter47/bungeetablistplus/bridge/AbstractBridge.java b/bridge/src/main/java/de/codecrafter47/bungeetablistplus/bridge/AbstractBridge.java index 836b0901..9b538bcf 100644 --- a/bridge/src/main/java/de/codecrafter47/bungeetablistplus/bridge/AbstractBridge.java +++ b/bridge/src/main/java/de/codecrafter47/bungeetablistplus/bridge/AbstractBridge.java @@ -289,64 +289,64 @@ public void resendUnconfirmedMessages() { } public void updateData(boolean isMainThread) throws IOException { - synchronized (updateDataLock) { - Map proxyIds = new HashMap<>(); + Map proxyIds = new HashMap<>(); - for (Map.Entry e : playerPlayerConnectionInfoMap.entrySet()) { - Player player = e.getKey(); - PlayerConnectionInfo connectionInfo = e.getValue(); + for (Map.Entry e : playerPlayerConnectionInfoMap.entrySet()) { + Player player = e.getKey(); + PlayerConnectionInfo connectionInfo = e.getValue(); - if (!connectionInfo.isConnectionValid) { - continue; - } + if (!connectionInfo.isConnectionValid) { + continue; + } - proxyIds.putIfAbsent(connectionInfo.proxyIdentifier, player); + proxyIds.putIfAbsent(connectionInfo.proxyIdentifier, player); - updatePlayerData(player, connectionInfo, isMainThread); - } + updatePlayerData(player, connectionInfo, isMainThread); + } - for (Map.Entry e : proxyIds.entrySet()) { - Integer proxyIdentifier = e.getKey(); - Player player = e.getValue(); - BridgeData bridgeData = serverBridgeDataMap.get(proxyIdentifier); + ArrayList dirtyEntries = new ArrayList<>(); - if (bridgeData == null) { - continue; - } + for (Map.Entry e : proxyIds.entrySet()) { + dirtyEntries.clear(); + Integer proxyIdentifier = e.getKey(); + Player player = e.getValue(); + BridgeData bridgeData = serverBridgeDataMap.get(proxyIdentifier); - int size = 0; + if (bridgeData == null) { + continue; + } - for (CacheEntry entry : bridgeData.requestedData) { - DataKey key = entry.key; - if (requiresMainThread(key) == isMainThread) { - Object value = serverDataAccess.get(key, server); - entry.dirty = !Objects.equals(value, entry.value); + for (CacheEntry entry : bridgeData.requestedData) { + DataKey key = entry.key; + if (requiresMainThread(key) == isMainThread) { + Object value = serverDataAccess.get(key, server); + synchronized (entry) { + boolean dirty = !Objects.equals(value, entry.value); entry.value = value; - if (entry.dirty) { - size++; + if (dirty) { + dirtyEntries.add(entry); } } } + } + synchronized (updateDataLock) { ByteArrayOutputStream byteArrayOutput = new ByteArrayOutputStream(); DataOutput output = new DataOutputStream(byteArrayOutput); - output.writeByte(BridgeProtocolConstants.MESSAGE_ID_UPDATE_DATA_SERVER); output.writeInt(proxyIdentifier + serverIdentifier); output.writeInt(bridgeData.nextOutgoingMessageId++); - output.writeInt(size); - - for (CacheEntry entry : bridgeData.requestedData) { - if (entry.dirty) { - output.writeInt(entry.netId); - output.writeBoolean(entry.value == null); - if (entry.value != null) { - try { - typeAdapterRegistry.getTypeAdapter((TypeToken) entry.key.getType()).write(output, entry.value); - } catch (IOException e1) { - e1.printStackTrace(); - } + output.writeInt(dirtyEntries.size()); + + for (CacheEntry entry : dirtyEntries) { + output.writeInt(entry.netId); + output.writeBoolean(entry.value == null); + if (entry.value != null) { + try { + typeAdapterRegistry.getTypeAdapter((TypeToken) entry.key.getType()).write(output, entry.value); + } catch (IOException e1) { + e1.printStackTrace(); } } } @@ -360,46 +360,44 @@ public void updateData(boolean isMainThread) throws IOException { } private void updatePlayerData(@Nonnull Player player, @Nonnull PlayerConnectionInfo connectionInfo, boolean isMainThread) throws IOException { - synchronized (updateDataLock) { - BridgeData bridgeData = connectionInfo.playerBridgeData; + BridgeData bridgeData = connectionInfo.playerBridgeData; - if (bridgeData == null) { - return; - } + if (bridgeData == null) { + return; + } - int size = 0; + ArrayList dirtyEntries = new ArrayList<>(); - for (CacheEntry entry : bridgeData.requestedData) { - DataKey key = entry.key; - if (requiresMainThread(key) == isMainThread) { - Object value = playerDataAccess.get(key, player); - entry.dirty = !Objects.equals(value, entry.value); - entry.value = value; + for (CacheEntry entry : bridgeData.requestedData) { + DataKey key = entry.key; + if (requiresMainThread(key) == isMainThread) { + Object value = playerDataAccess.get(key, player); + boolean dirty = !Objects.equals(value, entry.value); + entry.value = value; - if (entry.dirty) { - size++; - } + if (dirty) { + dirtyEntries.add(entry); } } + } - if (size != 0) { + if (!dirtyEntries.isEmpty()) { + synchronized (updateDataLock) { ByteArrayOutputStream byteArrayOutput = new ByteArrayOutputStream(); DataOutput output = new DataOutputStream(byteArrayOutput); output.writeByte(BridgeProtocolConstants.MESSAGE_ID_UPDATE_DATA); output.writeInt(connectionInfo.connectionIdentifier); output.writeInt(bridgeData.nextOutgoingMessageId++); - output.writeInt(size); - - for (CacheEntry entry : bridgeData.requestedData) { - if (entry.dirty) { - output.writeInt(entry.netId); - output.writeBoolean(entry.value == null); - if (entry.value != null) { - try { - typeAdapterRegistry.getTypeAdapter((TypeToken) entry.key.getType()).write(output, entry.value); - } catch (IOException e1) { - e1.printStackTrace(); - } + output.writeInt(dirtyEntries.size()); + + for (CacheEntry entry : dirtyEntries) { + output.writeInt(entry.netId); + output.writeBoolean(entry.value == null); + if (entry.value != null) { + try { + typeAdapterRegistry.getTypeAdapter((TypeToken) entry.key.getType()).write(output, entry.value); + } catch (IOException e1) { + e1.printStackTrace(); } } } @@ -423,7 +421,7 @@ public void setServerDataAccess(@Nonnull DataAccess serverDataAccess) { protected abstract void sendMessage(@Nonnull Player player, @Nonnull byte[] message); protected abstract void runAsync(@Nonnull Runnable task); - + protected abstract boolean requiresMainThread(@Nonnull DataKey key); private static class PlayerConnectionInfo { @@ -446,7 +444,6 @@ private static class CacheEntry { final int netId; @Nullable Object value = null; - boolean dirty = false; CacheEntry(@Nonnull DataKey key, int netId) { this.key = key; @@ -463,7 +460,7 @@ private static class BridgeData { long lastMessageSent = 0; /*** - * + * * @param key * @param netId * @return true if a new entry has been created From e1ef1b7586c8b81abc82d315a9ee023f3dbe8eee Mon Sep 17 00:00:00 2001 From: Joshua Castle <26531652+Kas-tle@users.noreply.github.com> Date: Sat, 30 Sep 2023 22:06:11 -0700 Subject: [PATCH 2/7] 1.20.2 workaround Signed-off-by: Joshua Castle <26531652+Kas-tle@users.noreply.github.com> --- .../handler/NewTabOverlayHandler.java | 11 +++++++++ .../util/ReflectionUtil.java | 24 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/bungee/src/main/java/codecrafter47/bungeetablistplus/handler/NewTabOverlayHandler.java b/bungee/src/main/java/codecrafter47/bungeetablistplus/handler/NewTabOverlayHandler.java index eba4958b..2a90a3ff 100644 --- a/bungee/src/main/java/codecrafter47/bungeetablistplus/handler/NewTabOverlayHandler.java +++ b/bungee/src/main/java/codecrafter47/bungeetablistplus/handler/NewTabOverlayHandler.java @@ -23,6 +23,7 @@ import codecrafter47.bungeetablistplus.util.BitSet; import codecrafter47.bungeetablistplus.util.ConcurrentBitSet; import codecrafter47.bungeetablistplus.util.Property119Handler; +import codecrafter47.bungeetablistplus.util.ReflectionUtil; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -39,6 +40,7 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +import java.lang.reflect.InvocationTargetException; import java.util.*; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.Executor; @@ -181,6 +183,15 @@ private void sendPacket(DefinedPacket packet) { logVersionMismatch = true; logger.warning("Cannot correctly update tablist for player " + player.getName() + "\nThe client and server versions do not match. Client >= 1.19.3, server < 1.19.3.\nUse ViaVersion on the spigot server for the best experience."); } + } else if (player.getPendingConnection().getVersion() >= 764) { + try { + // Ensure that unsafe packets are not sent in the config phase + // Why bungee doesn't expose this is beyond me... + // https://github.com/SpigotMC/BungeeCord/blob/1ef4d27dbea48a1d47501ad2be0d75e42cc2cc12/proxy/src/main/java/net/md_5/bungee/UserConnection.java#L182-L192 + ReflectionUtil.sendPacketQueued(player, packet); + } catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException | SecurityException e) { + logger.warning("Failed to queue packet for player " + player.getName() + ": " + e); + } } else { player.unsafe().sendPacket(packet); } diff --git a/bungee/src/main/java/codecrafter47/bungeetablistplus/util/ReflectionUtil.java b/bungee/src/main/java/codecrafter47/bungeetablistplus/util/ReflectionUtil.java index 78d08607..3f96bbae 100644 --- a/bungee/src/main/java/codecrafter47/bungeetablistplus/util/ReflectionUtil.java +++ b/bungee/src/main/java/codecrafter47/bungeetablistplus/util/ReflectionUtil.java @@ -20,11 +20,20 @@ import net.md_5.bungee.UserConnection; import net.md_5.bungee.api.connection.ProxiedPlayer; import net.md_5.bungee.netty.ChannelWrapper; +import net.md_5.bungee.protocol.DefinedPacket; import net.md_5.bungee.tab.TabList; import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; public class ReflectionUtil { + public static final Method SEND_PACKET_QUEUED; + + static { + SEND_PACKET_QUEUED = getMethodSilent(UserConnection.class, "sendPacketQueued", DefinedPacket.class); + } + public static void setTablistHandler(ProxiedPlayer player, TabList tablistHandler) throws NoSuchFieldException, IllegalAccessException { setField(UserConnection.class, player, "tabListHandler", tablistHandler, 5); } @@ -37,6 +46,10 @@ public static ChannelWrapper getChannelWrapper(ProxiedPlayer player) throws NoSu return getField(UserConnection.class, player, "ch", 50); } + public static void sendPacketQueued(ProxiedPlayer player, Object packet) throws IllegalAccessException, IllegalArgumentException, InvocationTargetException { + SEND_PACKET_QUEUED.invoke(player, packet); + } + public static void setField(Class clazz, Object instance, String field, Object value) throws NoSuchFieldException, IllegalAccessException { Field f = clazz.getDeclaredField(field); f.setAccessible(true); @@ -70,4 +83,15 @@ public static T getField(Class clazz, Object instance, String field, int } return getField(clazz, instance, field); } + + public static Method getMethodSilent(Class clazz, String method, Class... parameterTypes) { + Method m; + try { + m = clazz.getDeclaredMethod(method, parameterTypes); + } catch (NoSuchMethodException | SecurityException e) { + return null; + } + m.setAccessible(true); + return m; + } } From eb7f0353e864d21bbd90147a52ce55022c5a52e9 Mon Sep 17 00:00:00 2001 From: Joshua Castle <26531652+Kas-tle@users.noreply.github.com> Date: Sat, 30 Sep 2023 22:17:09 -0700 Subject: [PATCH 3/7] Build artifacts Signed-off-by: Joshua Castle <26531652+Kas-tle@users.noreply.github.com> --- .github/workflows/build.yml | 52 +++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 .github/workflows/build.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml new file mode 100644 index 00000000..a856063b --- /dev/null +++ b/.github/workflows/build.yml @@ -0,0 +1,52 @@ +name: Build With Gradle + +on: [ push, pull_request ] + +jobs: + build: + + runs-on: ubuntu-latest + + steps: + - name: Checkout repository and submodules + uses: actions/checkout@v3 + with: + submodules: recursive + + - name: Set up JDK 16 + uses: actions/setup-java@v2 + with: + distribution: 'temurin' + java-version: '16' + cache: 'gradle' + + - name: Build with Gradle + run: ./gradlew shadowJar + + - name: Archive artifacts (Bungee) + uses: actions/upload-artifact@v2 + if: success() + with: + name: BungeeTabListPlus Bungee + path: bootstrap-bungee/build/libs/BungeeTabListPlus-*-SNAPSHOT.jar + + - name: Archive artifacts (Bukkit) + uses: actions/upload-artifact@v2 + if: success() + with: + name: BungeeTabListPlus Bukkit Bridge + path: bootstrap-bukkit/build/libs/BungeeTabListPlus_BukkitBridge-*-SNAPSHOT.jar + + - name: Archive artifacts (Fabric 1.16.3) + uses: actions/upload-artifact@v2 + if: success() + with: + name: BungeeTabListPlus Fabric 1.16.3 Bridge + path: fabric-bridge-1.16.3/build/libs/btlp-fabric-bridge-*-SNAPSHOT.jar + + - name: Archive artifacts (Fabric 1.17) + uses: actions/upload-artifact@v2 + if: success() + with: + name: BungeeTabListPlus Fabric 1.17 Bridge + path: fabric-bridge-1.17/build/libs/btlp-fabric-bridge-*-SNAPSHOT.jar \ No newline at end of file From 2580622064c95b7f52af74e77045febdf2d750e4 Mon Sep 17 00:00:00 2001 From: Joshua Castle <26531652+Kas-tle@users.noreply.github.com> Date: Sat, 30 Sep 2023 22:57:14 -0700 Subject: [PATCH 4/7] Ignore InvocationTargetException as disconnect Signed-off-by: Joshua Castle <26531652+Kas-tle@users.noreply.github.com> --- .../bungeetablistplus/handler/NewTabOverlayHandler.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bungee/src/main/java/codecrafter47/bungeetablistplus/handler/NewTabOverlayHandler.java b/bungee/src/main/java/codecrafter47/bungeetablistplus/handler/NewTabOverlayHandler.java index 2a90a3ff..02af63b5 100644 --- a/bungee/src/main/java/codecrafter47/bungeetablistplus/handler/NewTabOverlayHandler.java +++ b/bungee/src/main/java/codecrafter47/bungeetablistplus/handler/NewTabOverlayHandler.java @@ -189,7 +189,9 @@ private void sendPacket(DefinedPacket packet) { // Why bungee doesn't expose this is beyond me... // https://github.com/SpigotMC/BungeeCord/blob/1ef4d27dbea48a1d47501ad2be0d75e42cc2cc12/proxy/src/main/java/net/md_5/bungee/UserConnection.java#L182-L192 ReflectionUtil.sendPacketQueued(player, packet); - } catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException | SecurityException e) { + } catch (InvocationTargetException e) { + // Ignore as this likely means the player has disconnected + } catch (IllegalAccessException | IllegalArgumentException | SecurityException e) { logger.warning("Failed to queue packet for player " + player.getName() + ": " + e); } } else { From b09e3a6aeb66c5d2759fa0728e62e89ae6388017 Mon Sep 17 00:00:00 2001 From: Joshua Castle <26531652+Kas-tle@users.noreply.github.com> Date: Sat, 30 Sep 2023 23:51:05 -0700 Subject: [PATCH 5/7] Reflect not needed if we bump bungee version Signed-off-by: Joshua Castle <26531652+Kas-tle@users.noreply.github.com> --- .../handler/NewTabOverlayHandler.java | 16 ++++--------- .../util/ReflectionUtil.java | 24 ------------------- 2 files changed, 5 insertions(+), 35 deletions(-) diff --git a/bungee/src/main/java/codecrafter47/bungeetablistplus/handler/NewTabOverlayHandler.java b/bungee/src/main/java/codecrafter47/bungeetablistplus/handler/NewTabOverlayHandler.java index 02af63b5..7247a0b7 100644 --- a/bungee/src/main/java/codecrafter47/bungeetablistplus/handler/NewTabOverlayHandler.java +++ b/bungee/src/main/java/codecrafter47/bungeetablistplus/handler/NewTabOverlayHandler.java @@ -23,7 +23,6 @@ import codecrafter47.bungeetablistplus.util.BitSet; import codecrafter47.bungeetablistplus.util.ConcurrentBitSet; import codecrafter47.bungeetablistplus.util.Property119Handler; -import codecrafter47.bungeetablistplus.util.ReflectionUtil; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -34,6 +33,7 @@ import de.codecrafter47.taboverlay.handler.*; import it.unimi.dsi.fastutil.objects.*; import lombok.*; +import net.md_5.bungee.UserConnection; import net.md_5.bungee.api.connection.ProxiedPlayer; import net.md_5.bungee.protocol.DefinedPacket; import net.md_5.bungee.protocol.packet.*; @@ -184,16 +184,10 @@ private void sendPacket(DefinedPacket packet) { logger.warning("Cannot correctly update tablist for player " + player.getName() + "\nThe client and server versions do not match. Client >= 1.19.3, server < 1.19.3.\nUse ViaVersion on the spigot server for the best experience."); } } else if (player.getPendingConnection().getVersion() >= 764) { - try { - // Ensure that unsafe packets are not sent in the config phase - // Why bungee doesn't expose this is beyond me... - // https://github.com/SpigotMC/BungeeCord/blob/1ef4d27dbea48a1d47501ad2be0d75e42cc2cc12/proxy/src/main/java/net/md_5/bungee/UserConnection.java#L182-L192 - ReflectionUtil.sendPacketQueued(player, packet); - } catch (InvocationTargetException e) { - // Ignore as this likely means the player has disconnected - } catch (IllegalAccessException | IllegalArgumentException | SecurityException e) { - logger.warning("Failed to queue packet for player " + player.getName() + ": " + e); - } + // Ensure that unsafe packets are not sent in the config phase + // Why bungee doesn't expose this via api beyond me... + // https://github.com/SpigotMC/BungeeCord/blob/1ef4d27dbea48a1d47501ad2be0d75e42cc2cc12/proxy/src/main/java/net/md_5/bungee/UserConnection.java#L182-L192 + ((UserConnection) player).sendPacketQueued(packet); } else { player.unsafe().sendPacket(packet); } diff --git a/bungee/src/main/java/codecrafter47/bungeetablistplus/util/ReflectionUtil.java b/bungee/src/main/java/codecrafter47/bungeetablistplus/util/ReflectionUtil.java index 3f96bbae..78d08607 100644 --- a/bungee/src/main/java/codecrafter47/bungeetablistplus/util/ReflectionUtil.java +++ b/bungee/src/main/java/codecrafter47/bungeetablistplus/util/ReflectionUtil.java @@ -20,20 +20,11 @@ import net.md_5.bungee.UserConnection; import net.md_5.bungee.api.connection.ProxiedPlayer; import net.md_5.bungee.netty.ChannelWrapper; -import net.md_5.bungee.protocol.DefinedPacket; import net.md_5.bungee.tab.TabList; import java.lang.reflect.Field; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; public class ReflectionUtil { - public static final Method SEND_PACKET_QUEUED; - - static { - SEND_PACKET_QUEUED = getMethodSilent(UserConnection.class, "sendPacketQueued", DefinedPacket.class); - } - public static void setTablistHandler(ProxiedPlayer player, TabList tablistHandler) throws NoSuchFieldException, IllegalAccessException { setField(UserConnection.class, player, "tabListHandler", tablistHandler, 5); } @@ -46,10 +37,6 @@ public static ChannelWrapper getChannelWrapper(ProxiedPlayer player) throws NoSu return getField(UserConnection.class, player, "ch", 50); } - public static void sendPacketQueued(ProxiedPlayer player, Object packet) throws IllegalAccessException, IllegalArgumentException, InvocationTargetException { - SEND_PACKET_QUEUED.invoke(player, packet); - } - public static void setField(Class clazz, Object instance, String field, Object value) throws NoSuchFieldException, IllegalAccessException { Field f = clazz.getDeclaredField(field); f.setAccessible(true); @@ -83,15 +70,4 @@ public static T getField(Class clazz, Object instance, String field, int } return getField(clazz, instance, field); } - - public static Method getMethodSilent(Class clazz, String method, Class... parameterTypes) { - Method m; - try { - m = clazz.getDeclaredMethod(method, parameterTypes); - } catch (NoSuchMethodException | SecurityException e) { - return null; - } - m.setAccessible(true); - return m; - } } From 9c20a0fa4df91bb46e864d8fb3c97c0e00f5d399 Mon Sep 17 00:00:00 2001 From: Joshua Castle <26531652+Kas-tle@users.noreply.github.com> Date: Sat, 30 Sep 2023 23:53:37 -0700 Subject: [PATCH 6/7] Unused import Signed-off-by: Joshua Castle <26531652+Kas-tle@users.noreply.github.com> --- .../bungeetablistplus/handler/NewTabOverlayHandler.java | 1 - 1 file changed, 1 deletion(-) diff --git a/bungee/src/main/java/codecrafter47/bungeetablistplus/handler/NewTabOverlayHandler.java b/bungee/src/main/java/codecrafter47/bungeetablistplus/handler/NewTabOverlayHandler.java index 7247a0b7..d3e271b9 100644 --- a/bungee/src/main/java/codecrafter47/bungeetablistplus/handler/NewTabOverlayHandler.java +++ b/bungee/src/main/java/codecrafter47/bungeetablistplus/handler/NewTabOverlayHandler.java @@ -40,7 +40,6 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; -import java.lang.reflect.InvocationTargetException; import java.util.*; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.Executor; From e55986e8f2f4775016c522749df882e5c30ccad4 Mon Sep 17 00:00:00 2001 From: Joshua Castle <26531652+Kas-tle@users.noreply.github.com> Date: Sun, 1 Oct 2023 00:14:33 -0700 Subject: [PATCH 7/7] Bump bungee Signed-off-by: Joshua Castle <26531652+Kas-tle@users.noreply.github.com> --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 68e3e1a1..6c9cc231 100644 --- a/build.gradle +++ b/build.gradle @@ -10,7 +10,7 @@ buildscript { ext { spigotVersion = '1.11-R0.1-SNAPSHOT' - bungeeVersion = '1.19-R0.1-SNAPSHOT' + bungeeVersion = '1.20-R0.2-SNAPSHOT' spongeVersion = '7.0.0' dataApiVersion = '1.0.2-SNAPSHOT' }