From 2f634c7ef7576d0011a745b806127c13c4856c14 Mon Sep 17 00:00:00 2001 From: Aikar Date: Sun, 12 Apr 2020 15:50:48 -0400 Subject: [PATCH] Forced Watchdog Crash support and Improve Async Shutdown If the request to shut down the server is received while we are in a watchdog hang, immediately treat it as a crash and begin the shutdown process. Shutdown process is now improved to also shutdown cleanly when not using restart scripts either. If a server is deadlocked, a server owner can send SIGUP (or any other signal the JVM understands to shut down as it currently does) and the watchdog will no longer need to wait until the full timeout, allowing you to trigger a close process and try to shut the server down gracefully, saving player and world data. Previously there was no way to trigger this outside of waiting for a full watchdog timeout, which may be set to a really long time... Additionally, fix everything to do with shutting the server down asynchronously. Previously, nearly everything about the process was fragile and unsafe. Main might not have actually been frozen, and might still be manipulating state. Or, some reuest might ask main to do something in the shutdown but main is dead. Or worse, other things might start closing down items such as the Console or Thread Pool before we are fully shutdown. This change tries to resolve all of these issues by moving everything into the stop method and guaranteeing only one thread is stopping the server. We then issue Thread Death to the main thread of another thread initiates the stop process. We have to ensure Thread Death propagates correctly though to stop main completely. This is to ensure that if main isn't truely stuck, it's not manipulating state we are trying to save. diff --git a/src/main/java/net/minecraft/server/CrashReport.java b/src/main/java/net/minecraft/server/CrashReport.java index 3de19c998b..c7dc8787cc 100644 --- a/src/main/java/net/minecraft/server/CrashReport.java +++ b/src/main/java/net/minecraft/server/CrashReport.java @@ -257,6 +257,7 @@ public class CrashReport { } public static CrashReport a(Throwable throwable, String s) { + if (throwable instanceof ThreadDeath) com.destroystokyo.paper.util.SneakyThrow.sneaky(throwable); // Paper while (throwable instanceof CompletionException && throwable.getCause() != null) { throwable = throwable.getCause(); } diff --git a/src/main/java/net/minecraft/server/DedicatedServer.java b/src/main/java/net/minecraft/server/DedicatedServer.java index 1ef7890da5..e62ca0543f 100644 --- a/src/main/java/net/minecraft/server/DedicatedServer.java +++ b/src/main/java/net/minecraft/server/DedicatedServer.java @@ -750,7 +750,7 @@ public class DedicatedServer extends MinecraftServer implements IMinecraftServer @Override public void stop() { super.stop(); - SystemUtils.f(); + //SystemUtils.f(); // Paper - moved into super } @Override diff --git a/src/main/java/net/minecraft/server/IAsyncTaskHandler.java b/src/main/java/net/minecraft/server/IAsyncTaskHandler.java index cfe43e882e..2632c7c3ec 100644 --- a/src/main/java/net/minecraft/server/IAsyncTaskHandler.java +++ b/src/main/java/net/minecraft/server/IAsyncTaskHandler.java @@ -135,6 +135,7 @@ public abstract class IAsyncTaskHandler implements Mailbox resourcePackRepository; @Nullable private ResourcePackSourceFolder resourcePackFolder; + public volatile Thread shutdownThread; // Paper public CommandDispatcher commandDispatcher; private final CraftingManager craftingManager; private final TagRegistry tagRegistry; @@ -176,7 +177,7 @@ public abstract class MinecraftServer extends IAsyncTaskHandlerReentrant {}; + } + // Paper end return new TickTask(this.ticks, runnable); } diff --git a/src/main/java/net/minecraft/server/PlayerList.java b/src/main/java/net/minecraft/server/PlayerList.java index dfe6251576..160476fa29 100644 --- a/src/main/java/net/minecraft/server/PlayerList.java +++ b/src/main/java/net/minecraft/server/PlayerList.java @@ -398,7 +398,7 @@ public abstract class PlayerList { cserver.getPluginManager().callEvent(playerQuitEvent); entityplayer.getBukkitEntity().disconnect(playerQuitEvent.getQuitMessage()); - entityplayer.playerTick();// SPIGOT-924 + if (server.isMainThread()) entityplayer.playerTick();// SPIGOT-924 // Paper - don't tick during emergency shutdowns (Watchdog) // CraftBukkit end // Paper start - Remove from collideRule team if needed diff --git a/src/main/java/net/minecraft/server/SystemUtils.java b/src/main/java/net/minecraft/server/SystemUtils.java index dc6d030621..bc8b904660 100644 --- a/src/main/java/net/minecraft/server/SystemUtils.java +++ b/src/main/java/net/minecraft/server/SystemUtils.java @@ -109,6 +109,7 @@ public class SystemUtils { return SystemUtils.c; } + public static void shutdownServerThreadPool() { f(); } // Paper - OBFHELPER public static void f() { SystemUtils.c.shutdown(); diff --git a/src/main/java/net/minecraft/server/World.java b/src/main/java/net/minecraft/server/World.java index 5e6481f115..d530508b61 100644 --- a/src/main/java/net/minecraft/server/World.java +++ b/src/main/java/net/minecraft/server/World.java @@ -786,6 +786,7 @@ public abstract class World implements GeneratorAccess, AutoCloseable { gameprofilerfiller.exit(); } catch (Throwable throwable) { + if (throwable instanceof ThreadDeath) throw throwable; // Paper // Paper start - Prevent tile entity and entity crashes String msg = "TileEntity threw exception at " + tileentity.world.getWorld().getName() + ":" + tileentity.position.getX() + "," + tileentity.position.getY() + "," + tileentity.position.getZ(); System.err.println(msg); @@ -861,6 +862,7 @@ public abstract class World implements GeneratorAccess, AutoCloseable { try { consumer.accept(entity); } catch (Throwable throwable) { + if (throwable instanceof ThreadDeath) throw throwable; // Paper // Paper start - Prevent tile entity and entity crashes String msg = "Entity threw exception at " + entity.world.getWorld().getName() + ":" + entity.locX() + "," + entity.locY() + "," + entity.locZ(); System.err.println(msg); diff --git a/src/main/java/org/bukkit/craftbukkit/CraftServer.java b/src/main/java/org/bukkit/craftbukkit/CraftServer.java index 8cc0f66ce5..dcc44be613 100644 --- a/src/main/java/org/bukkit/craftbukkit/CraftServer.java +++ b/src/main/java/org/bukkit/craftbukkit/CraftServer.java @@ -1705,7 +1705,7 @@ public final class CraftServer implements Server { @Override public boolean isPrimaryThread() { - return Thread.currentThread().equals(console.serverThread); // Paper - Fix issues with detecting main thread properly + return Thread.currentThread().equals(console.serverThread) || Thread.currentThread().equals(net.minecraft.server.MinecraftServer.getServer().shutdownThread); // Paper - Fix issues with detecting main thread properly, the only time Watchdog will be used is during a crash shutdown which is a "try our best" scenario } @Override diff --git a/src/main/java/org/bukkit/craftbukkit/util/ServerShutdownThread.java b/src/main/java/org/bukkit/craftbukkit/util/ServerShutdownThread.java index 449e99d1b6..899a525209 100644 --- a/src/main/java/org/bukkit/craftbukkit/util/ServerShutdownThread.java +++ b/src/main/java/org/bukkit/craftbukkit/util/ServerShutdownThread.java @@ -12,12 +12,25 @@ public class ServerShutdownThread extends Thread { @Override public void run() { try { + // Paper start - try to shutdown on main + server.safeShutdown(false, false); + for (int i = 1000; i > 0 && !server.hasStopped(); i -= 100) { + Thread.sleep(100); + } + if (server.hasStopped()) { + return; + } + // Looks stalled, close async org.spigotmc.AsyncCatcher.enabled = false; // Spigot org.spigotmc.AsyncCatcher.shuttingDown = true; // Paper + server.forceTicks = true; server.close(); + } catch (InterruptedException e) { + e.printStackTrace(); + // Paper end } finally { try { - net.minecrell.terminalconsole.TerminalConsoleAppender.close(); // Paper - Use TerminalConsoleAppender + //net.minecrell.terminalconsole.TerminalConsoleAppender.close(); // Paper - Move into stop } catch (Exception e) { } } diff --git a/src/main/java/org/spigotmc/RestartCommand.java b/src/main/java/org/spigotmc/RestartCommand.java index aefea3a9a8..123de5ac90 100644 --- a/src/main/java/org/spigotmc/RestartCommand.java +++ b/src/main/java/org/spigotmc/RestartCommand.java @@ -139,7 +139,7 @@ public class RestartCommand extends Command // Paper end // Paper start - copied from above and modified to return if the hook registered - private static boolean addShutdownHook(String restartScript) + public static boolean addShutdownHook(String restartScript) { String[] split = restartScript.split( " " ); if ( split.length > 0 && new File( split[0] ).isFile() ) diff --git a/src/main/java/org/spigotmc/WatchdogThread.java b/src/main/java/org/spigotmc/WatchdogThread.java index 5bdcdcf9e8..fe4b8caf28 100644 --- a/src/main/java/org/spigotmc/WatchdogThread.java +++ b/src/main/java/org/spigotmc/WatchdogThread.java @@ -41,6 +41,7 @@ public class WatchdogThread extends Thread { if ( instance == null ) { + if (timeoutTime <= 0) timeoutTime = 300; // Paper instance = new WatchdogThread( timeoutTime * 1000L, restart ); instance.start(); } @@ -67,12 +68,13 @@ public class WatchdogThread extends Thread // Paper start Logger log = Bukkit.getServer().getLogger(); long currentTime = monotonicMillis(); - if ( lastTick != 0 && currentTime > lastTick + earlyWarningEvery && !Boolean.getBoolean("disable.watchdog") ) + MinecraftServer server = MinecraftServer.getServer(); + if (lastTick != 0 && hasStarted && (!server.isRunning() || (currentTime > lastTick + earlyWarningEvery && !Boolean.getBoolean("disable.watchdog")) )) { - boolean isLongTimeout = currentTime > lastTick + timeoutTime; + boolean isLongTimeout = currentTime > lastTick + timeoutTime || (!server.isRunning() && !server.hasStopped() && currentTime > lastTick + 1000); // Don't spam early warning dumps if ( !isLongTimeout && (earlyWarningEvery <= 0 || !hasStarted || currentTime < lastEarlyWarning + earlyWarningEvery || currentTime < lastTick + earlyWarningDelay)) continue; - if ( !isLongTimeout && MinecraftServer.getServer().hasStopped()) continue; // Don't spam early watchdog warnings during shutdown, we'll come back to this... + if ( !isLongTimeout && server.hasStopped()) continue; // Don't spam early watchdog warnings during shutdown, we'll come back to this... lastEarlyWarning = currentTime; if (isLongTimeout) { // Paper end @@ -114,7 +116,7 @@ public class WatchdogThread extends Thread log.log( Level.SEVERE, "------------------------------" ); log.log( Level.SEVERE, "Server thread dump (Look for plugins here before reporting to Paper!):" ); // Paper ChunkTaskManager.dumpAllChunkLoadInfo(); // Paper - dumpThread( ManagementFactory.getThreadMXBean().getThreadInfo( MinecraftServer.getServer().serverThread.getId(), Integer.MAX_VALUE ), log ); + dumpThread( ManagementFactory.getThreadMXBean().getThreadInfo( server.serverThread.getId(), Integer.MAX_VALUE ), log ); log.log( Level.SEVERE, "------------------------------" ); // // Paper start - Only print full dump on long timeouts @@ -135,9 +137,24 @@ public class WatchdogThread extends Thread if ( isLongTimeout ) { - if ( restart && !MinecraftServer.getServer().hasStopped() ) + if ( !server.hasStopped() ) { - RestartCommand.restart(); + AsyncCatcher.enabled = false; // Disable async catcher incase it interferes with us + AsyncCatcher.shuttingDown = true; + server.forceTicks = true; + if (restart) { + RestartCommand.addShutdownHook( SpigotConfig.restartScript ); + } + // try one last chance to safe shutdown on main incase it 'comes back' + server.safeShutdown(false, restart); + try { + Thread.sleep(1000); + } catch (InterruptedException e) { + e.printStackTrace(); + } + if (!server.hasStopped()) { + server.close(); + } } break; } // Paper end -- 2.26.2