Effective (Forge)

Effective (Forge)

Mods
6M Downloads

Client crashes when rendering is concurrent to level ticking

MartijnMuijsers opened this issue ยท 0 comments

commented

When LiquidBlockRenderer.renderFluid is called on a different thread than LevelHandler.levelTick, the following occasionally happens leading to random crashes from time to time (more likely when many flowing water blocks are nearby due to the longer time that they are being processed):

  • On the tick thread, WaterfallCloudGenerators:78 begins computing, and ends up in the internal implementation of the stream operations, which are not thread-safe, and as a first step initializes the immutable non-resizable list to collect into, for which it uses GENERATORS.size as size
  • On the render thread, LiquidBlockRendererMixin:34 calls WaterfallCloudGenerators.addGenerator and completes, changing the size of WaterfallCloudGenerators.addGenerator.GENERATORS
  • On the tick thread, The internal implementation of the stream operations processes elements of GENERATORS of which there is now 1 more than the previously obtained size, as such not fitting in the initialized list

This leads to the following crash:

java.lang.ArrayIndexOutOfBoundsException: Index 966 out of bounds for length 966
	at java.util.stream.SortedOps$SizedRefSortingSink.accept(SortedOps.java:369) ~[?:?] {}
	at java.util.HashMap$KeySpliterator.forEachRemaining(HashMap.java:1707) ~[?:?] {}
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) ~[?:?] {}
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) ~[?:?] {}
	at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921) ~[?:?] {}
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?] {}
	at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682) ~[?:?] {}
	at bottomtextdanny.effective_fg.level.WaterfallCloudGenerators.tick(WaterfallCloudGenerators.java:78) ~[effective_fg-1.3.4.jar%23126!/:1.3] {re:mixin,re:classloading}
	at bottomtextdanny.effective_fg.level.LevelHandler.levelTick(LevelHandler.java:10) ~[effective_fg-1.3.4.jar%23126!/:1.3] {re:classloading}
	at net.minecraftforge.eventbus.EventBus.doCastFilter(EventBus.java:260) ~[eventbus-6.0.3.jar%23106!/:?] {}
	at net.minecraftforge.eventbus.EventBus.lambda$addListener$11(EventBus.java:252) ~[eventbus-6.0.3.jar%23106!/:?] {}
	at net.minecraftforge.eventbus.EventBus.post(EventBus.java:315) ~[eventbus-6.0.3.jar%23106!/:?] {}
	at net.minecraftforge.eventbus.EventBus.post(EventBus.java:296) ~[eventbus-6.0.3.jar%23106!/:?] {}
	at net.minecraftforge.event.ForgeEventFactory.onPostClientTick(ForgeEventFactory.java:803) ~[forge-1.19.2-43.1.1-universal.jar%23138!/:?] {re:classloading}
	...

Solution:
Around each usage of GENERATORS, use synchronized (GENERATORS) - this fixes the issue.
But implementation can be made slightly more optimal:

  • Instead of synchronizing around the entire removeIf block, synchronize over GENERATORS and clone it. Then, outside of synchronization, filter is to get the elements to remove. Then synchronize over GENERATORS and remove all elements to remove.
  • Instead of synchronizing around the entire sorting block, synchronize over GENERATORS and clone it. Then, outside of synchronization, sort it.