From 09e8ebb6054e3f37de7d78ab784147cbf62985d4 Mon Sep 17 00:00:00 2001 From: Aikar Date: Thu, 3 Mar 2016 01:17:12 -0600 Subject: [PATCH] Ensure commands are not ran async Plugins calling Player.chat("/foo") or Server.dispatchCommand() could trigger the server to execute a command while on another thread. These commands would then process EXPECTING to be on the main thread, leaving to very hard to trace concurrency issues. This change will synchronize the command execution back to the main thread, causing a big slowdown in execution but throwing an exception at same time to raise awareness that it is happening so that plugin authors can fix their code to stop executing commands async. diff --git a/src/main/java/net/minecraft/server/PlayerConnection.java b/src/main/java/net/minecraft/server/PlayerConnection.java index da25a8bea..48379f414 100644 --- a/src/main/java/net/minecraft/server/PlayerConnection.java +++ b/src/main/java/net/minecraft/server/PlayerConnection.java @@ -1311,6 +1311,29 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { } if (!async && s.startsWith("/")) { + // Paper Start + if (!org.spigotmc.AsyncCatcher.shuttingDown && !org.bukkit.Bukkit.isPrimaryThread()) { + final String fCommandLine = s; + MinecraftServer.LOGGER.log(org.apache.logging.log4j.Level.ERROR, "Command Dispatched Async: " + fCommandLine); + MinecraftServer.LOGGER.log(org.apache.logging.log4j.Level.ERROR, "Please notify author of plugin causing this execution to fix this bug! see: http://bit.ly/1oSiM6C", new Throwable()); + Waitable wait = new Waitable() { + @Override + protected Object evaluate() { + chat(fCommandLine, false); + return null; + } + }; + minecraftServer.processQueue.add(wait); + try { + wait.get(); + return; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); // This is proper habit for java. If we aren't handling it, pass it on! + } catch (Exception e) { + throw new RuntimeException("Exception processing chat command", e.getCause()); + } + } + // Paper End this.handleCommand(s); } else if (this.player.getChatFlags() == EntityHuman.EnumChatVisibility.SYSTEM) { // Do nothing, this is coming from a plugin diff --git a/src/main/java/org/bukkit/craftbukkit/CraftServer.java b/src/main/java/org/bukkit/craftbukkit/CraftServer.java index bfddd1685..462ad1024 100644 --- a/src/main/java/org/bukkit/craftbukkit/CraftServer.java +++ b/src/main/java/org/bukkit/craftbukkit/CraftServer.java @@ -645,6 +645,29 @@ public final class CraftServer implements Server { Validate.notNull(sender, "Sender cannot be null"); Validate.notNull(commandLine, "CommandLine cannot be null"); + // Paper Start + if (!org.spigotmc.AsyncCatcher.shuttingDown && !Bukkit.isPrimaryThread()) { + final CommandSender fSender = sender; + final String fCommandLine = commandLine; + Bukkit.getLogger().log(Level.SEVERE, "Command Dispatched Async: " + commandLine); + Bukkit.getLogger().log(Level.SEVERE, "Please notify author of plugin causing this execution to fix this bug! see: http://bit.ly/1oSiM6C", new Throwable()); + org.bukkit.craftbukkit.util.Waitable wait = new org.bukkit.craftbukkit.util.Waitable() { + @Override + protected Boolean evaluate() { + return dispatchCommand(fSender, fCommandLine); + } + }; + net.minecraft.server.MinecraftServer.getServer().processQueue.add(wait); + try { + return wait.get(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); // This is proper habit for java. If we aren't handling it, pass it on! + } catch (Exception e) { + throw new RuntimeException("Exception processing dispatch command", e.getCause()); + } + } + // Paper End + if (commandMap.dispatch(sender, commandLine)) { return true; } diff --git a/src/main/java/org/bukkit/craftbukkit/util/ServerShutdownThread.java b/src/main/java/org/bukkit/craftbukkit/util/ServerShutdownThread.java index a0cdd2317..984df4083 100644 --- a/src/main/java/org/bukkit/craftbukkit/util/ServerShutdownThread.java +++ b/src/main/java/org/bukkit/craftbukkit/util/ServerShutdownThread.java @@ -14,6 +14,7 @@ public class ServerShutdownThread extends Thread { public void run() { try { org.spigotmc.AsyncCatcher.enabled = false; // Spigot + org.spigotmc.AsyncCatcher.shuttingDown = true; // Paper server.stop(); } catch (ExceptionWorldConflict ex) { ex.printStackTrace(); diff --git a/src/main/java/org/spigotmc/AsyncCatcher.java b/src/main/java/org/spigotmc/AsyncCatcher.java index 4b3aa85c9..e44c23016 100644 --- a/src/main/java/org/spigotmc/AsyncCatcher.java +++ b/src/main/java/org/spigotmc/AsyncCatcher.java @@ -6,6 +6,7 @@ public class AsyncCatcher { public static boolean enabled = true; + public static boolean shuttingDown = false; // Paper public static void catchOp(String reason) { diff --git a/src/main/java/org/spigotmc/RestartCommand.java b/src/main/java/org/spigotmc/RestartCommand.java index 49768734d..947c43a5d 100644 --- a/src/main/java/org/spigotmc/RestartCommand.java +++ b/src/main/java/org/spigotmc/RestartCommand.java @@ -43,6 +43,7 @@ public class RestartCommand extends Command public static void restart(final File script) { AsyncCatcher.enabled = false; // Disable async catcher incase it interferes with us + org.spigotmc.AsyncCatcher.shuttingDown = true; // Paper try { if ( script.isFile() ) -- 2.16.1