diff options
Diffstat (limited to 'patches/server/0133-Properly-handle-async-calls-to-restart-the-server.patch')
| -rw-r--r-- | patches/server/0133-Properly-handle-async-calls-to-restart-the-server.patch | 290 |
1 files changed, 290 insertions, 0 deletions
diff --git a/patches/server/0133-Properly-handle-async-calls-to-restart-the-server.patch b/patches/server/0133-Properly-handle-async-calls-to-restart-the-server.patch new file mode 100644 index 0000000000..eb9dccee49 --- /dev/null +++ b/patches/server/0133-Properly-handle-async-calls-to-restart-the-server.patch @@ -0,0 +1,290 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Zach Brown <[email protected]> +Date: Fri, 12 May 2017 23:34:11 -0500 +Subject: [PATCH] Properly handle async calls to restart the server + +The watchdog thread calls the server restart function asynchronously. Prior to +this change, it attempted to do several non-safe operations from the watchdog +thread, rather than the main. Specifically, because of a separate upstream change, +it causes player entities to be ticked asynchronously, among other things. + +This is dangerous. + +This patch moves the old handling into a synchronous variant, for calls from the +restart command, and adds separate handling for async calls, such as those from +the watchdog thread. + +When calling from the watchdog thread, we cannot assume the main thread is in a +tickable state; it may be completely deadlocked. In order to handle this, we mark +the server as stopping, in order to account for situations where the server should +complete a tick reasonbly soon, i.e. 99% of cases. + +Should the server not enter a state where it is stopping within 10 seconds, We +will assume that the server has in fact deadlocked and will proceed to force +kill the server. + +This modification does not force restart the server should we actually enter a +deadlocked state where the server is stopping, whereas this will in most cases +exit within a reasonable amount of time, to put a fixed limit on a process that +will have plugins and worlds saving to the disk has a high potential to result +in corruption/dataloss. + +diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java +index 5a52369c1e4c462f401c4b6699a2b7376b8bc0e9..ec0fe27a6475cc3d097d394cf9694a17cba76332 100644 +--- a/src/main/java/net/minecraft/server/MinecraftServer.java ++++ b/src/main/java/net/minecraft/server/MinecraftServer.java +@@ -239,6 +239,7 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop<TickTa + private Map<ResourceKey<Level>, ServerLevel> levels; + private PlayerList playerList; + private volatile boolean running; ++ private volatile boolean isRestarting = false; // Paper - flag to signify we're attempting to restart + private boolean stopped; + private int tickCount; + private int ticksUntilAutosave; +@@ -934,7 +935,7 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop<TickTa + if (this.playerList != null) { + MinecraftServer.LOGGER.info("Saving players"); + this.playerList.saveAll(); +- this.playerList.removeAll(); ++ this.playerList.removeAll(this.isRestarting); // Paper + try { Thread.sleep(100); } catch (InterruptedException ex) {} // CraftBukkit - SPIGOT-625 - give server at least a chance to send packets + } + +@@ -1014,6 +1015,12 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop<TickTa + } + + public void halt(boolean waitForShutdown) { ++ // Paper start - allow passing of the intent to restart ++ this.safeShutdown(waitForShutdown, false); ++ } ++ public void safeShutdown(boolean waitForShutdown, boolean isRestarting) { ++ this.isRestarting = isRestarting; ++ // Paper end + this.running = false; + if (waitForShutdown) { + try { +diff --git a/src/main/java/net/minecraft/server/players/PlayerList.java b/src/main/java/net/minecraft/server/players/PlayerList.java +index 2753b821f17ea8950a80c5d0e96d7b367d00e068..5d7cf0bffae13ecbc460a0f0b74e110ef9380a14 100644 +--- a/src/main/java/net/minecraft/server/players/PlayerList.java ++++ b/src/main/java/net/minecraft/server/players/PlayerList.java +@@ -1174,8 +1174,15 @@ public abstract class PlayerList { + } + + public void removeAll() { ++ // Paper start - Extract method to allow for restarting flag ++ this.removeAll(false); ++ } ++ ++ public void removeAll(boolean isRestarting) { ++ // Paper end + // CraftBukkit start - disconnect safely + for (ServerPlayer player : this.players) { ++ if (isRestarting) player.connection.disconnect(org.spigotmc.SpigotConfig.restartMessage); else // Paper + player.connection.disconnect(this.server.server.shutdownMessage()); // CraftBukkit - add custom shutdown message // Paper - Adventure + } + // CraftBukkit end +diff --git a/src/main/java/org/spigotmc/RestartCommand.java b/src/main/java/org/spigotmc/RestartCommand.java +index 1e251d0f6de25a0a8c739c5f18ec5b949d4fc396..051b9e3a5d29a5840d596468e3ddd013bedc8da3 100644 +--- a/src/main/java/org/spigotmc/RestartCommand.java ++++ b/src/main/java/org/spigotmc/RestartCommand.java +@@ -45,86 +45,134 @@ public class RestartCommand extends Command + AsyncCatcher.enabled = false; // Disable async catcher incase it interferes with us + try + { +- String[] split = restartScript.split( " " ); +- if ( split.length > 0 && new File( split[0] ).isFile() ) ++ // Paper - extract method and cleanup ++ boolean isRestarting = addShutdownHook( restartScript ); ++ if ( isRestarting ) + { +- System.out.println( "Attempting to restart with " + restartScript ); ++ System.out.println( "Attempting to restart with " + SpigotConfig.restartScript ); ++ } else ++ { ++ System.out.println( "Startup script '" + SpigotConfig.restartScript + "' does not exist! Stopping server." ); ++ } ++ // Stop the watchdog ++ WatchdogThread.doStop(); + +- // Disable Watchdog +- WatchdogThread.doStop(); ++ shutdownServer( isRestarting ); ++ // Paper end ++ } catch ( Exception ex ) ++ { ++ ex.printStackTrace(); ++ } ++ } + +- // Kick all players +- for ( ServerPlayer p : (List<ServerPlayer>) MinecraftServer.getServer().getPlayerList().players ) +- { +- p.connection.disconnect(SpigotConfig.restartMessage); +- } +- // Give the socket a chance to send the packets +- try +- { +- Thread.sleep( 100 ); +- } catch ( InterruptedException ex ) +- { +- } +- // Close the socket so we can rebind with the new process +- MinecraftServer.getServer().getConnection().stop(); ++ // Paper start - sync copied from above with minor changes, async added ++ private static void shutdownServer(boolean isRestarting) ++ { ++ if ( MinecraftServer.getServer().isSameThread() ) ++ { ++ // Kick all players ++ for ( ServerPlayer p : com.google.common.collect.ImmutableList.copyOf( MinecraftServer.getServer().getPlayerList().players ) ) ++ { ++ p.connection.disconnect(SpigotConfig.restartMessage); ++ } ++ // Give the socket a chance to send the packets ++ try ++ { ++ Thread.sleep( 100 ); ++ } catch ( InterruptedException ex ) ++ { ++ } + +- // Give time for it to kick in +- try +- { +- Thread.sleep( 100 ); +- } catch ( InterruptedException ex ) +- { +- } ++ closeSocket(); + +- // Actually shutdown +- try +- { +- MinecraftServer.getServer().close(); +- } catch ( Throwable t ) +- { +- } ++ // Actually shutdown ++ try ++ { ++ MinecraftServer.getServer().close(); // calls stop() ++ } catch ( Throwable t ) ++ { ++ } ++ ++ // Actually stop the JVM ++ System.exit( 0 ); + +- // This will be done AFTER the server has completely halted +- Thread shutdownHook = new Thread() ++ } else ++ { ++ // Mark the server to shutdown at the end of the tick ++ MinecraftServer.getServer().safeShutdown( false, isRestarting ); ++ ++ // wait 10 seconds to see if we're actually going to try shutdown ++ try ++ { ++ Thread.sleep( 10000 ); ++ } ++ catch (InterruptedException ignored) ++ { ++ } ++ ++ // Check if we've actually hit a state where the server is going to safely shutdown ++ // if we have, let the server stop as usual ++ if (MinecraftServer.getServer().isStopped()) return; ++ ++ // If the server hasn't stopped by now, assume worse case and kill ++ closeSocket(); ++ System.exit( 0 ); ++ } ++ } ++ // Paper end ++ ++ // Paper - Split from moved code ++ private static void closeSocket() ++ { ++ // Close the socket so we can rebind with the new process ++ MinecraftServer.getServer().getConnection().stop(); ++ ++ // Give time for it to kick in ++ try ++ { ++ Thread.sleep( 100 ); ++ } catch ( InterruptedException ex ) ++ { ++ } ++ } ++ // Paper end ++ ++ // Paper start - copied from above and modified to return if the hook registered ++ private static boolean addShutdownHook(String restartScript) ++ { ++ String[] split = restartScript.split( " " ); ++ if ( split.length > 0 && new File( split[0] ).isFile() ) ++ { ++ Thread shutdownHook = new Thread() ++ { ++ @Override ++ public void run() + { +- @Override +- public void run() ++ try + { +- try ++ String os = System.getProperty( "os.name" ).toLowerCase(java.util.Locale.ENGLISH); ++ if ( os.contains( "win" ) ) + { +- String os = System.getProperty( "os.name" ).toLowerCase(java.util.Locale.ENGLISH); +- if ( os.contains( "win" ) ) +- { +- Runtime.getRuntime().exec( "cmd /c start " + restartScript ); +- } else +- { +- Runtime.getRuntime().exec( "sh " + restartScript ); +- } +- } catch ( Exception e ) ++ Runtime.getRuntime().exec( "cmd /c start " + restartScript ); ++ } else + { +- e.printStackTrace(); ++ Runtime.getRuntime().exec( "sh " + restartScript ); + } ++ } catch ( Exception e ) ++ { ++ e.printStackTrace(); + } +- }; +- +- shutdownHook.setDaemon( true ); +- Runtime.getRuntime().addShutdownHook( shutdownHook ); +- } else +- { +- System.out.println( "Startup script '" + SpigotConfig.restartScript + "' does not exist! Stopping server." ); +- +- // Actually shutdown +- try +- { +- MinecraftServer.getServer().close(); +- } catch ( Throwable t ) +- { + } +- } +- System.exit( 0 ); +- } catch ( Exception ex ) ++ }; ++ ++ shutdownHook.setDaemon( true ); ++ Runtime.getRuntime().addShutdownHook( shutdownHook ); ++ return true; ++ } else + { +- ex.printStackTrace(); ++ return false; + } + } ++ // Paper end ++ + } |