[1.19.2] Item model properties registration must be enqueued
XFactHD opened this issue · 3 comments
I noticed this while looking at the log of a user in the Forge Discord. The data structures used by ItemProperties are not thread-safe and as such must be enqueued to run sequentially through FMLClientSetupEvent#enqueueWork(), otherwise you may get a ConcurrentModificationException when at least one other mod makes the same mistake.
Example stacktrace
[17:53:02] [Worker-Main-2/ERROR] [ne.mi.fm.ja.FMLModContainer/LOADING]: Caught exception during event FMLClientSetupEvent dispatch for modid dungeons_mobs
java.util.ConcurrentModificationException: null
at java.util.HashMap.computeIfAbsent(HashMap.java:1221) ~[?:?] {re:mixin}
at net.minecraft.client.renderer.item.ItemProperties.register(ItemProperties.java:53) ~[client-1.19.2-20220805.130853-srg.jar#336!/:?] {re:classloading,pl:accesstransformer:B,pl:runtimedistcleaner:A}
at com.infamous.dungeons_mobs.client.ModItemModelProperties.<init>(ModItemModelProperties.java:10) ~[dungeons_mobs-1.19.2-4.0.6-beta.jar#266!/:1.19.2-4.0.6-beta] {re:classloading}
at com.infamous.dungeons_mobs.DungeonsMobs.doClientStuff(DungeonsMobs.java:92) ~[dungeons_mobs-1.19.2-4.0.6-beta.jar#266!/:1.19.2-4.0.6-beta] {re:classloading}
at net.minecraftforge.eventbus.EventBus.doCastFilter(EventBus.java:260) ~[eventbus-6.0.3.jar#129!/:?] {}
at net.minecraftforge.eventbus.EventBus.lambda$addListener$11(EventBus.java:252) ~[eventbus-6.0.3.jar#129!/:?] {}
at net.minecraftforge.eventbus.EventBus.post(EventBus.java:315) ~[eventbus-6.0.3.jar#129!/:?] {}
at net.minecraftforge.eventbus.EventBus.post(EventBus.java:296) ~[eventbus-6.0.3.jar#129!/:?] {}
at net.minecraftforge.fml.javafmlmod.FMLModContainer.acceptEvent(FMLModContainer.java:107) ~[javafmllanguage-1.19.2-43.2.0.jar#338!/:?] {}
at net.minecraftforge.fml.ModContainer.lambda$buildTransitionHandler$10(ModContainer.java:122) ~[fmlcore-1.19.2-43.2.0.jar#337!/:?] {}
at java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1804) [?:?] {}
at java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1796) [?:?] {}
at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:373) [?:?] {}
at java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1182) [?:?] {}
at java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1655) [?:?] {re:computing_frames}
at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1622) [?:?] {re:computing_frames}
at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:165) [?:?] {}
Forge version: 1.19.2-43.2.0
Dungeons Mobs version: 1.19.2-4.0.6-beta
Reading the involved code also makes me question why a constructor is used for the registration instead of a static method, considering the event bus registration of that instance seems to be completely pointless (https://github.com/Infamous-Misadventures/Dungeons-Mobs/blob/1.19/src/main/java/com/infamous/dungeons_mobs/DungeonsMobs.java#L92).
You're right. We had a similar issue in Dungeons Gear that was fixed. I had no idea the same problem was also present here.
As for why a constructor gets registered to the event bus, I...don't know what to tell you. I had the same reaction when I saw that in Dungeons Gear. I'll have to look back at the commit history and see who added that.
