ConcurrentModificationException in PacketListenerSet

[Protonull]

• [x] This issue is not solved in a development build

Describe the bug Throws a ConcurrentModificationException during listener registration.

[20:11:37 WARN]: [PacketPrinter] Task #5 for PacketPrinter v1.0-SNAPSHOT generated an exception
java.util.ConcurrentModificationException: null
    at java.base/java.util.HashMap$HashIterator.nextNode(HashMap.java:1605) ~[?:?]
    at java.base/java.util.HashMap$KeyIterator.next(HashMap.java:1628) ~[?:?]
    at ProtocolLib-5.3.0-SNAPSHOT.jar/com.comphenix.protocol.injector.collection.PacketListenerSet.addListener(PacketListenerSet.java:48) ~[ProtocolLib-5.3.0-SNAPSHOT.jar:?]
    at ProtocolLib-5.3.0-SNAPSHOT.jar/com.comphenix.protocol.injector.PacketFilterManager.addPacketListener(PacketFilterManager.java:313) ~[ProtocolLib-5.3.0-SNAPSHOT.jar:?]
    at PacketPrinter-1.0-SNAPSHOT.jar/uk.protonull.packetPrinter.PacketPrinter.lambda$onEnable$0(PacketPrinter.java:14) ~[PacketPrinter-1.0-SNAPSHOT.jar:?]
    at org.bukkit.craftbukkit.scheduler.CraftTask.run(CraftTask.java:86) ~[paper-1.21.1.jar:1.21.1-52-e08e667]
    at org.bukkit.craftbukkit.scheduler.CraftScheduler.mainThreadHeartbeat(CraftScheduler.java:475) ~[paper-1.21.1.jar:1.21.1-52-e08e667]
    at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:1226) ~[paper-1.21.1.jar:1.21.1-52-e08e667]
    at net.minecraft.server.MinecraftServer.lambda$spin$0(MinecraftServer.java:330) ~[paper-1.21.1.jar:1.21.1-52-e08e667]
    at java.base/java.lang.Thread.run(Thread.java:1583) ~[?:?]

The bug is caused by a .remove() call within a for-loop (link is anchored to current-latest commit, so no latest build fixes this). This could be fixed with replacing the short for-loop style with an explicit iterator.

---

To Reproduce Was only trying to create a listener that printed out all sent and received messages, this is the extent of the code:

ProtocolLibrary.getProtocolManager().addPacketListener(new PacketAdapter(
    this,
    PacketType.values()
) {
    @Override
    public void onPacketSending(PacketEvent event) {
        getLogger().info("Packet sent: [" + event.getPacketType() + "]: " + event.getPacket());
    }
    @Override
    public void onPacketReceiving(PacketEvent event) {
        getLogger().info("Packet received: [" + event.getPacketType() + "]: " + event.getPacket());
    }
});

---

Expected behaviour That it registers without throwing, particularly since PacketType.values() is so readily available that it seems like the intent is to be usable in this way.

---

Screenshots N/A

---

Version Info dump-2024-09-17_20.23.11.txt

TL;DO:

[20:11:22 INFO]: [bootstrap] Running Java 21 (OpenJDK 64-Bit Server VM 21.0.4+7-LTS; Amazon.com Inc. Corretto-21.0.4.7.1) on Linux 6.1.0-25-amd64 (amd64)
[20:11:22 INFO]: [bootstrap] Loading Paper 1.21.1-52-master@e08e667 (2024-08-26T18:02:06Z) for Minecraft 1.21.1
[20:11:33 INFO]: [ProtocolLib] Loading server plugin ProtocolLib v5.3.0-SNAPSHOT-726
[20:11:33 WARN]: [ProtocolLib] Version (MC: 1.21.1) has not yet been tested! Proceed with caution.

---

Additional context N/A

[BjornTheProgrammer]

Facing the same issue for 1.20.1, with the latest dev build. And almost the same code. Note that I'm intending to listen to every single packet for my use case.

ProtocolManager manager = ProtocolLibrary.getProtocolManager();
manager.addPacketListener(new PacketAdapter(this, PacketType.values()) {
    @Override
    public void onPacketReceiving(PacketEvent event) {
        try {
            PacketType packetType = event.getPacket().getType();
            System.out.println("[PACKET RECIEVED] " + packetType.toString());
        } catch(Exception e) {
            System.out.println("[PACKET READ FAIL]" + e.toString());
        }
    }

    @Override
    public void onPacketSending(PacketEvent event) {
        try {
            PacketType packetType = event.getPacket().getType();
            System.out.println("[PACKET SENT] " + packetType.toString());
        } catch(Exception e) {
            System.out.println("[PACKET READ FAIL]" + e.toString());
        }
    }
});

[01:56:51 WARN]: [ProtocolLib] Error filtering reports: java.lang.IllegalArgumentException: Cannot find report name for Plugin %s tried to register listener for unknown packet %s [direction: from %s]
[01:56:51 WARN]: [ProtocolLib] [OutboundPacketListenerSet] Plugin ListenerPlugin tried to register listener for unknown packet CUSTOM_SOUND_EFFECT[PLAY, SERVER, 237, classNames: [net.minecraft.network.protocol.game.PacketPlayOutCustomSoundEffect, net.minecraft.network.protocol.game.ClientboundCustomSoundEffectPacket, PacketPlayOutCustomSoundEffect, ClientboundCustomSoundEffectPacket, CustomSoundEffect, net.minecraft.network.play.server.SPacketCustomSound] (unregistered)] [direction: from SERVER]
[01:56:51 ERROR]: Error occurred while enabling ListenerPlugin v1.0-SNAPSHOT (Is it up to date?)
java.util.ConcurrentModificationException: null
    at java.util.HashMap$HashIterator.nextNode(HashMap.java:1605) ~[?:?]
    at java.util.HashMap$KeyIterator.next(HashMap.java:1628) ~[?:?]
    at com.comphenix.protocol.injector.collection.PacketListenerSet.addListener(PacketListenerSet.java:48) ~[ProtocolLib.jar:?]
    at com.comphenix.protocol.injector.PacketFilterManager.addPacketListener(PacketFilterManager.java:313) ~[ProtocolLib.jar:?]
    at org.listener.listenerplugin.ListenerPlugin.onEnable(ListenerPlugin.java:27) ~[ListenerPlugin-1.0-SNAPSHOT.jar:?]
    at org.bukkit.plugin.java.JavaPlugin.setEnabled(JavaPlugin.java:281) ~[paper-api-1.20.1-R0.1-SNAPSHOT.jar:?]
    at io.papermc.paper.plugin.manager.PaperPluginInstanceManager.enablePlugin(PaperPluginInstanceManager.java:189) ~[paper-1.20.1.jar:git-Paper-196]
    at io.papermc.paper.plugin.manager.PaperPluginManagerImpl.enablePlugin(PaperPluginManagerImpl.java:104) ~[paper-1.20.1.jar:git-Paper-196]
    at org.bukkit.plugin.SimplePluginManager.enablePlugin(SimplePluginManager.java:507) ~[paper-api-1.20.1-R0.1-SNAPSHOT.jar:?]
    at org.bukkit.craftbukkit.v1_20_R1.CraftServer.enablePlugin(CraftServer.java:642) ~[paper-1.20.1.jar:git-Paper-196]
    at org.bukkit.craftbukkit.v1_20_R1.CraftServer.enablePlugins(CraftServer.java:553) ~[paper-1.20.1.jar:git-Paper-196]
    at net.minecraft.server.MinecraftServer.loadWorld0(MinecraftServer.java:635) ~[paper-1.20.1.jar:git-Paper-196]
    at net.minecraft.server.MinecraftServer.loadLevel(MinecraftServer.java:434) ~[paper-1.20.1.jar:git-Paper-196]
    at net.minecraft.server.dedicated.DedicatedServer.initServer(DedicatedServer.java:308) ~[paper-1.20.1.jar:git-Paper-196]
    at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:1100) ~[paper-1.20.1.jar:git-Paper-196]
    at net.minecraft.server.MinecraftServer.lambda$spin$0(MinecraftServer.java:317) ~[paper-1.20.1.jar:git-Paper-196]
    at java.lang.Thread.run(Thread.java:1570) ~[?:?]

[BjornTheProgrammer]

Created pull request #3241 which fixes the issue.

[BjornTheProgrammer]

The pull request has been merged, this should be fixed!