311 lines
12 KiB
Diff
311 lines
12 KiB
Diff
From cad241a6391082af16b0a6472617360477d07168 Mon Sep 17 00:00:00 2001
|
|
From: Zach Brown <zach.brown@destroystokyo.com>
|
|
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 13c6b5ccd..908a5d273 100644
|
|
--- a/src/main/java/net/minecraft/server/MinecraftServer.java
|
|
+++ b/src/main/java/net/minecraft/server/MinecraftServer.java
|
|
@@ -71,6 +71,7 @@ public abstract class MinecraftServer implements ICommandListener, Runnable, IAs
|
|
public WorldServer[] worldServer;
|
|
private PlayerList v;
|
|
private boolean isRunning = true;
|
|
+ private boolean isRestarting = false; // Paper - flag to signify we're attempting to restart
|
|
private boolean isStopped;
|
|
private int ticks;
|
|
protected final Proxy e;
|
|
@@ -489,7 +490,7 @@ public abstract class MinecraftServer implements ICommandListener, Runnable, IAs
|
|
if (this.v != null) {
|
|
MinecraftServer.LOGGER.info("Saving players");
|
|
this.v.savePlayers();
|
|
- this.v.u();
|
|
+ this.v.u(isRestarting);
|
|
try { Thread.sleep(100); } catch (InterruptedException ex) {} // CraftBukkit - SPIGOT-625 - give server at least a chance to send packets
|
|
}
|
|
|
|
@@ -546,10 +547,18 @@ public abstract class MinecraftServer implements ICommandListener, Runnable, IAs
|
|
return this.isRunning;
|
|
}
|
|
|
|
+ // Paper start - allow passing of the intent to restart
|
|
public void safeShutdown() {
|
|
+ safeShutdown(false);
|
|
+ }
|
|
+
|
|
+ public void safeShutdown(boolean isRestarting) {
|
|
this.isRunning = false;
|
|
+ this.isRestarting = isRestarting;
|
|
}
|
|
|
|
+ // Paper end
|
|
+
|
|
// Paper start - Further improve server tick loop
|
|
private static final int TPS = 20;
|
|
private static final long SEC_IN_NANO = 1000000000;
|
|
@@ -1615,6 +1624,7 @@ public abstract class MinecraftServer implements ICommandListener, Runnable, IAs
|
|
return this.ab;
|
|
}
|
|
|
|
+ public final Thread getServerThread() { return this.aI(); } // Paper - OBFHELPER
|
|
public Thread aI() {
|
|
return this.serverThread;
|
|
}
|
|
diff --git a/src/main/java/net/minecraft/server/PlayerList.java b/src/main/java/net/minecraft/server/PlayerList.java
|
|
index 91136a8d8..cc6a209ea 100644
|
|
--- a/src/main/java/net/minecraft/server/PlayerList.java
|
|
+++ b/src/main/java/net/minecraft/server/PlayerList.java
|
|
@@ -1375,10 +1375,15 @@ public abstract class PlayerList {
|
|
entityplayer.playerInteractManager.b(world.getWorldData().getGameType());
|
|
}
|
|
|
|
+ // Paper start - Extract method to allow for restarting flag
|
|
public void u() {
|
|
+ u(false);
|
|
+ }
|
|
+
|
|
+ public void u(boolean isRestarting) {
|
|
// CraftBukkit start - disconnect safely
|
|
for (EntityPlayer player : this.players) {
|
|
- player.playerConnection.disconnect(this.server.server.getShutdownMessage()); // CraftBukkit - add custom shutdown message
|
|
+ player.playerConnection.disconnect(!isRestarting ? this.server.server.getShutdownMessage() : org.spigotmc.SpigotConfig.restartMessage); // CraftBukkit - add custom shutdown message // Paper - add isRestarting flag
|
|
}
|
|
// CraftBukkit end
|
|
// Paper start - Remove collideRule team if it exists
|
|
@@ -1389,6 +1394,7 @@ public abstract class PlayerList {
|
|
}
|
|
// Paper end
|
|
}
|
|
+ // Paper end
|
|
|
|
// CraftBukkit start
|
|
public void sendMessage(IChatBaseComponent[] iChatBaseComponents) {
|
|
diff --git a/src/main/java/org/spigotmc/RestartCommand.java b/src/main/java/org/spigotmc/RestartCommand.java
|
|
index 947c43a5d..f15fd9f37 100644
|
|
--- a/src/main/java/org/spigotmc/RestartCommand.java
|
|
+++ b/src/main/java/org/spigotmc/RestartCommand.java
|
|
@@ -46,88 +46,123 @@ public class RestartCommand extends Command
|
|
org.spigotmc.AsyncCatcher.shuttingDown = true; // Paper
|
|
try
|
|
{
|
|
- if ( script.isFile() )
|
|
+ // Paper - extract method and cleanup
|
|
+ boolean isRestarting = addShutdownHook(script);
|
|
+ if (isRestarting) {
|
|
+ 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();
|
|
+
|
|
+ shutdownServer(isRestarting);
|
|
+ } catch ( Exception ex )
|
|
+ {
|
|
+ ex.printStackTrace();
|
|
+ }
|
|
+ }
|
|
+
|
|
+ // Paper start - sync copied from above with minor changes, async added
|
|
+ private static void shutdownServer(boolean isRestarting)
|
|
+ {
|
|
+ if (MinecraftServer.getServer().isMainThread())
|
|
+ {
|
|
+ // Kick all players
|
|
+ for ( EntityPlayer p : com.google.common.collect.ImmutableList.copyOf( MinecraftServer.getServer().getPlayerList().players ) )
|
|
+ {
|
|
+ p.playerConnection.disconnect(SpigotConfig.restartMessage);
|
|
+ }
|
|
+ // Give the socket a chance to send the packets
|
|
+ try
|
|
+ {
|
|
+ Thread.sleep( 100 );
|
|
+ } catch ( InterruptedException ex )
|
|
{
|
|
- System.out.println( "Attempting to restart with " + SpigotConfig.restartScript );
|
|
+ }
|
|
|
|
- // Disable Watchdog
|
|
- WatchdogThread.doStop();
|
|
+ closeSocket();
|
|
|
|
- // Kick all players
|
|
- for ( EntityPlayer p : (List< EntityPlayer>) MinecraftServer.getServer().getPlayerList().players )
|
|
- {
|
|
- p.playerConnection.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().getServerConnection().b();
|
|
+ // Actually shutdown
|
|
+ try
|
|
+ {
|
|
+ MinecraftServer.getServer().stop();
|
|
+ } catch ( Throwable t )
|
|
+ {
|
|
+ }
|
|
|
|
- // Give time for it to kick in
|
|
- try
|
|
- {
|
|
- Thread.sleep( 100 );
|
|
- } catch ( InterruptedException ex )
|
|
- {
|
|
- }
|
|
+ // Actually stop the JVM
|
|
+ System.exit(0);
|
|
|
|
- // Actually shutdown
|
|
- try
|
|
- {
|
|
- MinecraftServer.getServer().stop();
|
|
- } catch ( Throwable t )
|
|
- {
|
|
- }
|
|
+ } else
|
|
+ {
|
|
+ // Mark the server to shutdown at the end of the tick
|
|
+ MinecraftServer.getServer().safeShutdown(isRestarting);
|
|
|
|
- // This will be done AFTER the server has completely halted
|
|
- Thread shutdownHook = new Thread()
|
|
- {
|
|
- @Override
|
|
- public void run()
|
|
- {
|
|
- try
|
|
- {
|
|
- String os = System.getProperty( "os.name" ).toLowerCase(java.util.Locale.ENGLISH);
|
|
- if ( os.contains( "win" ) )
|
|
- {
|
|
- Runtime.getRuntime().exec( "cmd /c start " + script.getPath() );
|
|
- } else
|
|
- {
|
|
- Runtime.getRuntime().exec( new String[]
|
|
- {
|
|
- "sh", script.getPath()
|
|
- } );
|
|
- }
|
|
- } catch ( Exception e )
|
|
- {
|
|
- e.printStackTrace();
|
|
- }
|
|
- }
|
|
- };
|
|
|
|
- shutdownHook.setDaemon( true );
|
|
- Runtime.getRuntime().addShutdownHook( shutdownHook );
|
|
- } else
|
|
+ // wait 10 seconds to see if we're actually going to try shutdown
|
|
+ try
|
|
+ {
|
|
+ Thread.sleep(10000);
|
|
+ }
|
|
+ catch (InterruptedException ignored)
|
|
{
|
|
- System.out.println( "Startup script '" + SpigotConfig.restartScript + "' does not exist! Stopping server." );
|
|
-
|
|
- // Actually shutdown
|
|
- try
|
|
- {
|
|
- MinecraftServer.getServer().stop();
|
|
- } catch ( Throwable t )
|
|
- {
|
|
- }
|
|
}
|
|
+
|
|
+ // 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 );
|
|
- } catch ( Exception ex )
|
|
+ }
|
|
+ }
|
|
+
|
|
+ // Paper - Split from moved code
|
|
+ private static void closeSocket() {
|
|
+ // Close the socket so we can rebind with the new process
|
|
+ MinecraftServer.getServer().getServerConnection().b();
|
|
+
|
|
+ // Give time for it to kick in
|
|
+ try
|
|
{
|
|
- ex.printStackTrace();
|
|
+ Thread.sleep( 100 );
|
|
+ } catch ( InterruptedException ex )
|
|
+ {
|
|
+ }
|
|
+ }
|
|
+ // Paper end
|
|
+
|
|
+ // Paper - copied from above and modified to return if the hook registered
|
|
+ private static boolean addShutdownHook(final File script) {
|
|
+
|
|
+ if (script.isFile()) {
|
|
+ Thread shutdownHook = new Thread() {
|
|
+ @Override
|
|
+ public void run() {
|
|
+ try {
|
|
+ String os = System.getProperty("os.name").toLowerCase(java.util.Locale.ENGLISH);
|
|
+ if (os.contains("win")) {
|
|
+ Runtime.getRuntime().exec("cmd /c start " + script.getPath());
|
|
+ } else {
|
|
+ Runtime.getRuntime().exec(new String[]
|
|
+ {
|
|
+ "sh", script.getPath()
|
|
+ });
|
|
+ }
|
|
+ } catch (Exception e) {
|
|
+ e.printStackTrace();
|
|
+ }
|
|
+ }
|
|
+ };
|
|
+
|
|
+ shutdownHook.setDaemon(true);
|
|
+ Runtime.getRuntime().addShutdownHook(shutdownHook);
|
|
+ return true;
|
|
+ } else {
|
|
+ return false;
|
|
}
|
|
}
|
|
}
|
|
--
|
|
2.16.2
|
|
|