papermc/Spigot-Server-Patches/0173-Properly-handle-async-calls-to-restart-the-server.patch
Shane Freeder ea855e2b46 Updated Upstream (Bukkit/CraftBukkit/Spigot)
Upstream has released updates that appears to apply and compile correctly.
This update has not been tested by PaperMC and as with ANY update, please do your own testing

Developers!: You will need to clean up your work/Minecraft/1.13.2 folder
for this

Also, restore a patch that was dropped in the last upstream

Bukkit Changes:
279eeab3 Fix command description not being set
96e2bb18 Remove debug print from SyntheticEventTest

CraftBukkit Changes:
d3ed1516 Fix dangerously threaded beacons
217a293d Don't relocate joptsimple to allow --help to work.
1be05a21 Prepare for imminent Java 12 release
a49270b2 Mappings Update
5259d80c SPIGOT-4669: Fix PlayerTeleportEvent coordinates for relative teleports

Spigot Changes:
e6eb36f2 Rebuild patches
2019-03-20 01:55:16 +00:00

298 lines
12 KiB
Diff

From d7b4a418de5fffaf1e4667ae79065bba032605fe 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 6283c774d..db511c1fe 100644
--- a/src/main/java/net/minecraft/server/MinecraftServer.java
+++ b/src/main/java/net/minecraft/server/MinecraftServer.java
@@ -85,6 +85,7 @@ public abstract class MinecraftServer implements IAsyncTaskHandler, IMojangStati
public final Map<DimensionManager, WorldServer> worldServer = Maps.newLinkedHashMap(); // CraftBukkit - keep order, k+v already use identity methods
private PlayerList playerList;
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 c;
@@ -655,7 +656,7 @@ public abstract class MinecraftServer implements IAsyncTaskHandler, IMojangStati
if (this.playerList != null) {
MinecraftServer.LOGGER.info("Saving players");
this.playerList.savePlayers();
- this.playerList.u();
+ this.playerList.u(isRestarting); // Paper
try { Thread.sleep(100); } catch (InterruptedException ex) {} // CraftBukkit - SPIGOT-625 - give server at least a chance to send packets
}
@@ -705,9 +706,16 @@ public abstract class MinecraftServer implements IAsyncTaskHandler, IMojangStati
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
private boolean canSleepForTick() {
return System.nanoTime() - lastTick + catchupTime < TICK_TIME; // Paper - improved "are we lagging" check to match our own
diff --git a/src/main/java/net/minecraft/server/PlayerList.java b/src/main/java/net/minecraft/server/PlayerList.java
index 9570a8800..9d44dcb3b 100644
--- a/src/main/java/net/minecraft/server/PlayerList.java
+++ b/src/main/java/net/minecraft/server/PlayerList.java
@@ -1336,10 +1336,15 @@ public abstract class PlayerList {
entityplayer.playerInteractManager.b(generatoraccess.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
@@ -1350,6 +1355,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 944151d14..061cbe7fc 100644
--- a/src/main/java/org/spigotmc/RestartCommand.java
+++ b/src/main/java/org/spigotmc/RestartCommand.java
@@ -46,86 +46,123 @@ public class RestartCommand extends Command
org.spigotmc.AsyncCatcher.shuttingDown = true; // Paper
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 " + 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 ) )
{
- System.out.println( "Attempting to restart with " + restartScript );
+ p.playerConnection.disconnect(SpigotConfig.restartMessage);
+ }
+ // Give the socket a chance to send the packets
+ try
+ {
+ Thread.sleep( 100 );
+ } catch ( InterruptedException ex )
+ {
+ }
- // 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 " + restartScript );
- } else
- {
- 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().stop();
- } catch ( Throwable t )
- {
- }
+ // 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 );
- } 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 String restartScript) {
+
+ String[] split = restartScript.split( " " );
+ if ( split.length > 0 && new File( split[0] ).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 " + restartScript);
+ } else {
+ Runtime.getRuntime().exec( "sh " + restartScript );
+ }
+ } catch (Exception e) {
+ e.printStackTrace();
+ }
+ }
+ };
+
+ shutdownHook.setDaemon(true);
+ Runtime.getRuntime().addShutdownHook(shutdownHook);
+ return true;
+ } else {
+ return false;
}
}
}
--
2.21.0