GitLab

Summary

The implementation of SimpleTerminableRegistry.close() cause ConcurrentModificationException in multi threads context.

Steps to reproduce

This bug occurs randomly when casting some aura and can not be reproduced stablely. I have checked the source and found out the cause.

Here is the stacktrace:

java.util.ConcurrentModificationException
	at java.base/java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1095)
	at java.base/java.util.ArrayList$ListItr.previous(ArrayList.java:1122)
	at com.google.common.collect.Lists$ReverseList$1.next(Lists.java:935)
	at java.base/java.lang.Iterable.forEach(Iterable.java:74)
	at MythicMobs-5.7.2.jar//io.lumine.mythic.bukkit.utils.terminable.SimpleTerminableRegistry.close(SimpleTerminableRegistry.java:31)
	at MythicMobs-5.7.2.jar//io.lumine.mythic.bukkit.utils.terminable.Terminable.closeAndReportException(Terminable.java:61)
	at MythicMobs-5.7.2.jar//io.lumine.mythic.bukkit.utils.terminable.Terminable.terminate(Terminable.java:18)
	at MythicMobs-5.7.2.jar//io.lumine.mythic.core.skills.auras.Aura$AuraTracker.close(Aura.java:634)
	at MythicMobs-5.7.2.jar//io.lumine.mythic.bukkit.utils.terminable.Terminable.closeAndReportException(Terminable.java:61)
	at MythicMobs-5.7.2.jar//io.lumine.mythic.bukkit.utils.terminable.Terminable.terminate(Terminable.java:18)
	at MythicMobs-5.7.2.jar//io.lumine.mythic.core.skills.auras.Aura$AuraTracker.run(Aura.java:568)
	at MythicMobs-5.7.2.jar//io.lumine.mythic.bukkit.utils.Delegates$RunnableToConsumer.accept(Delegates.java:93)
	at MythicMobs-5.7.2.jar//io.lumine.mythic.bukkit.utils.Schedulers$LumineTask.run(Schedulers.java:184)
	at org.bukkit.craftbukkit.scheduler.CraftTask.run(CraftTask.java:86)
	at org.bukkit.craftbukkit.scheduler.CraftAsyncTask.run(CraftAsyncTask.java:57)
	at com.destroystokyo.paper.ServerSchedulerReportingWrapper.run(ServerSchedulerReportingWrapper.java:22)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
	at java.base/java.lang.Thread.run(Thread.java:1583)

The SimpleTerminableRegistry.close() method will iterate the termibales#List field in multi threads context. The list has been synchronized through Collections.synchronizedList() method. Howerver, the Collections#SynchronizedList.iterator()
method is not synced and needs to be synced manually but not in the code. The List.forEach() method will invoke the unsynced List.iterator() method, causing ConcurrentModificationException.

Proposed fixes

Sync the iteration in some way.

I have checked the source and this bug still exists in MM-5.8.0. But have not gotten it reproduced yet because of randomness.