![Canary](https://media.forgecdn.net/avatars/thumbnails/596/701/256/256/637973011528270749.png)
[Regression] `TickingBlockEntity#getPos` sometimes returns null
andriihorpenko opened this issue ยท 13 comments
Version Information
canary-mc1.19.2.0.2.10
Expected Behavior
TickingBlockEntity#getPos
should never return null.
Actual Behavior
TickingBlockEntity#getPos
may return null, causing unexpected behavior.
Reproduction Steps
Since Canary 0.2.9, TickingBlockEntity#getPos
sometimes returns true, which breaks mods relying on non-null value, as there is no @Nullable
annotation and no once expects that it can be null.
Class net.minecraft.world.level.Level
has the following block of code (see below). Some mods are mixing into tickingblockentity.tick()
call to do various things (for instance, profiling how much tick time each block entity took).
while(iterator.hasNext()) {
TickingBlockEntity tickingblockentity = iterator.next();
if (tickingblockentity.isRemoved()) {
iterator.remove();
} else if (this.shouldTickBlocksAt(tickingblockentity.getPos())) {
tickingblockentity.tick(); // popular injection point for profiling mods
}
}
Seems like https://github.com/AbdElAziz333/Canary/blob/mc1.19.2/dev/src/main/java/com/abdelaziz/canary/mixin/world/block_entity_ticking/sleeping/LevelMixin.java should've prevented that, but according to my crash report, this check basically didn't run, letting tickingblockentity.tick()
actually occur.
This is my mixin making this issue possible:
@WrapOperation(method = "tickBlockEntities", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/level/block/entity/TickingBlockEntity;tick()V"))
private void captureBlockEntityTickTime(TickingBlockEntity instance, Operation<Void> original) {
System.out.println(instance.getPos()); // this causes NPE
}
Other Information
Note: this is not happening on Canary 0.2.8. This regression happens since 0.2.9 onwards.
Crash report: crash-2024-01-02_00.44.10-server.txt
And no, 0.3.0 has exactly the same issue as stated above.The crash report is identical.
Please try the new update from here for MC 1.19.2.
Note, 0.3.0 release became incompatible with ServerCore
https://bytebin.lucko.me/pDmO5UTOqO
Here are sufficient steps to get a crash:
- Install canary-mc1.19.2.0.3.0 (or canary-mc1.19.2.0.2.9)
- Add the following mixin in any sample mod:
import com.llamalad7.mixinextras.injector.wrapoperation.Operation;
import com.llamalad7.mixinextras.injector.wrapoperation.WrapOperation;
import net.minecraft.world.level.Level;
import net.minecraft.world.level.block.entity.TickingBlockEntity;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.injection.At;
@Mixin(Level.class)
public abstract class LevelMixin {
@WrapOperation(method = "tickBlockEntities", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/level/block/entity/TickingBlockEntity;tick()V"))
private void captureBlockEntityTickTime(TickingBlockEntity instance, Operation<Void> original) {
System.out.println(instance.getPos());
}
}
You should get a crash like this: crash-2024-01-02_00.44.10-server.txt
After testing this with only Canary and the code you provided, in both client and server it just prints the block positions like this:
BlockPos{x=-77, y=5, z=-268}
BlockPos{x=-82, y=11, z=-201}
BlockPos{x=-60, y=6, z=-284}
and doesn't crash at all, do you have specific mods to try with?
Maybe try with ModernFix and ServerCore, I guess they both provide a lot of optimizations as well. Mind that ServerCore crashes with latest 0.3.0 version.
It does not happen without Canary, and it does not happen if you have Canary 0.2.8 and below. I had Canary for quite a long time and this crash happens after updating to 0.2.9 and above.
Can you please try it in the latest update for 1.19.2 from here.
This fixes ServerCore incompatibility, but the original crash remains the same as it has nothing to do with ServerCore. I might try binary search to determine what exactly causes incompatibility with the Canary mixin states above as right now I have absolutely no clue.
In the crash report you can see what mixins were applied to Level class, this might help you find out which mods are somehow mixing into Level and causing such an unexpected behavior.
After testing this with only Canary and the code you provided, in both client and server it just prints the block positions like this:
BlockPos{x=-77, y=5, z=-268} BlockPos{x=-82, y=11, z=-201} BlockPos{x=-60, y=6, z=-284}
and doesn't crash at all, do you have specific mods to try with?
As for this: try simulating a situation when position is null. I don't know when this happens, seems like it's somehow connected to new "sleeping" thing.
I think I know how this happens. This mixin works just fine. The thing is that the position becomes NULL in a slightly different place.
Let's look at the following code again:
// Class net.minecraft.world.level.Level
} else if (this.shouldTickBlocksAt(tickingblockentity.getPos())) {
// position is not NULL yet
tickingblockentity.tick();
// position became NULL
}
Basically, after a block entity tick, its position may become null, causing unexpected behavior. The only block entity wrapper class I can think of is com.abdelaziz.canary.common.block.entity.SleepUntilTimeBlockEntityTickInvoker
, which returns position dynamically, potentially causing such an unexpected behavior.