From be47051b232a1fb30b960b87152f38ebe5fbac3f Mon Sep 17 00:00:00 2001 From: Sotr Date: Sun, 31 Mar 2019 15:31:25 +0800 Subject: [PATCH] Ensures thread safety, hopefully fixes GH-80 --- .../co/aikar/timings/ThreadAssertion.java | 10 ++++++--- .../java/co/aikar/timings/TimingHandler.java | 16 +++++++------- .../server/core/AkarinAsyncExecutor.java | 21 ++++++++++++------- .../net/minecraft/server/LoginListener.java | 5 +++++ .../java/net/minecraft/server/MCUtil.java | 7 ++++--- .../net/minecraft/server/MinecraftServer.java | 2 +- .../minecraft/server/PlayerConnection.java | 2 +- .../org/bukkit/craftbukkit/CraftServer.java | 4 ++-- src/main/java/org/spigotmc/AsyncCatcher.java | 2 +- 9 files changed, 41 insertions(+), 28 deletions(-) diff --git a/src/api/main/java/co/aikar/timings/ThreadAssertion.java b/src/api/main/java/co/aikar/timings/ThreadAssertion.java index ad4c11b7d..c0e501e8a 100644 --- a/src/api/main/java/co/aikar/timings/ThreadAssertion.java +++ b/src/api/main/java/co/aikar/timings/ThreadAssertion.java @@ -3,11 +3,15 @@ package co.aikar.timings; public class ThreadAssertion { private static boolean mainThread; - public static boolean isMainThread() { + public static boolean is() { return mainThread; } - static boolean setMainThread(boolean is) { - return mainThread = is; + static void start() { + mainThread = false; + } + + public static void close() { + mainThread = false; } } \ No newline at end of file diff --git a/src/api/main/java/co/aikar/timings/TimingHandler.java b/src/api/main/java/co/aikar/timings/TimingHandler.java index d01f2b731..42bb3113d 100644 --- a/src/api/main/java/co/aikar/timings/TimingHandler.java +++ b/src/api/main/java/co/aikar/timings/TimingHandler.java @@ -57,7 +57,6 @@ class TimingHandler implements Timing { private boolean added; private boolean timed; private boolean enabled; - private boolean unsafe; // Akarin TimingHandler(@Nonnull TimingIdentifier id) { // Akarin - javax.annotation this.identifier = id; @@ -114,7 +113,7 @@ class TimingHandler implements Timing { @Override public Timing startTimingUnsafe() { if (enabled && ++timingDepth == 1) { - unsafe = true; + ThreadAssertion.close(); // Akarin end start = System.nanoTime(); TIMING_STACK.addLast(this); @@ -124,11 +123,11 @@ class TimingHandler implements Timing { // Akarin start @Override public Timing startTiming(boolean assertThread) { - if (enabled && (ThreadAssertion.isMainThread() || Bukkit.isPrimaryThread()) && ++timingDepth == 1) { + if (enabled && (ThreadAssertion.is() || Bukkit.isPrimaryThread()) && ++timingDepth == 1) { start = System.nanoTime(); TIMING_STACK.addLast(this); if (assertThread && AkarinGlobalConfig.lazyThreadAssertion) - ThreadAssertion.setMainThread(true); + ThreadAssertion.start(); } return this; } @@ -144,13 +143,13 @@ class TimingHandler implements Timing { addDiff(System.nanoTime() - start, TIMING_STACK.peekLast()); start = 0; - unsafe = false; + ThreadAssertion.close(); } } // Akarin end public void stopTiming() { - if (enabled && timingDepth > 0 && (ThreadAssertion.isMainThread() || Bukkit.isPrimaryThread()) && --timingDepth == 0 && start != 0) { // Akarin + if (enabled && timingDepth > 0 && (ThreadAssertion.is() || Bukkit.isPrimaryThread()) && --timingDepth == 0 && start != 0) { // Akarin TimingHandler last; while ((last = TIMING_STACK.removeLast()) != this) { last.timingDepth = 0; @@ -167,8 +166,7 @@ class TimingHandler implements Timing { start = 0; // Akarin start if (AkarinGlobalConfig.lazyThreadAssertion) - ThreadAssertion.setMainThread(false); - unsafe = false; + ThreadAssertion.close(); // Akarin end } } @@ -231,7 +229,7 @@ class TimingHandler implements Timing { */ @Override public void close() { - if (unsafe) stopTimingUnsafe(); else stopTimingIfSync(); // Akarin + if (ThreadAssertion.is()) stopTimingUnsafe(); else stopTimingIfSync(); // Akarin } public boolean isSpecial() { diff --git a/src/main/java/io/akarin/server/core/AkarinAsyncExecutor.java b/src/main/java/io/akarin/server/core/AkarinAsyncExecutor.java index 24c05d851..386919e27 100644 --- a/src/main/java/io/akarin/server/core/AkarinAsyncExecutor.java +++ b/src/main/java/io/akarin/server/core/AkarinAsyncExecutor.java @@ -7,18 +7,19 @@ import java.util.concurrent.Future; import com.google.common.util.concurrent.ThreadFactoryBuilder; +import co.aikar.timings.ThreadAssertion; + public class AkarinAsyncExecutor { private static final ExecutorService singleExecutor = Executors.newSingleThreadExecutor(new ThreadFactoryBuilder().setNameFormat("Akarin Single Async Executor Thread - %1$d").build()); private static final ExecutorService asyncExecutor = Executors.newFixedThreadPool(getNThreads(), new ThreadFactoryBuilder().setNameFormat("Akarin Async Executor Thread - %1$d").build()); - private static int getNThreads(){ - int processors = Runtime.getRuntime().availableProcessors() / 2; - - if (processors < 2) - return 2; - if (processors > 8) - return 8; - return processors; + private static int getNThreads() { + int processors = Runtime.getRuntime().availableProcessors() / 2; + if (processors < 2) + return 2; + if (processors > 8) + return 8; + return processors; } /** @@ -26,6 +27,7 @@ public class AkarinAsyncExecutor { * @param run */ public static void scheduleSingleAsyncTask(Runnable run) { + ThreadAssertion.close(); singleExecutor.execute(run); } @@ -34,6 +36,7 @@ public class AkarinAsyncExecutor { * @param run */ public static void scheduleAsyncTask(Runnable run) { + ThreadAssertion.close(); asyncExecutor.execute(run); } @@ -43,6 +46,7 @@ public class AkarinAsyncExecutor { * @return */ public static Future scheduleSingleAsyncTask(Callable run) { + ThreadAssertion.close(); return singleExecutor.submit(run); } @@ -51,6 +55,7 @@ public class AkarinAsyncExecutor { * @param run */ public static Future scheduleAsyncTask(Callable run) { + ThreadAssertion.close(); return asyncExecutor.submit(run); } } \ No newline at end of file diff --git a/src/main/java/net/minecraft/server/LoginListener.java b/src/main/java/net/minecraft/server/LoginListener.java index dfe7a029f..2297ed5d0 100644 --- a/src/main/java/net/minecraft/server/LoginListener.java +++ b/src/main/java/net/minecraft/server/LoginListener.java @@ -4,6 +4,8 @@ import com.destroystokyo.paper.profile.CraftPlayerProfile; import com.destroystokyo.paper.profile.PlayerProfile; import com.mojang.authlib.GameProfile; import com.mojang.authlib.exceptions.AuthenticationUnavailableException; + +import co.aikar.timings.ThreadAssertion; import io.netty.channel.ChannelFuture; import java.math.BigInteger; import java.net.InetAddress; @@ -200,6 +202,7 @@ public class LoginListener implements PacketLoginInListener, ITickable { authenticatorPool.execute(new Runnable() { @Override public void run() { + ThreadAssertion.close(); // Akarin try { initUUID(); new LoginHandler().fireEvents(); @@ -228,6 +231,7 @@ public class LoginListener implements PacketLoginInListener, ITickable { // Paper start - Cache authenticator threads authenticatorPool.execute(new Runnable() { public void run() { + ThreadAssertion.close(); // Akarin GameProfile gameprofile = LoginListener.this.i; try { @@ -358,6 +362,7 @@ public class LoginListener implements PacketLoginInListener, ITickable { // Proceed with login authenticatorPool.execute(() -> { + ThreadAssertion.close(); // Akarin try { new LoginHandler().fireEvents(); } catch (Exception ex) { diff --git a/src/main/java/net/minecraft/server/MCUtil.java b/src/main/java/net/minecraft/server/MCUtil.java index f1f637ea4..b0e45f9f7 100644 --- a/src/main/java/net/minecraft/server/MCUtil.java +++ b/src/main/java/net/minecraft/server/MCUtil.java @@ -51,7 +51,7 @@ public final class MCUtil { } public static boolean isMainThread() { - return ThreadAssertion.isMainThread() && MinecraftServer.getServer().isMainThread(); // Akarin + return ThreadAssertion.is() && MinecraftServer.getServer().isMainThread(); // Akarin } private static class DelayedRunnable implements Runnable { @@ -118,7 +118,7 @@ public final class MCUtil { * @return */ public static void ensureMain(String reason, Runnable run) { - if (/*AsyncCatcher.enabled &&*/ !ThreadAssertion.isMainThread() && Thread.currentThread() != MinecraftServer.getServer().primaryThread) { // Akarin + if (/*AsyncCatcher.enabled &&*/ !ThreadAssertion.is() && Thread.currentThread() != MinecraftServer.getServer().primaryThread) { // Akarin if (reason != null) { new IllegalStateException("Asynchronous " + reason + "!").printStackTrace(); } @@ -143,7 +143,7 @@ public final class MCUtil { * @return */ public static T ensureMain(String reason, Supplier run) { - if (/*AsyncCatcher.enabled &&*/ !ThreadAssertion.isMainThread() && Thread.currentThread() != MinecraftServer.getServer().primaryThread) { // Akarin + if (/*AsyncCatcher.enabled &&*/ !ThreadAssertion.is() && Thread.currentThread() != MinecraftServer.getServer().primaryThread) { // Akarin if (reason != null) { new IllegalStateException("Asynchronous " + reason + "! Blocking thread until it returns ").printStackTrace(); } @@ -287,6 +287,7 @@ public final class MCUtil { * @param run */ public static void scheduleAsyncTask(Runnable run) { + ThreadAssertion.close(); // Akarin asyncExecutor.execute(run); } diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java index c97ef11d8..6cbc4e0f5 100644 --- a/src/main/java/net/minecraft/server/MinecraftServer.java +++ b/src/main/java/net/minecraft/server/MinecraftServer.java @@ -1728,7 +1728,7 @@ public abstract class MinecraftServer implements IAsyncTaskHandler, IMojangStati } public boolean isMainThread() { - return ThreadAssertion.isMainThread() || Thread.currentThread() == this.serverThread; // Akarin + return ThreadAssertion.is() || Thread.currentThread() == this.serverThread; // Akarin } public int aw() { diff --git a/src/main/java/net/minecraft/server/PlayerConnection.java b/src/main/java/net/minecraft/server/PlayerConnection.java index 61dc3e98f..c91402522 100644 --- a/src/main/java/net/minecraft/server/PlayerConnection.java +++ b/src/main/java/net/minecraft/server/PlayerConnection.java @@ -1742,7 +1742,7 @@ public class PlayerConnection implements PacketListenerPlayIn, ITickable { if (!async && s.startsWith("/")) { // Paper Start - if (!org.spigotmc.AsyncCatcher.shuttingDown && !ThreadAssertion.isMainThread() && !org.bukkit.Bukkit.isPrimaryThread()) { + if (!org.spigotmc.AsyncCatcher.shuttingDown && !ThreadAssertion.is() && !org.bukkit.Bukkit.isPrimaryThread()) { final String fCommandLine = s; MinecraftServer.LOGGER.log(org.apache.logging.log4j.Level.ERROR, "Command Dispatched Async: " + fCommandLine); // Akarin 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()); diff --git a/src/main/java/org/bukkit/craftbukkit/CraftServer.java b/src/main/java/org/bukkit/craftbukkit/CraftServer.java index 20dad476f..5ccf55e4a 100644 --- a/src/main/java/org/bukkit/craftbukkit/CraftServer.java +++ b/src/main/java/org/bukkit/craftbukkit/CraftServer.java @@ -710,7 +710,7 @@ public final class CraftServer implements Server { //org.spigotmc.AsyncCatcher.catchOp( "command dispatch" ); // Spigot // Akarin // Paper Start - if (!org.spigotmc.AsyncCatcher.shuttingDown && !ThreadAssertion.isMainThread() && !Bukkit.isPrimaryThread()) { // Akarin + if (!org.spigotmc.AsyncCatcher.shuttingDown && !ThreadAssertion.is() && !Bukkit.isPrimaryThread()) { // Akarin final CommandSender fSender = sender; final String fCommandLine = commandLine; Bukkit.getLogger().log(Level.SEVERE, "Command Dispatched Async: " + commandLine); @@ -1724,7 +1724,7 @@ public final class CraftServer implements Server { @Override public boolean isPrimaryThread() { - return ThreadAssertion.isMainThread() || Thread.currentThread().equals(console.primaryThread); + return ThreadAssertion.is() || Thread.currentThread().equals(console.primaryThread); } @Override diff --git a/src/main/java/org/spigotmc/AsyncCatcher.java b/src/main/java/org/spigotmc/AsyncCatcher.java index 323e1bc72..8b220cc5f 100644 --- a/src/main/java/org/spigotmc/AsyncCatcher.java +++ b/src/main/java/org/spigotmc/AsyncCatcher.java @@ -11,7 +11,7 @@ public class AsyncCatcher public static void catchOp(String reason) { - if ( enabled && !ThreadAssertion.isMainThread() && Thread.currentThread() != MinecraftServer.getServer().primaryThread ) + if ( enabled && !ThreadAssertion.is() && Thread.currentThread() != MinecraftServer.getServer().primaryThread ) { throw new IllegalStateException( "Asynchronous " + reason + "!" ); }