Cardinal Components API

Cardinal Components API

28M Downloads

[1.20.1 | Chunk] Components passed an `ImposterProtoChunk` that hasn't finished initializing

ByThePowerOfScience opened this issue ยท 1 comments

commented

Library Version: 5.2.3
Fabric Loader: 0.16.14
Fabric API: 0.92.5+1.20.1

Problem occurs when launching through Architectury's IntelliJ IDEA client run, but it shouldn't be limited to just that.

Problem Description

Let's say you register a chunk component like this:

// ChunkComponentInitializer entrypoint
@Override
void registerChunkComponentFactories(ChunkComponentFactoryRegistry registry) {
  registry.register(MY_TYPE, (ChunkAccess chunk) -> { new MyComponent(chunk) });
}

The problem is that during world load, that ChunkAccess chunk is a lot of the time an ImposterProtoChunk, one where wrapped is null. Since all ChunkAccess methods on IPC delegate to wrapped, every single useful method will result in a NPE.

Why This Happens

From checking the debugger, it seems like the Mixin that adds Components to chunks injects into the ChunkAccess constructor. The problem is, this is fired long before any subclasses' initializers finish running, meaning that the ChunkAccess we get isn't fully usable.

It's what's called a "leaking 'this'", which is something I never fully understood until now.

// IPC Constructor
public ImposterProtoChunk(LevelChunk wrapped, boolean allowWrites) {
  super(/* ... */); // <-- Where Components get access to the object
  this.wrapped = wrapped; // <-- Where you no longer get NPEs when accessing it
  this.allowWrites = allowWrites;
}
Stacktrace
java.lang.NullPointerException: Cannot invoke "net.minecraft.world.level.chunk.LevelChunk.getPos()" because "this.wrapped" is null
	at net.minecraft.world.level.chunk.ImposterProtoChunk.getPos(ImposterProtoChunk.java:141) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at btpos.mcmods.terminus.fabric.world.chunk.ChunkAuraData_Fabric.<init>(ChunkAuraData.kt:19) ~[main/:?]
	at btpos.mcmods.terminus.fabric.ComponentsRegistrar.registerChunkComponentFactories$lambda$0(ComponentsRegistrar.kt:13) ~[main/:?]
	at dev.onyxstudios.cca._generated_.GeneratedComponentContainer_0.<init>(Unknown Source) ~[?:?]
	at dev.onyxstudios.cca._generated_.GeneratedContainerFactory_0.createContainer(Unknown Source) ~[?:?]
	at dev.onyxstudios.cca.internal.chunk.StaticChunkComponentPlugin.createContainer(StaticChunkComponentPlugin.java:45) ~[cardinal-components-chunk-5.2.3.jar:?]
	at net.minecraft.world.level.chunk.ChunkAccess.handler$zhb000$cardinal-components-chunk$initComponents(ChunkAccess.java:558) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at net.minecraft.world.level.chunk.ChunkAccess.<init>(ChunkAccess.java:112) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at net.minecraft.world.level.chunk.ProtoChunk.<init>(ProtoChunk.java:69) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at net.minecraft.world.level.chunk.ProtoChunk.<init>(ProtoChunk.java:56) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at net.minecraft.world.level.chunk.ImposterProtoChunk.<init>(ImposterProtoChunk.java:38) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at net.minecraft.server.level.ChunkMap.method_17227(ChunkMap.java:747) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at com.mojang.datafixers.util.Either.lambda$mapLeft$0(Either.java:162) ~[datafixerupper-6.0.8.jar:?]
	at com.mojang.datafixers.util.Either$Left.map(Either.java:38) ~[datafixerupper-6.0.8.jar:?]
	at com.mojang.datafixers.util.Either.mapLeft(Either.java:162) ~[datafixerupper-6.0.8.jar:?]
	at net.minecraft.server.level.ChunkMap.method_20460(ChunkMap.java:739) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at java.util.concurrent.CompletableFuture$UniApply.tryFire(CompletableFuture.java:646) ~[?:?]
	at java.util.concurrent.CompletableFuture$Completion.run(CompletableFuture.java:482) ~[?:?]
	at net.minecraft.server.level.ChunkTaskPriorityQueueSorter.method_17634(ChunkTaskPriorityQueueSorter.java:62) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at net.minecraft.util.thread.BlockableEventLoop.doRunTask(BlockableEventLoop.java:156) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at net.minecraft.server.level.ServerChunkCache$MainThreadExecutor.doRunTask(ServerChunkCache.java:564) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at net.minecraft.util.thread.BlockableEventLoop.pollTask(BlockableEventLoop.java:130) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at net.minecraft.server.level.ServerChunkCache$MainThreadExecutor.pollTask(ServerChunkCache.java:573) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at net.minecraft.server.level.ServerChunkCache.pollTask(ServerChunkCache.java:282) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at net.minecraft.server.MinecraftServer.pollTaskInternal(MinecraftServer.java:770) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at net.minecraft.server.MinecraftServer.pollTask(MinecraftServer.java:758) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at net.minecraft.util.thread.BlockableEventLoop.managedBlock(BlockableEventLoop.java:139) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at net.minecraft.server.MinecraftServer.waitUntilNextTick(MinecraftServer.java:743) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at net.minecraft.server.MinecraftServer.prepareLevels(MinecraftServer.java:488) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at net.minecraft.server.MinecraftServer.loadLevel(MinecraftServer.java:327) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at net.minecraft.client.server.IntegratedServer.initServer(IntegratedServer.java:69) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:646) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at net.minecraft.server.MinecraftServer.method_29739(MinecraftServer.java:265) ~[minecraft-merged-ed39575182-1.20.1-loom.mappings.1_20_1.layered+hash.1900670477-v2.jar:?]
	at java.lang.Thread.run(Thread.java:840) ~[?:?]
commented

Sorry for the delay, and good analysis on your part. My general advice would be to defer any use of ChunkAccess in the constructor, possibly using patterns like lazy initialization. Fixing this on CCA's part would be a bit more difficult and possibly open a new can of worms.