From 57cf6084e28442357804b74e68726054fe981d4e Mon Sep 17 00:00:00 2001 From: Jonathan Coates Date: Sat, 22 Oct 2022 14:36:25 +0100 Subject: [PATCH] Manage ComputerThread's lifecycle in ComputerContext This converts ComputerThread from a singleton into a proper object, which is setup when starting a computer, and tore down when the ComputerContext is closed. While this is mostly for conceptual elegance, it does offer some concrete benefits: - You can now adjust the thread count without restarting the whole game (just leaving and rentering the world). Though, alas, no effect on servers. - We can run multiple ComputerThreads in parallel, which makes it much easier to run tests in parallel. This allows us to remove our rather silly IsolatedRunner test helper. --- .../computercraft/core/ComputerContext.java | 25 ++- .../core/computer/ComputerExecutor.java | 13 +- .../core/computer/ComputerThread.java | 191 ++++++++++-------- .../core/computer/TimeoutState.java | 11 +- .../shared/computer/core/ServerContext.java | 2 +- .../core/ComputerTestDelegate.java | 2 +- .../core/computer/ComputerBootstrap.java | 2 +- .../core/computer/ComputerThreadTest.java | 54 +++-- .../core/computer/FakeComputerManager.java | 61 +++--- .../computercraft/support/IsolatedRunner.java | 110 ---------- 10 files changed, 216 insertions(+), 255 deletions(-) delete mode 100644 src/test/java/dan200/computercraft/support/IsolatedRunner.java diff --git a/src/main/java/dan200/computercraft/core/ComputerContext.java b/src/main/java/dan200/computercraft/core/ComputerContext.java index 134673c22..1c7cf4622 100644 --- a/src/main/java/dan200/computercraft/core/ComputerContext.java +++ b/src/main/java/dan200/computercraft/core/ComputerContext.java @@ -5,6 +5,7 @@ */ package dan200.computercraft.core; +import dan200.computercraft.core.computer.ComputerThread; import dan200.computercraft.core.computer.GlobalEnvironment; import dan200.computercraft.core.computer.mainthread.MainThreadScheduler; import dan200.computercraft.core.lua.CobaltLuaMachine; @@ -16,12 +17,17 @@ import dan200.computercraft.core.lua.ILuaMachine; public final class ComputerContext implements AutoCloseable { private final GlobalEnvironment globalEnvironment; + private final ComputerThread computerScheduler; private final MainThreadScheduler mainThreadScheduler; private final ILuaMachine.Factory factory; - public ComputerContext( GlobalEnvironment globalEnvironment, MainThreadScheduler mainThreadScheduler, ILuaMachine.Factory factory ) + public ComputerContext( + GlobalEnvironment globalEnvironment, ComputerThread computerScheduler, + MainThreadScheduler mainThreadScheduler, ILuaMachine.Factory factory + ) { this.globalEnvironment = globalEnvironment; + this.computerScheduler = computerScheduler; this.mainThreadScheduler = mainThreadScheduler; this.factory = factory; } @@ -30,11 +36,12 @@ public final class ComputerContext implements AutoCloseable * Create a default {@link ComputerContext} with the given global environment. * * @param environment The current global environment. + * @param threads The number of threads to use for the {@link #computerScheduler()} * @param mainThreadScheduler The main thread scheduler to use. */ - public ComputerContext( GlobalEnvironment environment, MainThreadScheduler mainThreadScheduler ) + public ComputerContext( GlobalEnvironment environment, int threads, MainThreadScheduler mainThreadScheduler ) { - this( environment, mainThreadScheduler, CobaltLuaMachine::new ); + this( environment, new ComputerThread( threads ), mainThreadScheduler, CobaltLuaMachine::new ); } /** @@ -47,6 +54,17 @@ public final class ComputerContext implements AutoCloseable return globalEnvironment; } + /** + * The {@link ComputerThread} instance under which computers are run. This is closed when the context is closed, and + * so should be unique per-context. + * + * @return The current computer thread manager. + */ + public ComputerThread computerScheduler() + { + return computerScheduler; + } + /** * The {@link MainThreadScheduler} instance used to run main-thread tasks. * @@ -73,5 +91,6 @@ public final class ComputerContext implements AutoCloseable @Override public void close() { + computerScheduler().stop(); } } diff --git a/src/main/java/dan200/computercraft/core/computer/ComputerExecutor.java b/src/main/java/dan200/computercraft/core/computer/ComputerExecutor.java index aeb2299c5..e1645633a 100644 --- a/src/main/java/dan200/computercraft/core/computer/ComputerExecutor.java +++ b/src/main/java/dan200/computercraft/core/computer/ComputerExecutor.java @@ -61,7 +61,8 @@ final class ComputerExecutor private final ComputerEnvironment computerEnvironment; private final MetricsObserver metrics; private final List apis = new ArrayList<>(); - final TimeoutState timeout = new TimeoutState(); + private final ComputerThread scheduler; + final TimeoutState timeout; private FileSystem fileSystem; @@ -164,13 +165,15 @@ final class ComputerExecutor ComputerExecutor( Computer computer, ComputerEnvironment computerEnvironment, ComputerContext context ) { - // Ensure the computer thread is running as required. - ComputerThread.start(); - this.computer = computer; this.computerEnvironment = computerEnvironment; metrics = computerEnvironment.getMetrics(); luaFactory = context.luaFactory(); + scheduler = context.computerScheduler(); + timeout = new TimeoutState( scheduler ); + + // Ensure the computer thread is running as required. + scheduler.start(); Environment environment = computer.getEnvironment(); @@ -316,7 +319,7 @@ final class ComputerExecutor { synchronized( queueLock ) { - if( !onComputerQueue ) ComputerThread.queue( this ); + if( !onComputerQueue ) scheduler.queue( this ); } } diff --git a/src/main/java/dan200/computercraft/core/computer/ComputerThread.java b/src/main/java/dan200/computercraft/core/computer/ComputerThread.java index d4dedcfb2..d3b98e464 100644 --- a/src/main/java/dan200/computercraft/core/computer/ComputerThread.java +++ b/src/main/java/dan200/computercraft/core/computer/ComputerThread.java @@ -8,8 +8,8 @@ package dan200.computercraft.core.computer; import dan200.computercraft.ComputerCraft; import dan200.computercraft.shared.util.ThreadUtils; -import javax.annotation.Nonnull; import javax.annotation.Nullable; +import java.util.Objects; import java.util.TreeSet; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; @@ -19,28 +19,25 @@ import java.util.concurrent.locks.Condition; import java.util.concurrent.locks.LockSupport; import java.util.concurrent.locks.ReentrantLock; -import static dan200.computercraft.core.computer.TimeoutState.ABORT_TIMEOUT; -import static dan200.computercraft.core.computer.TimeoutState.TIMEOUT; - /** * Responsible for running all tasks from a {@link Computer}. - * + *

* This is split into two components: the {@link TaskRunner}s, which pull an executor from the queue and execute it, and * a single {@link Monitor} which observes all runners and kills them if they have not been terminated by * {@link TimeoutState#isSoftAborted()}. - * + *

* Computers are executed using a priority system, with those who have spent less time executing having a higher * priority than those hogging the thread. This, combined with {@link TimeoutState#isPaused()} means we can reduce the * risk of badly behaved computers stalling execution for everyone else. - * + *

* This is done using an implementation of Linux's Completely Fair Scheduler. When a computer executes, we compute what * share of execution time it has used (time executed/number of tasks). We then pick the computer who has the least * "virtual execution time" (aka {@link ComputerExecutor#virtualRuntime}). - * + *

* When adding a computer to the queue, we make sure its "virtual runtime" is at least as big as the smallest runtime. * This means that adding computers which have slept a lot do not then have massive priority over everyone else. See * {@link #queue(ComputerExecutor)} for how this is implemented. - * + *

* In reality, it's unlikely that more than a few computers are waiting to execute at once, so this will not have much * effect unless you have a computer hogging execution time. However, it is pretty effective in those situations. * @@ -49,6 +46,9 @@ import static dan200.computercraft.core.computer.TimeoutState.TIMEOUT; */ public final class ComputerThread { + private static final ThreadFactory monitorFactory = ThreadUtils.factory( "Computer-Monitor" ); + private static final ThreadFactory runnerFactory = ThreadUtils.factory( "Computer-Runner" ); + /** * How often the computer thread monitor should run. * @@ -58,7 +58,7 @@ public final class ComputerThread /** * The target latency between executing two tasks on a single machine. - * + *

* An average tick takes 50ms, and so we ideally need to have handled a couple of events within that window in order * to have a perceived low latency. */ @@ -66,7 +66,7 @@ public final class ComputerThread /** * The minimum value that {@link #DEFAULT_LATENCY} can have when scaled. - * + *

* From statistics gathered on SwitchCraft, almost all machines will execute under 15ms, 75% under 1.5ms, with the * mean being about 3ms. Most computers shouldn't be too impacted with having such a short period to execute in. */ @@ -87,36 +87,36 @@ public final class ComputerThread /** * Lock used for modifications to the array of current threads. */ - private static final Object threadLock = new Object(); + private final Object threadLock = new Object(); /** * Whether the computer thread system is currently running. */ - private static volatile boolean running = false; + private volatile boolean running = false; /** * The current task manager. */ - private static Thread monitor; + private @Nullable Thread monitor; /** * The array of current runners, and their owning threads. */ - private static TaskRunner[] runners; + private final TaskRunner[] runners; - private static long latency; - private static long minPeriod; + private final long latency; + private final long minPeriod; - private static final ReentrantLock computerLock = new ReentrantLock(); + private final ReentrantLock computerLock = new ReentrantLock(); - private static final Condition hasWork = computerLock.newCondition(); - private static final AtomicInteger idleWorkers = new AtomicInteger( 0 ); - private static final Condition monitorWakeup = computerLock.newCondition(); + private final Condition hasWork = computerLock.newCondition(); + private final AtomicInteger idleWorkers = new AtomicInteger( 0 ); + private final Condition monitorWakeup = computerLock.newCondition(); /** * Active queues to execute. */ - private static final TreeSet computerQueue = new TreeSet<>( ( a, b ) -> { + private final TreeSet computerQueue = new TreeSet<>( ( a, b ) -> { if( a == b ) return 0; // Should never happen, but let's be consistent here long at = a.virtualRuntime, bt = b.virtualRuntime; @@ -127,34 +127,27 @@ public final class ComputerThread /** * The minimum {@link ComputerExecutor#virtualRuntime} time on the tree. */ - private static long minimumVirtualRuntime = 0; + private long minimumVirtualRuntime = 0; - private static final ThreadFactory monitorFactory = ThreadUtils.factory( "Computer-Monitor" ); - private static final ThreadFactory runnerFactory = ThreadUtils.factory( "Computer-Runner" ); + public ComputerThread( int threadCount ) + { + runners = new TaskRunner[threadCount]; - private ComputerThread() {} + // latency and minPeriod are scaled by 1 + floor(log2(threads)). We can afford to execute tasks for + // longer when executing on more than one thread. + int factor = 64 - Long.numberOfLeadingZeros( runners.length ); + latency = DEFAULT_LATENCY * factor; + minPeriod = DEFAULT_MIN_PERIOD * factor; + } /** * Start the computer thread. */ - static void start() + void start() { synchronized( threadLock ) { running = true; - - if( runners == null ) - { - // TODO: Update this on config reloads. Or possibly on world restarts? - runners = new TaskRunner[ComputerCraft.computerThreads]; - - // latency and minPeriod are scaled by 1 + floor(log2(threads)). We can afford to execute tasks for - // longer when executing on more than one thread. - long factor = 64 - Long.numberOfLeadingZeros( runners.length ); - latency = DEFAULT_LATENCY * factor; - minPeriod = DEFAULT_MIN_PERIOD * factor; - } - for( int i = 0; i < runners.length; i++ ) { TaskRunner runner = runners[i]; @@ -174,21 +167,20 @@ public final class ComputerThread /** * Attempt to stop the computer thread. This interrupts each runner, and clears the task queue. */ - public static void stop() + public void stop() { synchronized( threadLock ) { running = false; - if( runners != null ) + for( TaskRunner runner : runners ) { - for( TaskRunner runner : runners ) - { - if( runner == null ) continue; + if( runner == null ) continue; - runner.running = false; - if( runner.owner != null ) runner.owner.interrupt(); - } + runner.running = false; + if( runner.owner != null ) runner.owner.interrupt(); } + + if( monitor != null ) monitor.interrupt(); } computerLock.lock(); @@ -200,17 +192,40 @@ public final class ComputerThread { computerLock.unlock(); } + + synchronized( threadLock ) + { + if( monitor != null ) tryJoin( monitor ); + for( TaskRunner runner : runners ) + { + if( runner != null && runner.owner != null ) tryJoin( runner.owner ); + } + } + } + + private static void tryJoin( Thread thread ) + { + try + { + thread.join( 100 ); + } + catch( InterruptedException e ) + { + throw new IllegalStateException( "Interrupted server thread while trying to stop " + thread.getName(), e ); + } + + if( thread.isAlive() ) ComputerCraft.log.error( "Failed to stop {}", thread.getName() ); } /** * Mark a computer as having work, enqueuing it on the thread. - * + *

* You must be holding {@link ComputerExecutor}'s {@code queueLock} when calling this method - it should only * be called from {@code enqueue}. * * @param executor The computer to execute work on. */ - static void queue( @Nonnull ComputerExecutor executor ) + void queue( ComputerExecutor executor ) { computerLock.lock(); try @@ -256,12 +271,12 @@ public final class ComputerThread /** * Update the {@link ComputerExecutor#virtualRuntime}s of all running tasks, and then update the * {@link #minimumVirtualRuntime} based on the current tasks. - * + *

* This is called before queueing tasks, to ensure that {@link #minimumVirtualRuntime} is up-to-date. * * @param current The machine which we updating runtimes from. */ - private static void updateRuntimes( @Nullable ComputerExecutor current ) + private void updateRuntimes( @Nullable ComputerExecutor current ) { long minRuntime = Long.MAX_VALUE; @@ -271,20 +286,16 @@ public final class ComputerThread // Update all the currently executing tasks long now = System.nanoTime(); int tasks = 1 + computerQueue.size(); - TaskRunner[] currentRunners = runners; - if( currentRunners != null ) + for( TaskRunner runner : runners ) { - for( TaskRunner runner : currentRunners ) - { - if( runner == null ) continue; - ComputerExecutor executor = runner.currentExecutor.get(); - if( executor == null ) continue; + if( runner == null ) continue; + ComputerExecutor executor = runner.currentExecutor.get(); + if( executor == null ) continue; - // We do two things here: first we update the task's virtual runtime based on when we - // last checked, and then we check the minimum. - minRuntime = Math.min( minRuntime, executor.virtualRuntime += (now - executor.vRuntimeStart) / tasks ); - executor.vRuntimeStart = now; - } + // We do two things here: first we update the task's virtual runtime based on when we + // last checked, and then we check the minimum. + minRuntime = Math.min( minRuntime, executor.virtualRuntime += (now - executor.vRuntimeStart) / tasks ); + executor.vRuntimeStart = now; } // And update the most recently executed one (if set). @@ -306,15 +317,18 @@ public final class ComputerThread * @param runner The runner this task was on. * @param executor The executor to requeue */ - private static void afterWork( TaskRunner runner, ComputerExecutor executor ) + private void afterWork( TaskRunner runner, ComputerExecutor executor ) { // Clear the executor's thread. Thread currentThread = executor.executingThread.getAndSet( null ); if( currentThread != runner.owner ) { + ComputerCraft.log.error( "Expected computer #{} to be running on {}, but already running on {}. This is a SERIOUS bug, please report with your debug.log.", - executor.getComputer().getID(), runner.owner.getName(), currentThread == null ? "nothing" : currentThread.getName() + executor.getComputer().getID(), + runner.owner == null ? "nothing" : runner.owner.getName(), + currentThread == null ? "nothing" : currentThread.getName() ); } @@ -344,7 +358,7 @@ public final class ComputerThread * @see #DEFAULT_MIN_PERIOD * @see #LATENCY_MAX_TASKS */ - static long scaledPeriod() + long scaledPeriod() { // +1 to include the current task int count = 1 + computerQueue.size(); @@ -356,7 +370,7 @@ public final class ComputerThread * * @return If we have work queued up. */ - static boolean hasPendingWork() + boolean hasPendingWork() { return !computerQueue.isEmpty(); } @@ -367,7 +381,7 @@ public final class ComputerThread * * @return If the computer threads are busy. */ - private static boolean isBusy() + private boolean isBusy() { return computerQueue.size() > idleWorkers.get(); } @@ -378,7 +392,7 @@ public final class ComputerThread * * @see TimeoutState */ - private static final class Monitor implements Runnable + private final class Monitor implements Runnable { @Override public void run() @@ -395,7 +409,10 @@ public final class ComputerThread } catch( InterruptedException e ) { - ComputerCraft.log.error( "Monitor thread interrupted. Computers may behave very badly!", e ); + if( running ) + { + ComputerCraft.log.error( "Monitor thread interrupted. Computers may behave very badly!", e ); + } break; } finally @@ -407,11 +424,9 @@ public final class ComputerThread } } - private static void checkRunners() + private void checkRunners() { - TaskRunner[] currentRunners = ComputerThread.runners; - if( currentRunners == null ) return; - + TaskRunner[] currentRunners = ComputerThread.this.runners; for( int i = 0; i < currentRunners.length; i++ ) { TaskRunner runner = currentRunners[i]; @@ -421,10 +436,9 @@ public final class ComputerThread if( !running ) continue; // Mark the old runner as dead and start a new one. - ComputerCraft.log.warn( "Previous runner ({}) has crashed, restarting!", - runner != null && runner.owner != null ? runner.owner.getName() : runner ); + ComputerCraft.log.warn( "Previous runner ({}) has crashed, restarting!", runner != null && runner.owner != null ? runner.owner.getName() : runner ); if( runner != null ) runner.running = false; - runnerFactory.newThread( runners[i] = new TaskRunner() ).start(); + runnerFactory.newThread( runner = runners[i] = new TaskRunner() ).start(); } // If the runner has no work, skip @@ -437,38 +451,38 @@ public final class ComputerThread // If we're still within normal execution times (TIMEOUT) or soft abort (ABORT_TIMEOUT), // then we can let the Lua machine do its work. long afterStart = executor.timeout.nanoCumulative(); - long afterHardAbort = afterStart - TIMEOUT - ABORT_TIMEOUT; + long afterHardAbort = afterStart - TimeoutState.TIMEOUT - TimeoutState.ABORT_TIMEOUT; if( afterHardAbort < 0 ) continue; // Set the hard abort flag. executor.timeout.hardAbort(); executor.abort(); - if( afterHardAbort >= ABORT_TIMEOUT * 2 ) + if( afterHardAbort >= TimeoutState.ABORT_TIMEOUT * 2 ) { // If we've hard aborted and interrupted, and we're still not dead, then mark the runner // as dead, finish off the task, and spawn a new runner. runner.reportTimeout( executor, afterStart ); runner.running = false; - runner.owner.interrupt(); + if( runner.owner != null ) runner.owner.interrupt(); ComputerExecutor thisExecutor = runner.currentExecutor.getAndSet( null ); if( thisExecutor != null ) afterWork( runner, executor ); synchronized( threadLock ) { - if( running && runners.length > i && runners[i] == runner ) + if( running && runners[i] == runner ) { runnerFactory.newThread( currentRunners[i] = new TaskRunner() ).start(); } } } - else if( afterHardAbort >= ABORT_TIMEOUT ) + else if( afterHardAbort >= TimeoutState.ABORT_TIMEOUT ) { // If we've hard aborted but we're still not dead, dump the stack trace and interrupt // the task. runner.reportTimeout( executor, afterStart ); - runner.owner.interrupt(); + if( runner.owner != null ) runner.owner.interrupt(); } } } @@ -476,13 +490,14 @@ public final class ComputerThread /** * Pulls tasks from the {@link #computerQueue} queue and runs them. - * + *

* This is responsible for running the {@link ComputerExecutor#work()}, {@link ComputerExecutor#beforeWork()} and * {@link ComputerExecutor#afterWork()} functions. Everything else is either handled by the executor, timeout * state or monitor. */ - private static final class TaskRunner implements Runnable + private final class TaskRunner implements Runnable { + @Nullable Thread owner; long lastReport = Long.MIN_VALUE; volatile boolean running = true; @@ -495,7 +510,7 @@ public final class ComputerThread owner = Thread.currentThread(); tasks: - while( running && ComputerThread.running ) + while( running && ComputerThread.this.running ) { // Wait for an active queue to execute ComputerExecutor executor; @@ -572,6 +587,8 @@ public final class ComputerThread if( lastReport != Long.MIN_VALUE && now - lastReport - REPORT_DEBOUNCE <= 0 ) return; lastReport = now; + Thread owner = Objects.requireNonNull( this.owner ); + StringBuilder builder = new StringBuilder() .append( "Terminating computer #" ).append( executor.getComputer().getID() ) .append( " due to timeout (running for " ).append( time * 1e-9 ) diff --git a/src/main/java/dan200/computercraft/core/computer/TimeoutState.java b/src/main/java/dan200/computercraft/core/computer/TimeoutState.java index 80be47199..021c706db 100644 --- a/src/main/java/dan200/computercraft/core/computer/TimeoutState.java +++ b/src/main/java/dan200/computercraft/core/computer/TimeoutState.java @@ -49,6 +49,8 @@ public final class TimeoutState */ public static final String ABORT_MESSAGE = "Too long without yielding"; + private final ComputerThread scheduler; + private boolean paused; private boolean softAbort; private volatile boolean hardAbort; @@ -73,6 +75,11 @@ public final class TimeoutState */ private long currentDeadline; + public TimeoutState( ComputerThread scheduler ) + { + this.scheduler = scheduler; + } + long nanoCumulative() { return System.nanoTime() - cumulativeStart; @@ -91,7 +98,7 @@ public final class TimeoutState // Important: The weird arithmetic here is important, as nanoTime may return negative values, and so we // need to handle overflow. long now = System.nanoTime(); - if( !paused ) paused = currentDeadline - now <= 0 && ComputerThread.hasPendingWork(); // now >= currentDeadline + if( !paused ) paused = currentDeadline - now <= 0 && scheduler.hasPendingWork(); // now >= currentDeadline if( !softAbort ) softAbort = now - cumulativeStart - TIMEOUT >= 0; // now - cumulativeStart >= TIMEOUT } @@ -143,7 +150,7 @@ public final class TimeoutState { long now = System.nanoTime(); currentStart = now; - currentDeadline = now + ComputerThread.scaledPeriod(); + currentDeadline = now + scheduler.scaledPeriod(); // Compute the "nominal start time". cumulativeStart = now - cumulativeElapsed; } diff --git a/src/main/java/dan200/computercraft/shared/computer/core/ServerContext.java b/src/main/java/dan200/computercraft/shared/computer/core/ServerContext.java index 4c27d8760..51cc00296 100644 --- a/src/main/java/dan200/computercraft/shared/computer/core/ServerContext.java +++ b/src/main/java/dan200/computercraft/shared/computer/core/ServerContext.java @@ -56,7 +56,7 @@ public final class ServerContext this.server = server; storageDir = server.getWorldPath( FOLDER ); mainThread = new MainThread(); - context = new ComputerContext( new Environment( server ), mainThread ); + context = new ComputerContext( new Environment( server ), ComputerCraft.computerThreads, mainThread ); idAssigner = new IDAssigner( storageDir.resolve( "ids.json" ) ); } diff --git a/src/test/java/dan200/computercraft/core/ComputerTestDelegate.java b/src/test/java/dan200/computercraft/core/ComputerTestDelegate.java index b6533c21f..dedfcf800 100644 --- a/src/test/java/dan200/computercraft/core/ComputerTestDelegate.java +++ b/src/test/java/dan200/computercraft/core/ComputerTestDelegate.java @@ -114,7 +114,7 @@ public class ComputerTestDelegate } BasicEnvironment environment = new BasicEnvironment( mount ); - context = new ComputerContext( environment, new FakeMainThreadScheduler() ); + context = new ComputerContext( environment, 1, new FakeMainThreadScheduler() ); computer = new Computer( context, environment, term, 0 ); computer.getEnvironment().setPeripheral( ComputerSide.TOP, new FakeModem() ); computer.addApi( new CctTestAPI() ); diff --git a/src/test/java/dan200/computercraft/core/computer/ComputerBootstrap.java b/src/test/java/dan200/computercraft/core/computer/ComputerBootstrap.java index a712eb667..4ff45f006 100644 --- a/src/test/java/dan200/computercraft/core/computer/ComputerBootstrap.java +++ b/src/test/java/dan200/computercraft/core/computer/ComputerBootstrap.java @@ -50,7 +50,7 @@ public class ComputerBootstrap Terminal term = new Terminal( ComputerCraft.computerTermWidth, ComputerCraft.computerTermHeight, true ); MainThread mainThread = new MainThread(); BasicEnvironment environment = new BasicEnvironment( mount ); - ComputerContext context = new ComputerContext( environment, mainThread ); + ComputerContext context = new ComputerContext( environment, 1, mainThread ); final Computer computer = new Computer( context, environment, term, 0 ); AssertApi api = new AssertApi(); diff --git a/src/test/java/dan200/computercraft/core/computer/ComputerThreadTest.java b/src/test/java/dan200/computercraft/core/computer/ComputerThreadTest.java index ebe854a76..87c6be49b 100644 --- a/src/test/java/dan200/computercraft/core/computer/ComputerThreadTest.java +++ b/src/test/java/dan200/computercraft/core/computer/ComputerThreadTest.java @@ -5,15 +5,16 @@ */ package dan200.computercraft.core.computer; -import dan200.computercraft.ComputerCraft; import dan200.computercraft.core.lua.MachineResult; import dan200.computercraft.support.ConcurrentHelpers; -import dan200.computercraft.support.IsolatedRunner; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; -import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.parallel.Execution; import org.junit.jupiter.api.parallel.ExecutionMode; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.concurrent.TimeUnit; @@ -22,33 +23,48 @@ import static org.hamcrest.Matchers.closeTo; import static org.junit.jupiter.api.Assertions.*; @Timeout( value = 15 ) -@ExtendWith( IsolatedRunner.class ) @Execution( ExecutionMode.CONCURRENT ) public class ComputerThreadTest { + private static final Logger LOGGER = LoggerFactory.getLogger( ComputerThreadTest.class ); + + private FakeComputerManager manager; + + @BeforeEach + public void before() + { + manager = new FakeComputerManager(); + } + + @AfterEach + public void after() + { + manager.close(); + } + @Test public void testSoftAbort() throws Exception { - Computer computer = FakeComputerManager.create(); - FakeComputerManager.enqueue( computer, timeout -> { + Computer computer = manager.create(); + manager.enqueue( computer, timeout -> { assertFalse( timeout.isSoftAborted(), "Should not start soft-aborted" ); long delay = ConcurrentHelpers.waitUntil( timeout::isSoftAborted ); assertThat( "Should be soft aborted", delay * 1e-9, closeTo( 7, 0.5 ) ); - ComputerCraft.log.info( "Slept for {}", delay ); + LOGGER.info( "Slept for {}", delay ); computer.shutdown(); return MachineResult.OK; } ); - FakeComputerManager.startAndWait( computer ); + manager.startAndWait( computer ); } @Test public void testHardAbort() throws Exception { - Computer computer = FakeComputerManager.create(); - FakeComputerManager.enqueue( computer, timeout -> { + Computer computer = manager.create(); + manager.enqueue( computer, timeout -> { assertFalse( timeout.isHardAborted(), "Should not start soft-aborted" ); assertThrows( InterruptedException.class, () -> Thread.sleep( 11_000 ), "Sleep should be hard aborted" ); @@ -58,14 +74,14 @@ public class ComputerThreadTest return MachineResult.OK; } ); - FakeComputerManager.startAndWait( computer ); + manager.startAndWait( computer ); } @Test public void testNoPauseIfNoOtherMachines() throws Exception { - Computer computer = FakeComputerManager.create(); - FakeComputerManager.enqueue( computer, timeout -> { + Computer computer = manager.create(); + manager.enqueue( computer, timeout -> { boolean didPause = ConcurrentHelpers.waitUntil( timeout::isPaused, 5, TimeUnit.SECONDS ); assertFalse( didPause, "Machine shouldn't have paused within 5s" ); @@ -73,15 +89,15 @@ public class ComputerThreadTest return MachineResult.OK; } ); - FakeComputerManager.startAndWait( computer ); + manager.startAndWait( computer ); } @Test public void testPauseIfSomeOtherMachine() throws Exception { - Computer computer = FakeComputerManager.create(); - FakeComputerManager.enqueue( computer, timeout -> { - long budget = ComputerThread.scaledPeriod(); + Computer computer = manager.create(); + manager.enqueue( computer, timeout -> { + long budget = manager.context().computerScheduler().scaledPeriod(); assertEquals( budget, TimeUnit.MILLISECONDS.toNanos( 25 ), "Budget should be 25ms" ); long delay = ConcurrentHelpers.waitUntil( timeout::isPaused ); @@ -91,8 +107,8 @@ public class ComputerThreadTest return MachineResult.OK; } ); - FakeComputerManager.createLoopingComputer(); + manager.createLoopingComputer(); - FakeComputerManager.startAndWait( computer ); + manager.startAndWait( computer ); } } diff --git a/src/test/java/dan200/computercraft/core/computer/FakeComputerManager.java b/src/test/java/dan200/computercraft/core/computer/FakeComputerManager.java index adf0ad245..1b3cec0d5 100644 --- a/src/test/java/dan200/computercraft/core/computer/FakeComputerManager.java +++ b/src/test/java/dan200/computercraft/core/computer/FakeComputerManager.java @@ -10,10 +10,9 @@ import dan200.computercraft.core.ComputerContext; import dan200.computercraft.core.lua.ILuaMachine; import dan200.computercraft.core.lua.MachineResult; import dan200.computercraft.core.terminal.Terminal; -import dan200.computercraft.support.IsolatedRunner; -import org.jetbrains.annotations.Nullable; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import java.io.InputStream; import java.util.HashMap; import java.util.Map; @@ -26,25 +25,36 @@ import java.util.concurrent.locks.ReentrantLock; /** * Creates "fake" computers, which just run user-defined tasks rather than Lua code. - *

- * Note, this will clobber some parts of the global state. It's recommended you use this inside an {@link IsolatedRunner}. */ -public class FakeComputerManager +public class FakeComputerManager implements AutoCloseable { interface Task { MachineResult run( TimeoutState state ) throws Exception; } - private static final ComputerContext context = new ComputerContext( - new BasicEnvironment(), new FakeMainThreadScheduler(), + private final Map> machines = new HashMap<>(); + private final ComputerContext context = new ComputerContext( + new BasicEnvironment(), + new ComputerThread( 1 ), + new FakeMainThreadScheduler(), args -> new DummyLuaMachine( args.timeout ) ); - private static final Map> machines = new HashMap<>(); - private static final Lock errorLock = new ReentrantLock(); - private static final Condition hasError = errorLock.newCondition(); - private static volatile Throwable error; + private final Lock errorLock = new ReentrantLock(); + private final Condition hasError = errorLock.newCondition(); + private volatile @Nullable Throwable error; + + @Override + public void close() + { + context.close(); + } + + public ComputerContext context() + { + return context; + } /** * Create a new computer which pulls from our task queue. @@ -52,20 +62,19 @@ public class FakeComputerManager * @return The computer. This will not be started yet, you must call {@link Computer#turnOn()} and * {@link Computer#tick()} to do so. */ - @Nonnull - public static Computer create() + public Computer create() { + Queue queue = new ConcurrentLinkedQueue<>(); Computer computer = new Computer( context, new BasicEnvironment(), new Terminal( 51, 19, true ), 0 ); - ConcurrentLinkedQueue tasks = new ConcurrentLinkedQueue<>(); - computer.addApi( new QueuePassingAPI( tasks ) ); - machines.put( computer, tasks ); + computer.addApi( new QueuePassingAPI( queue ) ); // Inject an extra API to pass the queue to the machine. + machines.put( computer, queue ); return computer; } /** * Create and start a new computer which loops forever. */ - public static void createLoopingComputer() + public void createLoopingComputer() { Computer computer = create(); enqueueForever( computer, t -> { @@ -82,7 +91,7 @@ public class FakeComputerManager * @param computer The computer to enqueue the work on. * @param task The task to run. */ - public static void enqueue( @Nonnull Computer computer, @Nonnull Task task ) + public void enqueue( Computer computer, Task task ) { machines.get( computer ).offer( task ); } @@ -94,7 +103,7 @@ public class FakeComputerManager * @param computer The computer to enqueue the work on. * @param task The task to run. */ - private static void enqueueForever( @Nonnull Computer computer, @Nonnull Task task ) + private void enqueueForever( Computer computer, Task task ) { machines.get( computer ).offer( t -> { MachineResult result = task.run( t ); @@ -112,7 +121,7 @@ public class FakeComputerManager * @param unit The time unit the duration is measured in. * @throws Exception An exception thrown by a running computer. */ - public static void sleep( long delay, TimeUnit unit ) throws Exception + public void sleep( long delay, TimeUnit unit ) throws Exception { errorLock.lock(); try @@ -132,7 +141,7 @@ public class FakeComputerManager * @param computer The computer to wait for. * @throws Exception An exception thrown by a running computer. */ - public static void startAndWait( Computer computer ) throws Exception + public void startAndWait( Computer computer ) throws Exception { computer.turnOn(); computer.tick(); @@ -140,16 +149,16 @@ public class FakeComputerManager do { sleep( 100, TimeUnit.MILLISECONDS ); - } while( ComputerThread.hasPendingWork() || computer.isOn() ); + } while( context.computerScheduler().hasPendingWork() || computer.isOn() ); rethrowIfNeeded(); } - private static void rethrowIfNeeded() throws Exception + private void rethrowIfNeeded() throws Exception { + Throwable error = this.error; if( error == null ) return; if( error instanceof Exception ) throw (Exception) error; - if( error instanceof Error ) throw (Error) error; rethrow( error ); } @@ -175,10 +184,10 @@ public class FakeComputerManager } } - private static class DummyLuaMachine implements ILuaMachine + private final class DummyLuaMachine implements ILuaMachine { private final TimeoutState state; - private @javax.annotation.Nullable Queue tasks; + private @Nullable Queue tasks; DummyLuaMachine( TimeoutState state ) { diff --git a/src/test/java/dan200/computercraft/support/IsolatedRunner.java b/src/test/java/dan200/computercraft/support/IsolatedRunner.java deleted file mode 100644 index 5d95fcd00..000000000 --- a/src/test/java/dan200/computercraft/support/IsolatedRunner.java +++ /dev/null @@ -1,110 +0,0 @@ -/* - * This file is part of ComputerCraft - http://www.computercraft.info - * Copyright Daniel Ratcliffe, 2011-2022. Do not distribute without permission. - * Send enquiries to dratcliffe@gmail.com - */ -package dan200.computercraft.support; - -import com.google.common.io.ByteStreams; -import net.minecraftforge.fml.unsafe.UnsafeHacks; -import org.junit.jupiter.api.extension.*; - -import java.io.IOException; -import java.io.InputStream; -import java.lang.reflect.Field; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; -import java.security.CodeSource; -import java.security.SecureClassLoader; - -/** - * Runs a test method in an entirely isolated {@link ClassLoader}, so you can mess around with as much of - * {@link dan200.computercraft} as you like. - * - * This IS NOT a good idea, but helps us run some tests in parallel while having lots of (terrible) - * global state. - */ -public class IsolatedRunner implements InvocationInterceptor, BeforeEachCallback, AfterEachCallback -{ - private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create( new Object() ); - - @Override - public void beforeEach( ExtensionContext context ) throws Exception - { - ClassLoader loader = context.getStore( NAMESPACE ).getOrComputeIfAbsent( IsolatedClassLoader.class ); - - // Rename the global thread group to something more obvious. - ThreadGroup group = (ThreadGroup) loader.loadClass( "dan200.computercraft.shared.util.ThreadUtils" ).getMethod( "group" ).invoke( null ); - Field field = ThreadGroup.class.getDeclaredField( "name" ); - UnsafeHacks.setField( field, group, "<" + context.getDisplayName() + ">" ); - } - - @Override - public void afterEach( ExtensionContext context ) throws Exception - { - ClassLoader loader = context.getStore( NAMESPACE ).get( IsolatedClassLoader.class, IsolatedClassLoader.class ); - loader.loadClass( "dan200.computercraft.core.computer.ComputerThread" ) - .getDeclaredMethod( "stop" ) - .invoke( null ); - } - - - @Override - public void interceptTestMethod( Invocation invocation, ReflectiveInvocationContext invocationContext, ExtensionContext extensionContext ) throws Throwable - { - invocation.skip(); - - ClassLoader loader = extensionContext.getStore( NAMESPACE ).get( IsolatedClassLoader.class, IsolatedClassLoader.class ); - Method method = invocationContext.getExecutable(); - - Class ourClass = loader.loadClass( method.getDeclaringClass().getName() ); - Method ourMethod = ourClass.getDeclaredMethod( method.getName(), method.getParameterTypes() ); - - try - { - ourMethod.invoke( ourClass.getConstructor().newInstance(), invocationContext.getArguments().toArray() ); - } - catch( InvocationTargetException e ) - { - throw e.getTargetException(); - } - } - - private static class IsolatedClassLoader extends SecureClassLoader - { - IsolatedClassLoader() - { - super( IsolatedClassLoader.class.getClassLoader() ); - } - - @Override - public Class loadClass( String name, boolean resolve ) throws ClassNotFoundException - { - synchronized( getClassLoadingLock( name ) ) - { - Class c = findLoadedClass( name ); - if( c != null ) return c; - - if( name.startsWith( "dan200.computercraft." ) ) - { - CodeSource parentSource = getParent().loadClass( name ).getProtectionDomain().getCodeSource(); - - byte[] contents; - try( InputStream stream = getResourceAsStream( name.replace( '.', '/' ) + ".class" ) ) - { - if( stream == null ) throw new ClassNotFoundException( name ); - contents = ByteStreams.toByteArray( stream ); - } - catch( IOException e ) - { - throw new ClassNotFoundException( name, e ); - } - - return defineClass( name, contents, 0, contents.length, parentSource ); - } - } - - return super.loadClass( name, resolve ); - } - } -}