From 9bc57b8c28b8773512b0edd537e0ea5aa7ad7701 Mon Sep 17 00:00:00 2001 From: Zach Brown 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. Therefore, we kill that thread right then and there. diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java index 40514d042..1d0dc7a0d 100644 --- a/src/main/java/net/minecraft/server/MinecraftServer.java +++ b/src/main/java/net/minecraft/server/MinecraftServer.java @@ -1612,6 +1612,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/org/spigotmc/RestartCommand.java b/src/main/java/org/spigotmc/RestartCommand.java index 49768734d..35c828805 100644 --- a/src/main/java/org/spigotmc/RestartCommand.java +++ b/src/main/java/org/spigotmc/RestartCommand.java @@ -52,36 +52,7 @@ public class RestartCommand extends Command // Disable Watchdog WatchdogThread.doStop(); - // 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(); - - // Give time for it to kick in - try - { - Thread.sleep( 100 ); - } catch ( InterruptedException ex ) - { - } - - // Actually shutdown - try - { - MinecraftServer.getServer().stop(); - } catch ( Throwable t ) - { - } + shutdownServer(); // Paper - Moved to function that will handle sync and async // This will be done AFTER the server has completely halted Thread shutdownHook = new Thread() @@ -129,4 +100,53 @@ public class RestartCommand extends Command ex.printStackTrace(); } } + + // Paper start - sync copied from above with minor changes, async added + private static void shutdownServer() + { + 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 ) + { + } + + closeSocket(); + + // Actually shutdown + try + { + MinecraftServer.getServer().stop(); + } catch ( Throwable t ) + { + } + } else + { + closeSocket(); + MinecraftServer.getServer().getServerThread().stop(); + } + } + + // 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 } -- 2.13.0