Stereowalker / Survive

https://modrinth.com/mod/survive
https://www.curseforge.com/minecraft/mc-mods/survive
Other
19 stars 18 forks source link

[BUG] ItemProperties setup causing ConcurrentModificationException #243

Open Wabbit0101 opened 2 years ago

Wabbit0101 commented 2 years ago

Forge version: 1.18.2-40.1.20 UnionLib version: 1.18.2-7.1.3-Forge Survive version: 1.18.2-7.1.0

Occasionally the following error happens on startup (note this is a sample specific to my testing for 7.1.0):

-- Head --
Thread: Render thread
Stacktrace:
    at java.util.HashMap.computeIfAbsent(HashMap.java:1221) ~[?:?] {}
-- MOD survive --
Details:
    Mod File: Survive-1.18.2-7.1.0_mapped_parchment_2022.05.02-1.18.2.jar
    Failure message: Survive (survive) encountered an error during the sided_setup event phase
        java.util.ConcurrentModificationException: null
    Mod Version: 1.18.2-7.1.0
    Mod Issue URL: https://github.com/Stereowalker/Survive/issues
    Exception message: java.util.ConcurrentModificationException
Stacktrace:
    at java.util.HashMap.computeIfAbsent(HashMap.java:1221) ~[?:?] {}
    at net.minecraft.client.renderer.item.ItemProperties.register(ItemProperties.java:60) ~[forge-1.18.2-40.1.20_mapped_parchment_2022.05.02-1.18.2-recomp.jar%2376!/:?] {re:classloading,pl:runtimedistcleaner:A}
    at com.stereowalker.survive.compat.SItemProperties.registerAll(SItemProperties.java:18) ~[Survive-1.18.2-7.1.0_mapped_parchment_2022.05.02-1.18.2.jar%2384!/:1.18.2-7.1.0] {re:classloading}
    at com.stereowalker.survive.Survive.clientRegistries(Survive.java:235) ~[Survive-1.18.2-7.1.0_mapped_parchment_2022.05.02-1.18.2.jar%2384!/:1.18.2-7.1.0] {re:mixin,re:classloading}
    at net.minecraftforge.eventbus.EventBus.doCastFilter(EventBus.java:247) ~[eventbus-5.0.7.jar%239!/:?] {}
...

The root cause is this line in source Survive::clientRegistries not being done inside of an enqueueWork wrapper...

SItemProperties.registerAll();

like so:

event.enqueueWork(()->SItemProperties.registerAll());

The Mojang ItemProperties.register is not threadsafe (it uses a non-threadsafe static HashMap) so mods need to wrap this call to prevent random concurrent modification errors like we see above as this forge event is dispatched to mods in parallel threads.