diff --git a/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityEventManager.java b/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityEventManager.java index e931576a0..2cfd51115 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityEventManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityEventManager.java @@ -9,7 +9,6 @@ import com.velocitypowered.api.event.EventManager; import com.velocitypowered.api.event.PostOrder; import com.velocitypowered.api.event.Subscribe; import com.velocitypowered.api.plugin.PluginManager; -import com.velocitypowered.proxy.util.concurrency.RecordingThreadFactory; import net.kyori.event.EventSubscriber; import net.kyori.event.PostResult; import net.kyori.event.SimpleEventBus; @@ -43,14 +42,12 @@ public class VelocityEventManager implements EventManager { new ASMEventExecutorFactory<>(new PluginClassLoader(new URL[0])), new VelocityMethodScanner()); private final ExecutorService service; - private final RecordingThreadFactory recordingThreadFactory; private final PluginManager pluginManager; public VelocityEventManager(PluginManager pluginManager) { this.pluginManager = pluginManager; - this.recordingThreadFactory = new RecordingThreadFactory(new ThreadFactoryBuilder() + this.service = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors(), new ThreadFactoryBuilder() .setNameFormat("Velocity Event Executor - #%d").setDaemon(true).build()); - this.service = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors(), recordingThreadFactory); } @Override @@ -88,17 +85,11 @@ public class VelocityEventManager implements EventManager { logger.error("Some errors occurred whilst posting event {}.", event); int i = 0; for (Throwable exception : result.exceptions().values()) { - logger.error("#{}: \n", i++, exception); + logger.error("#{}: \n", ++i, exception); } } }; - if (recordingThreadFactory.currentlyInFactory()) { - // Optimization: fire the event immediately, we are on the event handling thread. - runEvent.run(); - return CompletableFuture.completedFuture(event); - } - CompletableFuture eventFuture = new CompletableFuture<>(); service.execute(() -> { runEvent.run(); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/util/concurrency/RecordingThreadFactory.java b/proxy/src/main/java/com/velocitypowered/proxy/util/concurrency/RecordingThreadFactory.java deleted file mode 100644 index 1de4b165d..000000000 --- a/proxy/src/main/java/com/velocitypowered/proxy/util/concurrency/RecordingThreadFactory.java +++ /dev/null @@ -1,44 +0,0 @@ -package com.velocitypowered.proxy.util.concurrency; - -import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Preconditions; -import com.google.common.collect.MapMaker; - -import java.util.Collections; -import java.util.Set; -import java.util.concurrent.ThreadFactory; - -/** - * A {@link ThreadFactory} that records the threads it has created. Once a thread terminates, it is automatically removed - * from the recorder. - */ -public class RecordingThreadFactory implements ThreadFactory { - private final ThreadFactory backing; - private final Set threads = Collections.newSetFromMap(new MapMaker().weakKeys().makeMap()); - - public RecordingThreadFactory(ThreadFactory backing) { - this.backing = Preconditions.checkNotNull(backing, "backing"); - } - - @Override - public Thread newThread(Runnable runnable) { - Preconditions.checkNotNull(runnable, "runnable"); - return backing.newThread(() -> { - threads.add(Thread.currentThread()); - try { - runnable.run(); - } finally { - threads.remove(Thread.currentThread()); - } - }); - } - - public boolean currentlyInFactory() { - return threads.contains(Thread.currentThread()); - } - - @VisibleForTesting - int size() { - return threads.size(); - } -} diff --git a/proxy/src/test/java/com/velocitypowered/proxy/scheduler/VelocitySchedulerTest.java b/proxy/src/test/java/com/velocitypowered/proxy/scheduler/VelocitySchedulerTest.java index 7be0008c8..b1001b2c0 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/scheduler/VelocitySchedulerTest.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/scheduler/VelocitySchedulerTest.java @@ -12,7 +12,7 @@ import java.util.concurrent.atomic.AtomicInteger; import static org.junit.jupiter.api.Assertions.assertEquals; class VelocitySchedulerTest { - // TODO: The timings here will be inaccurate on slow systems. Need to find a testing-friendly replacement for Thread.sleep() + // TODO: The timings here will be inaccurate on slow systems. @Test void buildTask() throws Exception { diff --git a/proxy/src/test/java/com/velocitypowered/proxy/util/concurrency/RecordingThreadFactoryTest.java b/proxy/src/test/java/com/velocitypowered/proxy/util/concurrency/RecordingThreadFactoryTest.java deleted file mode 100644 index 33fabfcf3..000000000 --- a/proxy/src/test/java/com/velocitypowered/proxy/util/concurrency/RecordingThreadFactoryTest.java +++ /dev/null @@ -1,69 +0,0 @@ -package com.velocitypowered.proxy.util.concurrency; - -import org.junit.jupiter.api.Test; - -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.Executors; -import java.util.concurrent.ThreadFactory; - -import static org.junit.jupiter.api.Assertions.*; - -class RecordingThreadFactoryTest { - - @Test - void newThread() throws Exception { - RecordingThreadFactory factory = new RecordingThreadFactory(Executors.defaultThreadFactory()); - CountDownLatch started = new CountDownLatch(1); - CountDownLatch endThread = new CountDownLatch(1); - factory.newThread(() -> { - started.countDown(); - assertTrue(factory.currentlyInFactory()); - assertEquals(1, factory.size()); - try { - endThread.await(); - } catch (InterruptedException e) { - fail(e); - } - }).start(); - started.await(); - assertFalse(factory.currentlyInFactory()); - assertEquals(1, factory.size()); - endThread.countDown(); - - // Wait a little bit to ensure the thread got shut down - Thread.sleep(10); - assertEquals(0, factory.size()); - } - - @Test - void cleanUpAfterExceptionThrown() throws Exception { - CountDownLatch started = new CountDownLatch(1); - CountDownLatch endThread = new CountDownLatch(1); - CountDownLatch hasEnded = new CountDownLatch(1); - RecordingThreadFactory factory = new RecordingThreadFactory((ThreadFactory) r -> { - Thread t = new Thread(r); - t.setUncaughtExceptionHandler((t1, e) -> hasEnded.countDown()); - return t; - }); - factory.newThread(() -> { - started.countDown(); - assertTrue(factory.currentlyInFactory()); - assertEquals(1, factory.size()); - try { - endThread.await(); - } catch (InterruptedException e) { - fail(e); - } - throw new RuntimeException(""); - }).start(); - started.await(); - assertFalse(factory.currentlyInFactory()); - assertEquals(1, factory.size()); - endThread.countDown(); - hasEnded.await(); - - // Wait a little bit to ensure the thread got shut down - Thread.sleep(10); - assertEquals(0, factory.size()); - } -} \ No newline at end of file