From 506457d87804bb67d1b16c85e4868ba3d0dfc4ee Mon Sep 17 00:00:00 2001 From: altrisi Date: Thu, 21 Oct 2021 04:06:19 +0200 Subject: [PATCH 1/4] Fix Stack Overflow in dev environment with mojmap (#1134) --- .../java/carpet/fakes/ThreadedAnvilChunkStorageInterface.java | 2 +- src/main/java/carpet/mixins/ServerChunkManager_tickMixin.java | 2 +- .../ThreadedAnvilChunkStorage_scarpetChunkCreationMixin.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/carpet/fakes/ThreadedAnvilChunkStorageInterface.java b/src/main/java/carpet/fakes/ThreadedAnvilChunkStorageInterface.java index 7054bd23..d1473e53 100644 --- a/src/main/java/carpet/fakes/ThreadedAnvilChunkStorageInterface.java +++ b/src/main/java/carpet/fakes/ThreadedAnvilChunkStorageInterface.java @@ -14,5 +14,5 @@ public interface ThreadedAnvilChunkStorageInterface void releaseRelightTicket(ChunkPos pos); - Iterable getChunks(); + Iterable getChunksCM(); } diff --git a/src/main/java/carpet/mixins/ServerChunkManager_tickMixin.java b/src/main/java/carpet/mixins/ServerChunkManager_tickMixin.java index 5a198899..26fd34ac 100644 --- a/src/main/java/carpet/mixins/ServerChunkManager_tickMixin.java +++ b/src/main/java/carpet/mixins/ServerChunkManager_tickMixin.java @@ -62,7 +62,7 @@ public abstract class ServerChunkManager_tickMixin // simplified chunk tick iteration assuming world is frozen otherwise as suggested by Hadron67 // to be kept in sync with the original injection source if (!debug){ - List holders = Lists.newArrayList(((ThreadedAnvilChunkStorageInterface)threadedAnvilChunkStorage).getChunks()); + List holders = Lists.newArrayList(((ThreadedAnvilChunkStorageInterface)threadedAnvilChunkStorage).getChunksCM()); Collections.shuffle(holders); for (ChunkHolder holder: holders){ Optional optional = holder.getTickingFuture().getNow(ChunkHolder.UNLOADED_WORLD_CHUNK).left(); diff --git a/src/main/java/carpet/mixins/ThreadedAnvilChunkStorage_scarpetChunkCreationMixin.java b/src/main/java/carpet/mixins/ThreadedAnvilChunkStorage_scarpetChunkCreationMixin.java index 3940b5ee..f2c4db11 100644 --- a/src/main/java/carpet/mixins/ThreadedAnvilChunkStorage_scarpetChunkCreationMixin.java +++ b/src/main/java/carpet/mixins/ThreadedAnvilChunkStorage_scarpetChunkCreationMixin.java @@ -484,7 +484,7 @@ public abstract class ThreadedAnvilChunkStorage_scarpetChunkCreationMixin implem } @Override - public Iterable getChunks() { + public Iterable getChunksCM() { return entryIterator(); } } From 07140983471214ddf7b8bd4eb679046ef2063305 Mon Sep 17 00:00:00 2001 From: Fx Morin Date: Wed, 20 Oct 2021 22:08:23 -0400 Subject: [PATCH 2/4] Ported rule `updateSuppressionCrashFix` from Carpet-Extra + More (#1133) * Posted rule `updateSuppressionCrashFix` from Carpet-Extra Also added catch for update suppression in neighbour update during ServerPlayerEntity tick. Added Logger `updateSuppressedCrashes` so not everyone sees the crash messages Added Coords to crash message so we now know where it originates from * Make it easier to read * that `s` has been there for a long time * Made `updateSuppressionCrashFix` compatible with carpet-extra --- src/main/java/carpet/CarpetSettings.java | 6 ++ .../carpet/helpers/ThrowableSuppression.java | 12 ++++ .../java/carpet/logging/LoggerRegistry.java | 5 +- ...Server_updateSuppressionCrashFixMixin.java | 57 +++++++++++++++++++ ...Entity_updateSuppressionCrashFixMixin.java | 55 ++++++++++++++++++ .../World_updateSuppressionCrashFixMixin.java | 32 +++++++++++ src/main/resources/carpet.mixins.json | 6 +- 7 files changed, 169 insertions(+), 4 deletions(-) create mode 100644 src/main/java/carpet/helpers/ThrowableSuppression.java create mode 100644 src/main/java/carpet/mixins/MinecraftServer_updateSuppressionCrashFixMixin.java create mode 100644 src/main/java/carpet/mixins/ServerPlayerEntity_updateSuppressionCrashFixMixin.java create mode 100644 src/main/java/carpet/mixins/World_updateSuppressionCrashFixMixin.java diff --git a/src/main/java/carpet/CarpetSettings.java b/src/main/java/carpet/CarpetSettings.java index 10fc3567..966f4f02 100644 --- a/src/main/java/carpet/CarpetSettings.java +++ b/src/main/java/carpet/CarpetSettings.java @@ -961,6 +961,12 @@ public class CarpetSettings ) public static String updateSuppressionBlock = "false"; + @Rule( + desc = "Fixes update suppression causing server crashes.", + category = {BUGFIX} + ) + public static boolean updateSuppressionCrashFix = false; + public static int getInteger(String s) { try { return Integer.parseInt(s); diff --git a/src/main/java/carpet/helpers/ThrowableSuppression.java b/src/main/java/carpet/helpers/ThrowableSuppression.java new file mode 100644 index 00000000..76ae4e14 --- /dev/null +++ b/src/main/java/carpet/helpers/ThrowableSuppression.java @@ -0,0 +1,12 @@ +package carpet.helpers; + +import net.minecraft.util.math.BlockPos; + +public class ThrowableSuppression extends RuntimeException{ + + public final BlockPos pos; + public ThrowableSuppression(String message, BlockPos pos) { + super(message); + this.pos = pos; + } +} \ No newline at end of file diff --git a/src/main/java/carpet/logging/LoggerRegistry.java b/src/main/java/carpet/logging/LoggerRegistry.java index be181fea..677f2553 100644 --- a/src/main/java/carpet/logging/LoggerRegistry.java +++ b/src/main/java/carpet/logging/LoggerRegistry.java @@ -28,6 +28,7 @@ public class LoggerRegistry public static boolean __packets; public static boolean __pathfinding; public static boolean __explosions; + public static boolean __updateSuppressedCrashes; public static void initLoggers() { @@ -47,6 +48,7 @@ public class LoggerRegistry registerLogger("counter",HUDLogger.stardardHUDLogger("counter","white", Arrays.stream(DyeColor.values()).map(Object::toString).toArray(String[]::new))); registerLogger("mobcaps", HUDLogger.stardardHUDLogger("mobcaps", "dynamic",new String[]{"dynamic", "overworld", "nether","end"})); registerLogger("explosions", HUDLogger.stardardLogger("explosions", "brief",new String[]{"brief", "full"})); + registerLogger("updateSuppressedCrashes", Logger.stardardLogger("updateSuppressedCrashes", null,null)); } @@ -172,7 +174,4 @@ public class LoggerRegistry log.onPlayerDisconnect(player); } } - - - } diff --git a/src/main/java/carpet/mixins/MinecraftServer_updateSuppressionCrashFixMixin.java b/src/main/java/carpet/mixins/MinecraftServer_updateSuppressionCrashFixMixin.java new file mode 100644 index 00000000..67d42b8e --- /dev/null +++ b/src/main/java/carpet/mixins/MinecraftServer_updateSuppressionCrashFixMixin.java @@ -0,0 +1,57 @@ +package carpet.mixins; + +import carpet.CarpetSettings; +import carpet.helpers.ThrowableSuppression; +import carpet.logging.LoggerRegistry; +import carpet.utils.Messenger; +import net.minecraft.server.MinecraftServer; +import net.minecraft.server.world.ServerWorld; +import net.minecraft.text.BaseText; +import net.minecraft.util.crash.CrashException; +import net.minecraft.util.math.BlockPos; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Redirect; + +import java.util.function.BooleanSupplier; + +@Mixin(MinecraftServer.class) +public class MinecraftServer_updateSuppressionCrashFixMixin { + + @Redirect( + method = "tickWorlds", + at = @At( + value = "INVOKE", + target = "Lnet/minecraft/server/world/ServerWorld;tick(Ljava/util/function/BooleanSupplier;)V" + ), + require = 0 + ) + private void fixUpdateSuppressionCrashTick(ServerWorld serverWorld, BooleanSupplier shouldKeepTicking){ + if (!CarpetSettings.updateSuppressionCrashFix) { + serverWorld.tick(shouldKeepTicking); + return; + } + try { + serverWorld.tick(shouldKeepTicking); + } catch (CrashException e) { + if (!(e.getCause() instanceof ThrowableSuppression throwableSuppression)) throw e; + logUpdateSuppression(throwableSuppression.pos); + } catch (ThrowableSuppression e) { + logUpdateSuppression(e.pos); + } + } + + + private void logUpdateSuppression(BlockPos pos) { + if(LoggerRegistry.__updateSuppressedCrashes) { + LoggerRegistry.getLogger("updateSuppressedCrashes").log(() -> { + return new BaseText[]{Messenger.c( + "w Server crash prevented in: ", + "m world tick ", + "w - at: ", + "g [ " + pos.toShortString() + " ]" + )}; + }); + } + } +} diff --git a/src/main/java/carpet/mixins/ServerPlayerEntity_updateSuppressionCrashFixMixin.java b/src/main/java/carpet/mixins/ServerPlayerEntity_updateSuppressionCrashFixMixin.java new file mode 100644 index 00000000..b1c3c009 --- /dev/null +++ b/src/main/java/carpet/mixins/ServerPlayerEntity_updateSuppressionCrashFixMixin.java @@ -0,0 +1,55 @@ +package carpet.mixins; + +import carpet.CarpetSettings; +import carpet.helpers.ThrowableSuppression; +import carpet.logging.LoggerRegistry; +import carpet.utils.Messenger; +import net.minecraft.entity.player.PlayerEntity; +import net.minecraft.server.network.ServerPlayerEntity; +import net.minecraft.text.BaseText; +import net.minecraft.util.crash.CrashException; +import net.minecraft.util.math.BlockPos; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Redirect; + +@Mixin(ServerPlayerEntity.class) +public class ServerPlayerEntity_updateSuppressionCrashFixMixin { + + @Redirect( + method = "playerTick()V", + at = @At( + value = "INVOKE", + target = "Lnet/minecraft/entity/player/PlayerEntity;tick()V" + ), + require = 0 + ) + private void fixUpdateSuppressionCrashPlayerTick(PlayerEntity playerEntity){ + if (!CarpetSettings.updateSuppressionCrashFix) { + playerEntity.tick(); + return; + } + try { + playerEntity.tick(); + } catch (CrashException e) { + if (!(e.getCause() instanceof ThrowableSuppression throwableSuppression)) throw e; + logUpdateSuppressionPlayer(throwableSuppression.pos); + } catch (ThrowableSuppression e) { + logUpdateSuppressionPlayer(e.pos); + } + } + + + private void logUpdateSuppressionPlayer(BlockPos pos) { + if(LoggerRegistry.__updateSuppressedCrashes) { + LoggerRegistry.getLogger("updateSuppressedCrashes").log(() -> { + return new BaseText[]{Messenger.c( + "w Server crash prevented in: ", + "m player tick ", + "w - at: ", + "g [ " + pos.toShortString() + " ]" + )}; + }); + } + } +} diff --git a/src/main/java/carpet/mixins/World_updateSuppressionCrashFixMixin.java b/src/main/java/carpet/mixins/World_updateSuppressionCrashFixMixin.java new file mode 100644 index 00000000..300b8c90 --- /dev/null +++ b/src/main/java/carpet/mixins/World_updateSuppressionCrashFixMixin.java @@ -0,0 +1,32 @@ +package carpet.mixins; + +import carpet.CarpetSettings; +import carpet.helpers.ThrowableSuppression; +import net.minecraft.block.Block; +import net.minecraft.block.BlockState; +import net.minecraft.util.math.BlockPos; +import net.minecraft.world.World; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Inject; +import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; +import org.spongepowered.asm.mixin.injection.callback.LocalCapture; + +@Mixin(World.class) +public class World_updateSuppressionCrashFixMixin { + + @Inject( + method = "updateNeighbor", + at = @At( + value="INVOKE", + target = "Lnet/minecraft/util/crash/CrashReport;create(Ljava/lang/Throwable;Ljava/lang/String;)Lnet/minecraft/util/crash/CrashReport;" + ), + locals = LocalCapture.CAPTURE_FAILHARD, + require = 0 + ) + public void checkUpdateSuppression(BlockPos sourcePos, Block sourceBlock, BlockPos neighborPos, CallbackInfo ci, BlockState state,Throwable throwable){ + if(CarpetSettings.updateSuppressionCrashFix && (throwable instanceof ThrowableSuppression || throwable instanceof StackOverflowError)){ + throw new ThrowableSuppression("Update suppression",neighborPos); + } + } +} diff --git a/src/main/resources/carpet.mixins.json b/src/main/resources/carpet.mixins.json index a0839af2..23a5e35f 100644 --- a/src/main/resources/carpet.mixins.json +++ b/src/main/resources/carpet.mixins.json @@ -220,7 +220,11 @@ "LivingEntity_creativeFlyMixin", "VeryBiasedToBottomHeightProvider_cleanLogsMixin", - "TrapezoidHeightProvider_cleanLogsMixin" + "TrapezoidHeightProvider_cleanLogsMixin", + + "World_updateSuppressionCrashFixMixin", + "MinecraftServer_updateSuppressionCrashFixMixin", + "ServerPlayerEntity_updateSuppressionCrashFixMixin" ], "client": [ "RenderTickCounter_tickSpeedMixin", From f17b63a8f72d67de92393d04da8f98d0db6047b3 Mon Sep 17 00:00:00 2001 From: intricate Date: Wed, 20 Oct 2021 23:12:23 -0300 Subject: [PATCH 3/4] Fix #1126 (#1132) --- .../java/carpet/logging/logHelpers/ExplosionLogHelper.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/carpet/logging/logHelpers/ExplosionLogHelper.java b/src/main/java/carpet/logging/logHelpers/ExplosionLogHelper.java index ede43e3e..05121056 100644 --- a/src/main/java/carpet/logging/logHelpers/ExplosionLogHelper.java +++ b/src/main/java/carpet/logging/logHelpers/ExplosionLogHelper.java @@ -28,6 +28,7 @@ public class ExplosionLogHelper private static long lastGametime = 0; private static int explosionCountInCurretGT = 0; + private static boolean newTick; public ExplosionLogHelper(Entity entity, double x, double y, double z, float power, boolean createFire, Explosion.DestructionType blockDestructionType) { this.entity = entity; @@ -44,14 +45,16 @@ public class ExplosionLogHelper public void onExplosionDone(long gametime) { - List messages = new ArrayList<>(); + newTick = false; if (!(lastGametime == gametime)){ explosionCountInCurretGT = 0; lastGametime = gametime; - messages.add(c("wb tick : ", "d " + gametime)); + newTick = true; } explosionCountInCurretGT++; LoggerRegistry.getLogger("explosions").log( (option) -> { + List messages = new ArrayList<>(); + if(newTick) messages.add(c("wb tick : ", "d " + gametime)); if ("brief".equals(option)) { messages.add( c("d #" + explosionCountInCurretGT,"gb ->", From 50b3aa88323b8b76259b08a5f845afa5b4fd63c7 Mon Sep 17 00:00:00 2001 From: Jake K <68135816+Sylk0s@users.noreply.github.com> Date: Wed, 20 Oct 2021 22:16:05 -0400 Subject: [PATCH 4/4] fixed logger option bug (#1124) * fixed logger option bug * implemented strict option for logger args * added back deprecated logger constructor Co-authored-by: gnembon <41132274+gnembon@users.noreply.github.com> --- src/main/java/carpet/commands/LogCommand.java | 10 ++++++-- src/main/java/carpet/logging/HUDLogger.java | 16 +++++++++--- src/main/java/carpet/logging/Logger.java | 25 +++++++++++++++++-- .../java/carpet/logging/LoggerRegistry.java | 4 +-- 4 files changed, 46 insertions(+), 9 deletions(-) diff --git a/src/main/java/carpet/commands/LogCommand.java b/src/main/java/carpet/commands/LogCommand.java index d7400183..fc142c16 100644 --- a/src/main/java/carpet/commands/LogCommand.java +++ b/src/main/java/carpet/commands/LogCommand.java @@ -18,6 +18,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Arrays; import static com.mojang.brigadier.arguments.StringArgumentType.getString; import static net.minecraft.command.CommandSource.suggestMatching; @@ -201,8 +202,13 @@ public class LogCommand Messenger.m(source, "r Unknown logger: ","rb "+logname); return 0; } + if (!LoggerRegistry.getLogger(logname).isOptionValid(option)) + { + Messenger.m(source, "r Invalid option: ", "rb "+option); + return 0; + } LoggerRegistry.subscribePlayer(player_name, logname, option); - if (option!=null) + if (option != null) { Messenger.m(source, "gi Subscribed to " + logname + "(" + option + ")"); } @@ -210,6 +216,6 @@ public class LogCommand { Messenger.m(source, "gi Subscribed to " + logname); } - return 1; + return 1; } } diff --git a/src/main/java/carpet/logging/HUDLogger.java b/src/main/java/carpet/logging/HUDLogger.java index 018a3dfe..be5a695b 100644 --- a/src/main/java/carpet/logging/HUDLogger.java +++ b/src/main/java/carpet/logging/HUDLogger.java @@ -8,11 +8,16 @@ import java.lang.reflect.Field; public class HUDLogger extends Logger { static Logger stardardHUDLogger(String logName, String def, String [] options) + { + return stardardHUDLogger(logName, def, options, false); + } + + static Logger stardardHUDLogger(String logName, String def, String [] options, boolean strictOptions) { // should convert to factory method if more than 2 classes are here try { - return new HUDLogger(LoggerRegistry.class.getField("__"+logName), logName, def, options); + return new HUDLogger(LoggerRegistry.class.getField("__"+logName), logName, def, options, strictOptions); } catch (NoSuchFieldException e) { @@ -20,9 +25,14 @@ public class HUDLogger extends Logger } } - public HUDLogger(Field field, String logName, String def, String[] options) + public HUDLogger(Field field, String logName, String def, String[] options, boolean strictOptions) { - super(field, logName, def, options); + super(field, logName, def, options, strictOptions); + } + + @Deprecated + public HUDLogger(Field field, String logName, String def, String[] options) { + super(field, logName, def, options, false); } @Override diff --git a/src/main/java/carpet/logging/Logger.java b/src/main/java/carpet/logging/Logger.java index 1d30aa66..f6b2bd7e 100644 --- a/src/main/java/carpet/logging/Logger.java +++ b/src/main/java/carpet/logging/Logger.java @@ -32,11 +32,18 @@ public class Logger private Field acceleratorField; + private boolean strictOptions; + static Logger stardardLogger(String logName, String def, String [] options) + { + return stardardLogger(logName, def, options, false); + } + + static Logger stardardLogger(String logName, String def, String [] options, boolean strictOptions) { try { - return new Logger(LoggerRegistry.class.getField("__"+logName), logName, def, options); + return new Logger(LoggerRegistry.class.getField("__"+logName), logName, def, options, strictOptions); } catch (NoSuchFieldException e) { @@ -44,7 +51,12 @@ public class Logger } } - public Logger(Field acceleratorField, String logName, String def, String [] options) + @Deprecated + public Logger(Field acceleratorField, String logName, String def, String [] options) { + this(acceleratorField, logName, def, options, false); + } + + public Logger(Field acceleratorField, String logName, String def, String [] options, boolean strictOptions) { subscribedOnlinePlayers = new HashMap<>(); subscribedOfflinePlayers = new HashMap<>(); @@ -52,6 +64,7 @@ public class Logger this.logName = logName; this.default_option = def; this.options = options; + this.strictOptions = strictOptions; if (acceleratorField == null) CarpetSettings.LOG.error("[CM] Logger "+getLogName()+" is missing a specified accelerator"); } @@ -232,4 +245,12 @@ public class Logger if (options != null && Arrays.asList(options).contains(arg)) return arg; return null; } + + public boolean isOptionValid(String option) { + if (strictOptions) + { + return Arrays.asList(this.options).contains(option); + } + return true; + } } diff --git a/src/main/java/carpet/logging/LoggerRegistry.java b/src/main/java/carpet/logging/LoggerRegistry.java index 677f2553..9a2d84ab 100644 --- a/src/main/java/carpet/logging/LoggerRegistry.java +++ b/src/main/java/carpet/logging/LoggerRegistry.java @@ -39,7 +39,7 @@ public class LoggerRegistry public static void registerLoggers() { - registerLogger("tnt", Logger.stardardLogger( "tnt", "brief", new String[]{"brief", "full"})); + registerLogger("tnt", Logger.stardardLogger( "tnt", "brief", new String[]{"brief", "full"}, true)); registerLogger("projectiles", Logger.stardardLogger("projectiles", "brief", new String[]{"brief", "full"})); registerLogger("fallingBlocks",Logger.stardardLogger("fallingBlocks", "brief", new String[]{"brief", "full"})); registerLogger("pathfinding", Logger.stardardLogger("pathfinding", "20", new String[]{"2", "5", "10"})); @@ -47,7 +47,7 @@ public class LoggerRegistry registerLogger("packets", HUDLogger.stardardHUDLogger("packets", null, null)); registerLogger("counter",HUDLogger.stardardHUDLogger("counter","white", Arrays.stream(DyeColor.values()).map(Object::toString).toArray(String[]::new))); registerLogger("mobcaps", HUDLogger.stardardHUDLogger("mobcaps", "dynamic",new String[]{"dynamic", "overworld", "nether","end"})); - registerLogger("explosions", HUDLogger.stardardLogger("explosions", "brief",new String[]{"brief", "full"})); + registerLogger("explosions", Logger.stardardLogger("explosions", "brief",new String[]{"brief", "full"}, true)); registerLogger("updateSuppressedCrashes", Logger.stardardLogger("updateSuppressedCrashes", null,null)); }