CC: Tweaked

CC: Tweaked

42M Downloads

ItemProperty registration isn't thread safe

pupnewfster opened this issue ยท 2 comments

commented

Minecraft Version

1.18.x

Version

1.100.5

Details

Crash report; https://gist.github.com/pupnewfster/78af1652f0cce80d5a82b3b19349ad56

The issue is a race/threading error as ItemProperties is backed by a basic hashmap so if two mods are improperly registering their item properties at the same time then a crash happens. (I am unsure which mod is the other one who is doing it as well but it definitely would be great for CC to fix it on your side as well). The fix should be as simple as wrapping all of this inside an enqueueWork call:

event.enqueueWork(() -> {
//register item properties
});
commented

Thank you, good spot! Dread to think how difficult this was to pin down.

I thought I'd done an audit of these when I added them, but clearly not on the client side - container registration was wrong too.

commented

Thanks for the quick fix!

Honestly wasn't really difficult to pin down once I had it randomly crash my instance as I remember running into issues with ItemProperties in the past so reading the crash log was pretty straightforward. When I first ported to using it after mojang stopped having it be done in constructors, I ran into the crash from time to time trying to launch my dev environment as multiple modules of Mekanism make use of it so occasionally they threw the CME. So at some point I just went through and looked at the implementation of the various methods I call to ensure that if they use a basic collection that isn't synchronized then I make sure to enqueue it.