From 32bac210ffbe88652bb184c5a63e35405cf49217 Mon Sep 17 00:00:00 2001 From: Runemoro Date: Wed, 25 Apr 2018 14:59:32 -0400 Subject: [PATCH] Fix MC-128953 --- .../org/dimdev/vanillafix/GuiCrashScreen.java | 4 + .../mixins/MixinNetHandlerPlayServer.java | 6 - .../mixins/client/IFixedMinecraft.java | 5 + .../mixins/client/MixinMinecraft.java | 134 +++++++++++++----- 4 files changed, 104 insertions(+), 45 deletions(-) create mode 100644 src/main/java/org/dimdev/vanillafix/mixins/client/IFixedMinecraft.java diff --git a/src/main/java/org/dimdev/vanillafix/GuiCrashScreen.java b/src/main/java/org/dimdev/vanillafix/GuiCrashScreen.java index aee6043c..7721c2ba 100644 --- a/src/main/java/org/dimdev/vanillafix/GuiCrashScreen.java +++ b/src/main/java/org/dimdev/vanillafix/GuiCrashScreen.java @@ -1,5 +1,6 @@ package org.dimdev.vanillafix; +import net.minecraft.client.Minecraft; import net.minecraft.client.gui.*; import net.minecraft.client.resources.I18n; import net.minecraft.crash.CrashReport; @@ -9,6 +10,8 @@ import net.minecraftforge.fml.relauncher.SideOnly; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.dimdev.ddutils.HasteUpload; +import org.dimdev.vanillafix.mixins.client.IFixedMinecraft; +import org.dimdev.vanillafix.mixins.client.MixinMinecraft; import java.io.File; import java.net.URI; @@ -39,6 +42,7 @@ public class GuiCrashScreen extends GuiScreen { try { if (button.id == 0) { mc.displayGuiScreen(new GuiMainMenu()); + ((IFixedMinecraft) mc).clearCurrentReport(); } else if (button.id == 1) { if (hasteLink == null) { hasteLink = HasteUpload.uploadToHaste(HASTE_BASE_URL, "txt", report.getCompleteReport()); diff --git a/src/main/java/org/dimdev/vanillafix/mixins/MixinNetHandlerPlayServer.java b/src/main/java/org/dimdev/vanillafix/mixins/MixinNetHandlerPlayServer.java index ad3a7a90..0821ed92 100644 --- a/src/main/java/org/dimdev/vanillafix/mixins/MixinNetHandlerPlayServer.java +++ b/src/main/java/org/dimdev/vanillafix/mixins/MixinNetHandlerPlayServer.java @@ -9,7 +9,6 @@ import net.minecraft.network.play.INetHandlerPlayServer; import net.minecraft.network.play.client.CPacketConfirmTeleport; import net.minecraft.network.play.client.CPacketPlayer; import net.minecraft.server.MinecraftServer; -import net.minecraft.util.math.BlockPos; import net.minecraft.util.math.Vec3d; import net.minecraft.util.text.ITextComponent; import net.minecraft.util.text.TextComponentTranslation; @@ -117,9 +116,7 @@ public abstract class MixinNetHandlerPlayServer implements INetHandlerPlayServer // Instead, we will send another packet both here and in processConfirmTeleport if the position the client // was sent is no longer good (exceeds tolerance): - LOGGER.info("Player sent move packets before confirming"); if (targetPos.squareDistanceTo(player.posX, player.posY, player.posZ) > 1.0D) { - LOGGER.info("Distance became too big: " + targetPos + " vs. " + new Vec3d(player.posX, player.posY, player.posY)); setPlayerLocation(player.posX, player.posY, player.posZ, player.rotationYaw, player.rotationPitch); } @@ -199,7 +196,6 @@ public abstract class MixinNetHandlerPlayServer implements INetHandlerPlayServer // Entity.move already made sure that the player didn't cheat, and reverting the move would be wrong because // the prevX/Y/Z is no longer good in the new dimension. This fixes MC-123364. if (player.isInvulnerableDimensionChange()) { - LOGGER.info("Doing teleport"); // A better name for invulnerableDimensionChange would be "lastBlockCollisionCausedPlayerMove". See // https://github.com/ModCoderPack/MCPBot-Issues/issues/624. This happens when the move caused a // collision that teleported the player elsewhere. @@ -283,11 +279,9 @@ public abstract class MixinNetHandlerPlayServer implements INetHandlerPlayServer if (packet.getTeleportId() == teleportId) { if (targetPos.squareDistanceTo(player.posX, player.posY, player.posZ) > 1.0D) { - LOGGER.info("Position accepted no longer good"); // The client accepted the position change, but it's too late, something moved the player. Sync it again. setPlayerLocation(player.posX, player.posY, player.posZ, player.rotationYaw, player.rotationPitch); } else { - LOGGER.info("Teleport accepted"); targetPos = null; } } diff --git a/src/main/java/org/dimdev/vanillafix/mixins/client/IFixedMinecraft.java b/src/main/java/org/dimdev/vanillafix/mixins/client/IFixedMinecraft.java new file mode 100644 index 00000000..ca807c2e --- /dev/null +++ b/src/main/java/org/dimdev/vanillafix/mixins/client/IFixedMinecraft.java @@ -0,0 +1,5 @@ +package org.dimdev.vanillafix.mixins.client; + +public interface IFixedMinecraft { + public void clearCurrentReport(); +} diff --git a/src/main/java/org/dimdev/vanillafix/mixins/client/MixinMinecraft.java b/src/main/java/org/dimdev/vanillafix/mixins/client/MixinMinecraft.java index 4752dbcb..6ee18ec5 100644 --- a/src/main/java/org/dimdev/vanillafix/mixins/client/MixinMinecraft.java +++ b/src/main/java/org/dimdev/vanillafix/mixins/client/MixinMinecraft.java @@ -3,6 +3,8 @@ package org.dimdev.vanillafix.mixins.client; import net.minecraft.client.Minecraft; import net.minecraft.client.gui.GuiMemoryErrorScreen; import net.minecraft.client.gui.GuiScreen; +import net.minecraft.client.multiplayer.WorldClient; +import net.minecraft.client.renderer.RenderGlobal; import net.minecraft.crash.CrashReport; import net.minecraft.init.Bootstrap; import net.minecraft.profiler.ISnooperInfo; @@ -14,10 +16,7 @@ import net.minecraftforge.fml.relauncher.SideOnly; import org.apache.logging.log4j.Logger; import org.dimdev.vanillafix.GuiCrashScreen; import org.lwjgl.LWJGLException; -import org.spongepowered.asm.mixin.Final; -import org.spongepowered.asm.mixin.Mixin; -import org.spongepowered.asm.mixin.Overwrite; -import org.spongepowered.asm.mixin.Shadow; +import org.spongepowered.asm.mixin.*; import javax.annotation.Nullable; import java.io.File; @@ -28,18 +27,18 @@ import java.util.Date; @SuppressWarnings({"unused", "NonConstantFieldWithUpperCaseName", "RedundantThrows"}) // Shadow @SideOnly(Side.CLIENT) @Mixin(Minecraft.class) -public abstract class MixinMinecraft implements IThreadListener, ISnooperInfo { +@Implements(@Interface(iface = IFixedMinecraft.class, prefix = "minecraft$")) +public abstract class MixinMinecraft implements IThreadListener, ISnooperInfo, IFixedMinecraft { @Shadow volatile boolean running; @Shadow private boolean hasCrashed; @Shadow private CrashReport crashReporter; + @Shadow private long debugCrashKeyPressTime; @Shadow private void init() throws LWJGLException, IOException {} @Shadow private void runGameLoop() throws IOException {} - @Shadow public void freeMemory() {} - @Shadow public void displayGuiScreen(@Nullable GuiScreen guiScreenIn) {} @Shadow public CrashReport addGraphicsAndWorldToCrashReport(CrashReport theCrash) { return null; } @@ -50,6 +49,20 @@ public abstract class MixinMinecraft implements IThreadListener, ISnooperInfo { @Shadow public void displayCrashReport(CrashReport crashReportIn) {} + @Shadow public abstract void loadWorld(@Nullable WorldClient worldClientIn); + + @Shadow public WorldClient world; + + @Shadow public static byte[] memoryReserve; + + @Shadow public RenderGlobal renderGlobal; + + private CrashReport currentReport = null; + + /** + * Allows the player to choose to return to the title screen after a crash, or get + * a pasteable link to the crash report on paste.dimdev.org. + */ @SuppressWarnings("CallToSystemGC") @Overwrite public void run() { @@ -58,9 +71,9 @@ public abstract class MixinMinecraft implements IThreadListener, ISnooperInfo { try { init(); } catch (Throwable throwable) { - CrashReport crashreport = CrashReport.makeCrashReport(throwable, "Initializing game"); - crashreport.makeCategory("Initialization"); - displayCrashReport(addGraphicsAndWorldToCrashReport(crashreport)); // TODO: GUI for this too + CrashReport report = CrashReport.makeCrashReport(throwable, "Initializing game"); + report.makeCategory("Initialization"); + displayCrashReport(addGraphicsAndWorldToCrashReport(report)); // TODO: GUI for this too return; } @@ -71,8 +84,8 @@ public abstract class MixinMinecraft implements IThreadListener, ISnooperInfo { runGameLoop(); } catch (OutOfMemoryError e) { freeMemory(); + // TODO: Use displayCrashScreen? displayGuiScreen(new GuiMemoryErrorScreen()); - System.gc(); } catch (ReportedException e) { addGraphicsAndWorldToCrashReport(e.getCrashReport()); freeMemory(); @@ -95,35 +108,78 @@ public abstract class MixinMinecraft implements IThreadListener, ISnooperInfo { } public void displayCrashScreen(CrashReport report) { - try { - File crashReportsDir = new File(Minecraft.getMinecraft().mcDataDir, "crash-reports"); - File crashReportSaveFile = new File(crashReportsDir, "crash-" + new SimpleDateFormat("yyyy-MM-dd_HH.mm.ss").format(new Date()) + "-client.txt"); - - // Print the report in bootstrap - Bootstrap.printToSYSOUT(report.getCompleteReport()); - - // Save the report and print file in bootstrap - File reportFile = null; - if (report.getFile() != null) { - reportFile = report.getFile(); - } else if (report.saveToFile(crashReportSaveFile)) { - reportFile = crashReportSaveFile; - } - - if (reportFile != null) { - Bootstrap.printToSYSOUT("Recoverable game crash! Crash report saved to: " + reportFile); - } else { - Bootstrap.printToSYSOUT("Recoverable game crash! Crash report could not be saved."); - } - - // Display the crash screen - displayGuiScreen(new GuiCrashScreen(reportFile, report)); - - // Keep running - hasCrashed = false; - } catch (Throwable e) { - LOGGER.error("The crash screen threw an error, reverting to default crash report", e); + if (currentReport != null) { + // There was already a crash being reported, the crash screen might have + // crashed. Report it normally instead. + LOGGER.error("An uncaught exception occured while displaying the crash screen, making normal report instead", report.getCrashCause()); displayCrashReport(report); + return; } + + currentReport = report; + + File crashReportsDir = new File(Minecraft.getMinecraft().mcDataDir, "crash-reports"); + File crashReportSaveFile = new File(crashReportsDir, "crash-" + new SimpleDateFormat("yyyy-MM-dd_HH.mm.ss").format(new Date()) + "-client.txt"); + + // Print the report in bootstrap + Bootstrap.printToSYSOUT(report.getCompleteReport()); + + // Save the report and print file in bootstrap + File reportFile = null; + if (report.getFile() != null) { + reportFile = report.getFile(); + } else if (report.saveToFile(crashReportSaveFile)) { + reportFile = crashReportSaveFile; + } + + if (reportFile != null) { + Bootstrap.printToSYSOUT("Recoverable game crash! Crash report saved to: " + reportFile); + } else { + Bootstrap.printToSYSOUT("Recoverable game crash! Crash report could not be saved."); + } + + // Reset hasCrashed and debugCrashKeyPressTime + hasCrashed = false; + debugCrashKeyPressTime = -1; + + // Display the crash screen + displayGuiScreen(new GuiCrashScreen(reportFile, report)); + } + + /** + * Disconnect from the current world and free memory, using a memory reserve + * to make sure that an OutOfMemory doesn't happen while doing this. + *

+ * Bugs Fixed: + * - https://bugs.mojang.com/browse/MC-128953 + * - Memory reserve not recreated after out-of memory + */ + @SuppressWarnings("CallToSystemGC") + @Overwrite + public void freeMemory() { + try { + memoryReserve = new byte[0]; + renderGlobal.deleteAllDisplayLists(); + } catch (Throwable ignored) {} + + try { + System.gc(); + // Fix: Close the connection to avoid receiving packets from old server + // when playing in another world (MC-128953) + if (world != null) { + world.sendQuittingDisconnectingPacket(); + } + loadWorld(null); + } catch (Throwable ignored) {} + + System.gc(); + + // Fix: Re-create memory reserve so that future crashes work well too + memoryReserve = new byte[10485760]; + } + + @Override + public void clearCurrentReport() { + currentReport = null; } }