291 Zeilen
12 KiB
Diff
291 Zeilen
12 KiB
Diff
From 0000000000000000000000000000000000000000 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 56d51073eb0cd902fa2c1e790201f74d120b149f..7335f9bb936eeb585ee077b0b9c461d7946d6134 100644
|
|
--- a/src/main/java/net/minecraft/server/MinecraftServer.java
|
|
+++ b/src/main/java/net/minecraft/server/MinecraftServer.java
|
|
@@ -242,6 +242,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;
|
|
@@ -938,7 +939,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
|
|
}
|
|
|
|
@@ -1018,6 +1019,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 f47c1ccba18b98854fb3671ed5bbce6548a5ef27..c2faf513ad435f979d889d43809e2caa92d11b18 100644
|
|
--- a/src/main/java/net/minecraft/server/players/PlayerList.java
|
|
+++ b/src/main/java/net/minecraft/server/players/PlayerList.java
|
|
@@ -1143,8 +1143,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(net.kyori.adventure.text.serializer.legacy.LegacyComponentSerializer.legacySection().deserialize(org.spigotmc.SpigotConfig.restartMessage)); else // Paper
|
|
player.connection.disconnect(java.util.Objects.requireNonNullElseGet(this.server.server.shutdownMessage(), net.kyori.adventure.text.Component::empty)); // 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 c30f9899ec5529e49919367ab4e0ae797b429afa..9a1fffa0c07c24eed8bd6ea8f3cd26c9f926c54d 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
|
|
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( CraftChatMessage.fromStringOrEmpty( 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( CraftChatMessage.fromStringOrEmpty( 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
|
|
+
|
|
}
|