From 2aa53ac9f67c953bdeac7f66834cdf8fb343799d 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 8df30e3d0..df3077c9d 100644 --- a/src/main/java/net/minecraft/server/MinecraftServer.java +++ b/src/main/java/net/minecraft/server/MinecraftServer.java @@ -1630,6 +1630,7 @@ public abstract class MinecraftServer implements Runnable, ICommandListener, 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.windows.1