Skip to content

Fix ConcurrentModificationException when shutting down async listeners#3169

Merged
dmulloy2 merged 2 commits intodmulloy2:masterfrom
SzczurekYT:master
Sep 23, 2024
Merged

Fix ConcurrentModificationException when shutting down async listeners#3169
dmulloy2 merged 2 commits intodmulloy2:masterfrom
SzczurekYT:master

Conversation

@SzczurekYT
Copy link
Copy Markdown
Contributor

@SzczurekYT SzczurekYT commented Aug 4, 2024

This makes a copy of the list before iterating over it so that ConcurrentModificationException isn't thrown

For example the FastLogin plugin calls unregisterAsyncHandlers like this in their onDisabled method

if (getServer().getPluginManager().isPluginEnabled("ProtocolLib")) {
            ProtocolLibrary.getProtocolManager().getAsynchronousManager().unregisterAsyncHandlers(this);
        }

which resulted in this error before the fix

[11:02:00 ERROR]: Error occurred while disabling FastLogin v1.12-SNAPSHOT-4dd6b9a
java.util.ConcurrentModificationException: null
        at java.base/java.util.HashMap$HashIterator.nextNode(HashMap.java:1605) ~[?:?]
        at java.base/java.util.HashMap$ValueIterator.next(HashMap.java:1633) ~[?:?]
        at com.google.common.collect.TransformedIterator.next(TransformedIterator.java:52) ~[guava-32.1.2-jre.jar:?]
        at com.google.common.collect.Iterators$ConcatenatedIterator.hasNext(Iterators.java:1400) ~[guava-32.1.2-jre.jar:?]
        at ProtocolLib.jar/com.comphenix.protocol.async.AsyncFilterManager.unregisterAsyncHandlers(AsyncFilterManager.java:289) ~[ProtocolLib.jar:?]
        at ProtocolLib.jar/com.comphenix.protocol.async.AsyncFilterManager.unregisterAsyncHandlers(AsyncFilterManager.java:283) ~[ProtocolLib.jar:?]
        at FastLoginBukkit.jar/com.github.games647.fastlogin.bukkit.FastLoginBukkit.onDisable(FastLoginBukkit.java:198) ~[FastLoginBukkit.jar:?]
        at org.bukkit.plugin.java.JavaPlugin.setEnabled(JavaPlugin.java:291) ~[paper-api-1.20.6-R0.1-SNAPSHOT.jar:?]
        at io.papermc.paper.plugin.manager.PaperPluginInstanceManager.disablePlugin(PaperPluginInstanceManager.java:237) ~[paper-1.20.6.jar:1.20.6-148-20f5165]
        at io.papermc.paper.plugin.manager.PaperPluginInstanceManager.disablePlugins(PaperPluginInstanceManager.java:161) ~[paper-1.20.6.jar:1.20.6-148-20f5165]
        at io.papermc.paper.plugin.manager.PaperPluginManagerImpl.disablePlugins(PaperPluginManagerImpl.java:97) ~[paper-1.20.6.jar:1.20.6-148-20f5165]
        at org.bukkit.plugin.SimplePluginManager.disablePlugins(SimplePluginManager.java:541) ~[paper-api-1.20.6-R0.1-SNAPSHOT.jar:?]
        at org.bukkit.craftbukkit.CraftServer.disablePlugins(CraftServer.java:595) ~[paper-1.20.6.jar:1.20.6-148-20f5165]
        at net.minecraft.server.MinecraftServer.stopServer(MinecraftServer.java:978) ~[paper-1.20.6.jar:1.20.6-148-20f5165]
        at net.minecraft.server.dedicated.DedicatedServer.stopServer(DedicatedServer.java:842) ~[paper-1.20.6.jar:1.20.6-148-20f5165]
        at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:1273) ~[paper-1.20.6.jar:1.20.6-148-20f5165]
        at net.minecraft.server.MinecraftServer.lambda$spin$0(MinecraftServer.java:323) ~[paper-1.20.6.jar:1.20.6-148-20f5165]
        at java.base/java.lang.Thread.run(Thread.java:1583) ~[?:?]

On thing that bothers me though is the : null part of the exception. Is this the correct fix? I don't know, but it works
¯\_(ツ)_/¯

@dmulloy2 dmulloy2 enabled auto-merge (squash) August 16, 2024 16:52
@SzczurekYT
Copy link
Copy Markdown
Contributor Author

SzczurekYT commented Aug 16, 2024

Hmm
Locally the tests also failed (before making the change) so probably something is wrong with master branch.

@dmulloy2 dmulloy2 merged commit d636be8 into dmulloy2:master Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmulloy2 dmulloy2 dmulloy2 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@SzczurekYT @dmulloy2