Immersive Weathering [FABRIC]

Describe the bug:
Since this is a multi-threading related bug, there is a chance that IW is not the cause and that another mod may be the culprit. So far no other mod has appeared in the stack-trace of a crash. Just vanilla world gen code and IW.

The main question of this issue is to ask:

Is it to safe to use the Level/ServerLevel random property now that world generation is multi-threaded?

Server is crashing with an Accessing LegacyRandomSource from multiple threads. This error is thrown by Minecraft code checking that an object isn't being accessed across threads unsafely - a bit like a CME check. Only 2 stack traces have shown in crash reports so far.

Vanilla world generation placing natural spawns:

java.lang.IllegalStateException: Accessing LegacyRandomSource from multiple threads
	at net.minecraft.util.ThreadingDetector.m_199417_(ThreadingDetector.java:84) ~[server-1.19.2-20220805.130853-srg.jar%23270!/:?] {re:classloading}
	at net.minecraft.world.level.levelgen.LegacyRandomSource.m_64707_(LegacyRandomSource.java:49) ~[server-1.19.2-20220805.130853-srg.jar%23270!/:?] {re:classloading}
	at net.minecraft.world.level.levelgen.BitRandomSource.m_188501_(BitRandomSource.java:56) ~[server-1.19.2-20220805.130853-srg.jar%23270!/:?] {re:classloading}
	at net.minecraft.world.level.NaturalSpawner.m_47038_(NaturalSpawner.java:154) ~[server-1.19.2-20220805.130853-srg.jar%23270!/:?] {re:mixin,re:classloading,pl:mixin:APP:more_babies.mixins.json:world.level.NaturalSpawnerMixin,pl:mixin:A}

IW's ConfigurableBlockGrowth canGrow method during world tick:

java.lang.IllegalStateException: Accessing LegacyRandomSource from multiple threads
	at net.minecraft.util.ThreadingDetector.m_199417_(ThreadingDetector.java:84) ~[server-1.19.2-20220805.130853-srg.jar%23270!/:?] {re:classloading}
	at net.minecraft.world.level.levelgen.LegacyRandomSource.m_64707_(LegacyRandomSource.java:49) ~[server-1.19.2-20220805.130853-srg.jar%23270!/:?] {re:classloading}
	at net.minecraft.world.level.levelgen.BitRandomSource.m_188501_(BitRandomSource.java:56) ~[server-1.19.2-20220805.130853-srg.jar%23270!/:?] {re:classloading}
	at com.ordana.immersive_weathering.data.block_growths.growths.ConfigurableBlockGrowth.canGrow(ConfigurableBlockGrowth.java:179) ~[immersive_weathering-1.19.2-1.2.3-forge.jar%23175!/:?] {re:classloading}

Versions: (BEFORE SUBMITTING A BUG REPORT, make sure you have the most up-to-date versions of Immersive Weathering, Moonlight Lib and Supplementaries)
Minecraft version: 1.19.2
Immersive Weathering version: 1.2.3 - There is a newer version but the suspected code is unchanged
Moonlight Lib version: 2.1.7
Fabric API / QSL / Forge version: Forge-1.19.2-43.1.52

Other mods:
Modpack: Better Minecraft 1.19 [Forge] - v9

Logs:

Crash in vanilla code: https://gist.github.com/TigerWalts/3b5884032a88883f7e68bd1645fc6a10

Crash in IW code: https://gist.github.com/TigerWalts/3f0dda4f515b2749cc27243253cdacf1

To Reproduce:
Random chance of occurring during world generation.

Expected behavior:
N/A

Screenshots:
N/A

Additional info:
Optifine ran into a similar problem. I suspect using the Level/ServerLevel RNG object was the cause there too:

sp614x/optifine#6858

Just had another crash but this time another mod triggered it and block_growths was set to false in the config. So IW is unlikely to be the cause.

This time it was Bygone Nether during world generation:

java.lang.IllegalStateException: Accessing LegacyRandomSource from multiple threads
    at net.minecraft.util.ThreadingDetector.m_199417_(ThreadingDetector.java:84) ~[server-1.19.2-20220805.130853-srg.jar%23270!/:?] {re:classloading}
    at net.minecraft.world.level.levelgen.LegacyRandomSource.m_64707_(LegacyRandomSource.java:49) ~[server-1.19.2-20220805.130853-srg.jar%23270!/:?] {re:classloading}
    at net.minecraft.world.level.levelgen.BitRandomSource.m_188503_(BitRandomSource.java:33) ~[server-1.19.2-20220805.130853-srg.jar%23270!/:?] {re:classloading}
    at com.izofar.bygonenether.entity.WarpedEnderMan.randomVariant(WarpedEnderMan.java:167) ~[bygonenether-1.2.1-1.19.x.jar%23122!/:1.2.1] {re:classloading}
    at com.izofar.bygonenether.entity.WarpedEnderMan.<init>(WarpedEnderMan.java:58) ~[bygonenether-1.2.1-1.19.x.jar%23122!/:1.2.1] {re:classloading}
    at net.minecraft.world.entity.EntityType.m_20615_(EntityType.java:469) ~[server-1.19.2-20220805.130853-srg.jar%23270!/:?] {re:mixin,re:classloading,pl:mixin:APP:architectury-common.mixins.json:inject.MixinEntityType,pl:mixin:A}
    at com.izofar.bygonenether.world.feature.MobFeature.m_142674_(MobFeature.java:32) ~[bygonenether-1.2.1-1.19.x.jar%23122!/:1.2.1] {re:classloading}
    at net.minecraft.world.level.levelgen.feature.Feature.m_225028_(Feature.java:154) ~[server-1.19.2-20220805.130853-srg.jar%23270!/:?] {re:mixin,re:classloading,pl:mixin:APP:farmersdelight.mixins.json:KeepRichSoilMixin,pl:mixin:A}
    at net.minecraft.world.level.levelgen.feature.ConfiguredFeature.m_224953_(ConfiguredFeature.java:27) ~[server-1.19.2-20220805.130853-srg.jar%23270!/:?] {re:mixin,re:classloading,pl:mixin:APP:blue_skies.mixins.json:ConfiguredFeatureMixin,pl:mixin:A}
    at net.minecraft.world.level.levelgen.placement.PlacedFeature.m_226362_(PlacedFeature.java:56) ~[server-1.19.2-20220805.130853-srg.jar%23270!/:?] {re:classloading}

Not closing this though as I still don't know if using the level's random object during world tick is safe.

Savage and Ravage appears to be another mod guilty of this:

team-abnormals/savage-and-ravage#119

I think that it's clear that using Level/ServerLevel.random is not thread safe and an alternate source of random numbers should be used.

If a thread safe random source isn't available on any of the objects you are working with, then ThreadLocalRandom is a safe choice:

https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/concurrent/ThreadLocalRandom.html

Immersive-Weathering/common/src/main/java/com/ordana/immersive_weathering/data/block_growths/growths/ConfigurableBlockGrowth.java

Lines 177 to 187 in 53bf701

    
           private boolean canGrow(BlockPos pos, Level level, Supplier<Holder<Biome>> biome) { 
        
               if (this.growthChance == 0) return false; 
        
               if (level.random.nextFloat() < this.growthChance) { 
        
                   for (IPositionRuleTest positionTest : this.positionTests) { 
        
                       //they all need to be true 
        
                       if (!positionTest.test(biome, pos, level)) return false; 
        
                   } 
        
                   return PlatformHelper.isAreaLoaded(level, pos, maxRange); 
        
               } 
        
               return false; 
        
           }

Becomes:

    private boolean canGrow(BlockPos pos, Level level, Supplier<Holder<Biome>> biome) {
        if (this.growthChance == 0) return false;
        if (ThreadLocalRandom.current().nextFloat(1.0) < this.growthChance) {
            for (IPositionRuleTest positionTest : this.positionTests) {
                //they all need to be true
                if (!positionTest.test(biome, pos, level)) return false;
            }
            return PlatformHelper.isAreaLoaded(level, pos, maxRange);
        }
        return false;
    }

Assuming this.growthChance is bound between 0.0 and 1.0.

	private boolean canGrow(BlockPos pos, Level level, Supplier<Holder<Biome>> biome) {
	if (this.growthChance == 0) return false;
	if (level.random.nextFloat() < this.growthChance) {
	for (IPositionRuleTest positionTest : this.positionTests) {
	//they all need to be true
	if (!positionTest.test(biome, pos, level)) return false;
	}
	return PlatformHelper.isAreaLoaded(level, pos, maxRange);
	}
	return false;
	}

Share to