From 0000000000000000000000000000000000000000 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/network/ServerGamePacketListenerImpl.java b/src/main/java/net/minecraft/server/network/ServerGamePacketListenerImpl.java index da9f4b3337b49597c17b50964656457cd629a0b7..22c2c121bbcc7b0e15d73d20c9cc83d5fb085e5f 100644 --- a/src/main/java/net/minecraft/server/network/ServerGamePacketListenerImpl.java +++ b/src/main/java/net/minecraft/server/network/ServerGamePacketListenerImpl.java @@ -1846,6 +1846,29 @@ public class ServerGamePacketListenerImpl implements ServerPlayerConnection, Ser } 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; + } + }; + server.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.getChatVisibility() == ChatVisiblity.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 5e5163d180742c33fc6f045530906d76df758c96..747cdfbbd5d22094dcc43208036accff70356c7e 100644 --- a/src/main/java/org/bukkit/craftbukkit/CraftServer.java +++ b/src/main/java/org/bukkit/craftbukkit/CraftServer.java @@ -767,6 +767,28 @@ public final class CraftServer implements Server { Validate.notNull(commandLine, "CommandLine cannot be null"); org.spigotmc.AsyncCatcher.catchOp("command dispatch"); // Spigot + // 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 (this.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 19c44daaa407b7c1c7a7ffe56fef8c8814c6d5b2..6a073a9dc44d93eba296a0e18a9c7be8a7881725 100644 --- a/src/main/java/org/bukkit/craftbukkit/util/ServerShutdownThread.java +++ b/src/main/java/org/bukkit/craftbukkit/util/ServerShutdownThread.java @@ -13,6 +13,7 @@ public class ServerShutdownThread extends Thread { public void run() { try { org.spigotmc.AsyncCatcher.enabled = false; // Spigot + org.spigotmc.AsyncCatcher.shuttingDown = true; // Paper this.server.close(); } finally { try { diff --git a/src/main/java/org/spigotmc/AsyncCatcher.java b/src/main/java/org/spigotmc/AsyncCatcher.java index bbf0d9d9c44fe8d7add2f978994ec129420814c7..7a23b56752f6733ee626a8b1e4c3b78591855c4e 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 882e93ad4471e3688f2fcfb1e6f16926786ee5e7..94d8ba376cd1f024b244654cac9bb62bb19e3060 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 private static void restart(final String restartScript) { AsyncCatcher.enabled = false; // Disable async catcher incase it interferes with us + org.spigotmc.AsyncCatcher.shuttingDown = true; // Paper try { String[] split = restartScript.split( " " );