Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.19.2] Item model properties registration must be enqueued #182

Open
XFactHD opened this issue Mar 28, 2023 · 3 comments
Open

[1.19.2] Item model properties registration must be enqueued #182

XFactHD opened this issue Mar 28, 2023 · 3 comments
Labels
bug Something isn't working fixed in latest - backport needed Needs a backport to earlier versions

Comments

@XFactHD
Copy link

XFactHD commented Mar 28, 2023

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).

@Thelnfamous1
Copy link
Collaborator

Thelnfamous1 commented Mar 29, 2023

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.

@Thelnfamous1 Thelnfamous1 added the bug Something isn't working label Mar 29, 2023
Patrigan added a commit that referenced this issue Apr 6, 2023
@Patrigan Patrigan added the fixed in latest - backport needed Needs a backport to earlier versions label Apr 6, 2023
@Patrigan
Copy link
Collaborator

Patrigan commented Apr 6, 2023

Fixed for 1.19.2 in next version. Need to backport

@Thelnfamous1
Copy link
Collaborator

Fixed in 4.0.7-beta for 1.19.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed in latest - backport needed Needs a backport to earlier versions
Projects
None yet
Development

No branches or pull requests

3 participants