Paper/Spigot-Server-Patches/0149-Properly-handle-async-calls-to-restart-the-server.patch
Spottedleaf 7c640a1ae2 Updated Upstream (Bukkit/CraftBukkit/Spigot) (#2415)
* fixup patch and rebuild

* 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

Bukkit Changes:
bde198c9 SPIGOT-5246: PlayerQuitEvent.get/setQuitMessage() is incorrectly marked as NotNull
24ad5a79 SPIGOT-5240: Vector.angle not valid for angles very close to each other
a143db9a SPIGOT-5231: ShotAtAngle API for Fireworks
10db5c3d SPIGOT-5226: Update Javadoc of PlayerDeathEvent

CraftBukkit Changes:
1ec1b05e SPIGOT-5245: Unneeded cast to WorldNBTStorage in CraftWorld#getWorldFolder
e5e8eec2 SPIGOT-5241: setAttributeModifiers does not work on untouched stack
803eaa31 SPIGOT-5231: ShotAtAngle API for Fireworks
7881d2ae SPIGOT-5237: Horses, pigs do not drop their inventory
06efc9ec Don't accept connections until all plugins have enabled
da62a66a SPIGOT-5225: World handle isn't closed if world is unloaded without saving
104b3831 SPIGOT-5222: Cannot get Long values from Entity memory
f0b3fe43 SPIGOT-5220: Server CPU usage reaches 100% when stdin is null

Spigot Changes:
e5b1b5db SPIGOT-5235: Destroy expired area effect clouds / fireworks that are inactive
cbcc8e87 Make region files more reliable to write to
8887c5f4 Remove redundant late-bind option
dac29063 Rebuild patches

* Preserve old flush on save flag for reliable regionfiles

Originally this patch was in paper

* Fix some issues with the death event

- Entities potentially entering a glitched state to the client where
they appear to be falling over
- Donkeys losing their chest if the event was cancelled (only an
issue since the upstream merge)
- Some wither death logic running for an entity killed by a wither
2019-08-05 11:35:40 -05:00

312 lines
12 KiB
Diff

From 7dc83c0fff37f349a9d529e535df8226404a49b2 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 6a2933f4ff..f621350ea6 100644
--- a/src/main/java/net/minecraft/server/MinecraftServer.java
+++ b/src/main/java/net/minecraft/server/MinecraftServer.java
@@ -88,6 +88,7 @@ public abstract class MinecraftServer extends IAsyncTaskHandlerReentrant<TickTas
public final Map<DimensionManager, WorldServer> worldServer = Maps.newLinkedHashMap(); // CraftBukkit - keep order, k+v already use identity methods
private PlayerList playerList;
private volatile boolean isRunning = true;
+ private volatile boolean isRestarting = false; // Paper - flag to signify we're attempting to restart
private boolean isStopped;
private int ticks;
protected final Proxy proxy;
@@ -714,7 +715,7 @@ public abstract class MinecraftServer extends IAsyncTaskHandlerReentrant<TickTas
if (this.playerList != null) {
MinecraftServer.LOGGER.info("Saving players");
this.playerList.savePlayers();
- this.playerList.shutdown();
+ this.playerList.shutdown(this.isRestarting); // Paper
try { Thread.sleep(100); } catch (InterruptedException ex) {} // CraftBukkit - SPIGOT-625 - give server at least a chance to send packets
}
@@ -768,8 +769,13 @@ public abstract class MinecraftServer extends IAsyncTaskHandlerReentrant<TickTas
return this.isRunning;
}
+ // Paper start - allow passing of the intent to restart
public void safeShutdown(boolean flag) {
+ this.safeShutdown(flag, false);
+ }
+ public void safeShutdown(boolean flag, boolean isRestarting) {
this.isRunning = false;
+ this.isRestarting = isRestarting;
if (flag) {
try {
this.serverThread.join();
@@ -779,6 +785,7 @@ public abstract class MinecraftServer extends IAsyncTaskHandlerReentrant<TickTas
}
}
+ // Paper end
// Spigot Start
private static double calcTps(double avg, double exp, double tps)
diff --git a/src/main/java/net/minecraft/server/PlayerList.java b/src/main/java/net/minecraft/server/PlayerList.java
index ccf8b7c707..d23e4e5e3d 100644
--- a/src/main/java/net/minecraft/server/PlayerList.java
+++ b/src/main/java/net/minecraft/server/PlayerList.java
@@ -1041,10 +1041,15 @@ public abstract class PlayerList {
entityplayer.playerInteractManager.b(generatoraccess.getWorldData().getGameType());
}
+ // Paper start - Extract method to allow for restarting flag
public void shutdown() {
+ this.shutdown(false);
+ }
+
+ public void shutdown(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
@@ -1055,6 +1060,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 ccea803f58..aefea3a9a8 100644
--- a/src/main/java/org/spigotmc/RestartCommand.java
+++ b/src/main/java/org/spigotmc/RestartCommand.java
@@ -46,86 +46,134 @@ 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 " + 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 ( 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();
+ // 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 )
+ {
+ }
- // 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().getServerConnection().b();
+
+ // 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
+
}
--
2.22.0