From 01e8ce8d2d541f00af68243b3252e60a3ea8fd1d Mon Sep 17 00:00:00 2001 From: Aikar Date: Thu, 16 Apr 2020 04:53:50 -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 SIGHUP (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. --- ...own-server-during-a-watchdog-hang-gr.patch | 137 -------- ...Crash-support-and-Improve-Async-Shut.patch | 330 ++++++++++++++++++ ...imise-entity-hard-collision-checking.patch | 6 +- 3 files changed, 333 insertions(+), 140 deletions(-) delete mode 100644 Spigot-Server-Patches/0469-Allow-shutting-down-server-during-a-watchdog-hang-gr.patch create mode 100644 Spigot-Server-Patches/0469-Forced-Watchdog-Crash-support-and-Improve-Async-Shut.patch diff --git a/Spigot-Server-Patches/0469-Allow-shutting-down-server-during-a-watchdog-hang-gr.patch b/Spigot-Server-Patches/0469-Allow-shutting-down-server-during-a-watchdog-hang-gr.patch deleted file mode 100644 index f56eb515ee..0000000000 --- a/Spigot-Server-Patches/0469-Allow-shutting-down-server-during-a-watchdog-hang-gr.patch +++ /dev/null @@ -1,137 +0,0 @@ -From 9aa8c43914b453de4e0a7449f3c70b4f6b07d0e8 Mon Sep 17 00:00:00 2001 -From: Aikar -Date: Sun, 12 Apr 2020 15:50:48 -0400 -Subject: [PATCH] Allow shutting down server during a watchdog hang gracefully - -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... - -diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java -index 2686874f2..a9b533751 100644 ---- a/src/main/java/net/minecraft/server/MinecraftServer.java -+++ b/src/main/java/net/minecraft/server/MinecraftServer.java -@@ -176,7 +176,7 @@ public abstract class MinecraftServer extends IAsyncTaskHandlerReentrant 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 -diff --git a/src/main/java/org/spigotmc/RestartCommand.java b/src/main/java/org/spigotmc/RestartCommand.java -index aefea3a9a..123de5ac9 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 5bdcdcf9e..6dda6d168 100644 ---- a/src/main/java/org/spigotmc/WatchdogThread.java -+++ b/src/main/java/org/spigotmc/WatchdogThread.java -@@ -69,10 +69,11 @@ public class WatchdogThread extends Thread - long currentTime = monotonicMillis(); - if ( lastTick != 0 && currentTime > lastTick + earlyWarningEvery && !Boolean.getBoolean("disable.watchdog") ) - { -- boolean isLongTimeout = currentTime > lastTick + timeoutTime; -+ MinecraftServer server = MinecraftServer.getServer(); -+ boolean isLongTimeout = currentTime > lastTick + timeoutTime || !server.isRunning(); - // 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 +115,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 +136,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.25.1 - diff --git a/Spigot-Server-Patches/0469-Forced-Watchdog-Crash-support-and-Improve-Async-Shut.patch b/Spigot-Server-Patches/0469-Forced-Watchdog-Crash-support-and-Improve-Async-Shut.patch new file mode 100644 index 0000000000..fbbccf2e33 --- /dev/null +++ b/Spigot-Server-Patches/0469-Forced-Watchdog-Crash-support-and-Improve-Async-Shut.patch @@ -0,0 +1,330 @@ +From dcc37cb0137276aedac50c019b6558ec03cbb7b6 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 3de19c998..c7dc8787c 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 1ef7890da..e62ca0543 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/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java +index 3dbd75ad0..a386c1ab4 100644 +--- a/src/main/java/net/minecraft/server/MinecraftServer.java ++++ b/src/main/java/net/minecraft/server/MinecraftServer.java +@@ -144,6 +144,7 @@ public abstract class MinecraftServer extends IAsyncTaskHandlerReentrant 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 6d6fbf2f5..0e1703c9d 100644 +--- a/src/main/java/net/minecraft/server/PlayerList.java ++++ b/src/main/java/net/minecraft/server/PlayerList.java +@@ -411,7 +411,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 dc6d03062..bc8b90466 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 d554d4cf0..46e19d916 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 8cc0f66ce..dcc44be61 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 449e99d1b..899a52520 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 aefea3a9a..123de5ac9 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 5bdcdcf9e..acc880c7c 100644 +--- a/src/main/java/org/spigotmc/WatchdogThread.java ++++ b/src/main/java/org/spigotmc/WatchdogThread.java +@@ -67,12 +67,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 ( !server.isRunning() || (lastTick != 0 && 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 +115,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 +136,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.25.1 + diff --git a/Spigot-Server-Patches/0479-Optimise-entity-hard-collision-checking.patch b/Spigot-Server-Patches/0479-Optimise-entity-hard-collision-checking.patch index ede8383a38..16ec246d51 100644 --- a/Spigot-Server-Patches/0479-Optimise-entity-hard-collision-checking.patch +++ b/Spigot-Server-Patches/0479-Optimise-entity-hard-collision-checking.patch @@ -1,4 +1,4 @@ -From 9714a144f0d9467d7ad571d35a8603bc1acf1e94 Mon Sep 17 00:00:00 2001 +From b5b07ca3fba55030c3fad46887a64b39da500b32 Mon Sep 17 00:00:00 2001 From: Spottedleaf Date: Wed, 15 Apr 2020 18:08:53 -0700 Subject: [PATCH] Optimise entity hard collision checking @@ -178,10 +178,10 @@ index 4157e50e4..5135308fb 100644 return stream.filter(axisalignedbb1::c).map(VoxelShapes::a); diff --git a/src/main/java/net/minecraft/server/World.java b/src/main/java/net/minecraft/server/World.java -index d554d4cf0..5fa045998 100644 +index 46e19d916..09b03afff 100644 --- a/src/main/java/net/minecraft/server/World.java +++ b/src/main/java/net/minecraft/server/World.java -@@ -1170,6 +1170,32 @@ public abstract class World implements GeneratorAccess, AutoCloseable { +@@ -1172,6 +1172,32 @@ public abstract class World implements GeneratorAccess, AutoCloseable { return this.getChunkAt(i, j, ChunkStatus.FULL, false); }