Chicken-Bones / NotEnoughItems

MIT License
130 stars 129 forks source link

[1.7.10] CME caused by calling itemstack.getTooltip on Filter Thread #361

Closed codewarrior0 closed 7 years ago

codewarrior0 commented 7 years ago

Originally reported in Blood-Asp/GT5-Unofficial#720

The symptom is a ConcurrentModification error when opening a Forestry Worktable which contains a GregTech Integrated Circuit as a stored recipe.

[07:44:09] [Client thread/FATAL]: Reported exception thrown!
net.minecraft.util.ReportedException: Rendering screen
    at net.minecraft.client.renderer.EntityRenderer.func_78480_b(EntityRenderer.java:1092) ~[blt.class:?]
    at net.minecraft.client.Minecraft.func_71411_J(Minecraft.java:1001) ~[bao.class:?]
    at net.minecraft.client.Minecraft.func_99999_d(Minecraft.java:898) [bao.class:?]
    at net.minecraft.client.main.Main.main(SourceFile:148) [Main.class:?]
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_73]
    at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) ~[?:1.8.0_73]
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) ~[?:1.8.0_73]
    at java.lang.reflect.Method.invoke(Unknown Source) ~[?:1.8.0_73]
    at net.minecraft.launchwrapper.Launch.launch(Launch.java:135) [launchwrapper-1.12.jar:?]
    at net.minecraft.launchwrapper.Launch.main(Launch.java:28) [launchwrapper-1.12.jar:?]
Caused by: java.util.ConcurrentModificationException
    at java.util.HashMap$HashIterator.nextNode(Unknown Source) ~[?:1.8.0_73]
    at java.util.HashMap$EntryIterator.next(Unknown Source) ~[?:1.8.0_73]
    at java.util.HashMap$EntryIterator.next(Unknown Source) ~[?:1.8.0_73]
    at java.util.Hashtable.putAll(Unknown Source) ~[?:1.8.0_73]
    at cpw.mods.fml.common.registry.LanguageRegistry.injectLanguage(LanguageRegistry.java:253) ~[LanguageRegistry.class:?]
    at gregtech.api.util.GT_LanguageManager.addStringLocalization(GT_LanguageManager.java:28) ~[GT_LanguageManager.class:?]
    at gregtech.api.util.GT_LanguageManager.addStringLocalization(GT_LanguageManager.java:21) ~[GT_LanguageManager.class:?]
    at gregtech.common.items.GT_IntegratedCircuit_Item.addAdditionalToolTips(GT_IntegratedCircuit_Item.java:82) ~[GT_IntegratedCircuit_Item.class:?]
    at gregtech.api.items.GT_Generic_Item.func_77624_a(GT_Generic_Item.java:98) ~[GT_Generic_Item.class:?]
    at net.minecraft.item.ItemStack.func_82840_a(ItemStack.java:525) ~[add.class:?]
    at forestry.core.gui.widgets.ItemStackWidgetBase.getToolTip(ItemStackWidgetBase.java:41) ~[ItemStackWidgetBase.class:?]
    at forestry.core.gui.GuiUtil.drawToolTips(GuiUtil.java:133) ~[GuiUtil.class:?]

The CME indicates that another thread had modified a hash table owned by the LanguageRegistry; this can only be done by calling injectLanguage from another thread. Investigation with the IntelliJ IDEA remote debugger revealed that the other thread is the NEI Item Filtering Thread:

      at cpw.mods.fml.common.registry.LanguageRegistry.injectLanguage(LanguageRegistry.java:247)
      at gregtech.api.util.GT_LanguageManager.addStringLocalization(GT_LanguageManager.java:33)
      at gregtech.common.blocks.GT_Item_Machines.func_77624_a(GT_Item_Machines.java:53)
      at net.minecraft.item.ItemStack.func_82840_a(ItemStack.java:525)
      at codechicken.nei.guihook.GuiContainerManager.itemDisplayNameMultiline(GuiContainerManager.java:119)
      at codechicken.nei.guihook.GuiContainerManager.concatenatedDisplayName(GuiContainerManager.java:162)
      at codechicken.nei.api.ItemInfo.getSearchName(ItemInfo.java:529)
      at codechicken.nei.ItemList$PatternItemFilter.matches(ItemList.java:65)
      at codechicken.nei.ItemList$AnyMultiItemFilter.matches(ItemList.java:110)
      at codechicken.nei.ItemList$AllMultiItemFilter.matches(ItemList.java:85)
      at codechicken.nei.ItemList$2.execute(ItemList.java:239)
      at codechicken.nei.RestartableTask$1.run(RestartableTask.java:24)

I expected that at some point I would see the CME be raised from this other thread, but as it turns out, the filtering thread simply throws away this exception whenever it occurs.

So now I'm wondering: itemstack.getTooltip is not thread-safe. Should it be made thread safe in FML? Or should NEI stop calling it from a non-Client thread?

Chicken-Bones commented 7 years ago

The call to injectLanguage should be moved onto the main thread. NEI is now being maintained at https://github.com/TheCBProject/NotEnoughItems