Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client gametest threading tweaks #4304

Merged
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Ensure game closes promptly when it crashes
Earthcomputer committed Dec 16, 2024
commit cc4950ea9fff9ca637659e9071a278d03a25b80e
Original file line number Diff line number Diff line change
@@ -40,15 +40,11 @@ public static void start() {
for (FabricClientGameTest gameTest : gameTests) {
context.restoreDefaultGameOptions();

try {
gameTest.runTest(context);
} finally {
context.getInput().clearKeysDown();
checkFinalGameTestState(context, gameTest.getClass().getName());
}
}
gameTest.runTest(context);

context.clickScreenButton("menu.quit");
context.getInput().clearKeysDown();
checkFinalGameTestState(context, gameTest.getClass().getName());
}
});
}

Original file line number Diff line number Diff line change
@@ -26,6 +26,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import net.minecraft.client.MinecraftClient;

/**
* <h1>Implementation notes</h1>
*
@@ -75,6 +77,7 @@ private ThreadingImpl() {
private static final int PHASE_MASK = 3;

public static final Phaser PHASER = new Phaser();
private static volatile boolean enablePhases = true;

public static volatile boolean isClientRunning = false;
public static volatile boolean clientCanAcceptTasks = false;
@@ -87,48 +90,72 @@ private ThreadingImpl() {
@Nullable
public static Thread testThread = null;
public static final Semaphore TEST_SEMAPHORE = new Semaphore(0);
@Nullable
public static Throwable testFailureException = null;

@Nullable
public static Runnable taskToRun = null;

private static volatile boolean gameCrashed = false;

public static void enterPhase(int phase) {
while ((PHASER.getPhase() & PHASE_MASK) != phase) {
while (enablePhases && (PHASER.getPhase() & PHASE_MASK) != phase) {
PHASER.arriveAndAwaitAdvance();
}

if (enablePhases) {
PHASER.arriveAndAwaitAdvance();
}
}

PHASER.arriveAndAwaitAdvance();
public static void setGameCrashed() {
enablePhases = false;
gameCrashed = true;
}

public static void runTestThread(Runnable test) {
public static void runTestThread(Runnable testRunner) {
Preconditions.checkState(testThread == null, "There is already a test thread running");

testThread = new Thread(() -> {
PHASER.register();
enterPhase(PHASE_TEST);

try {
test.run();
testRunner.run();
} catch (Throwable e) {
LOGGER.error("Failed to run client gametests", e);
System.exit(1);
testFailureException = e;
} finally {
PHASER.arriveAndDeregister();

if (clientCanAcceptTasks) {
CLIENT_SEMAPHORE.release();
runOnClient(() -> MinecraftClient.getInstance().scheduleStop());
}

if (serverCanAcceptTasks) {
SERVER_SEMAPHORE.release();
if (testFailureException != null) {
// Log this now in case the client has stopped or is otherwise unable to rethrow our exception
LOGGER.error("Client gametests failed with an exception", testFailureException);
}

testThread = null;
deregisterTestThread();
}
});
testThread.setName("Test thread");
testThread.setDaemon(true);
testThread.start();
}

private static void deregisterTestThread() {
testThread = null;
enablePhases = false;
PHASER.arriveAndDeregister();

if (clientCanAcceptTasks) {
CLIENT_SEMAPHORE.release();
}

if (serverCanAcceptTasks) {
SERVER_SEMAPHORE.release();
}
}

public static void checkOnGametestThread(String methodName) {
Preconditions.checkState(Thread.currentThread() == testThread, "%s can only be called from the client gametest thread", methodName);
}
@@ -207,5 +234,17 @@ public static void runTick() {
}

enterPhase(PHASE_TEST);

// Check if the game has crashed during this tick. If so, don't do any more work in the test
if (gameCrashed) {
deregisterTestThread();

try {
// wait until game is closed
new Semaphore(0).acquire();
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -50,7 +50,7 @@ public class MinecraftClientMixin {
private Overlay overlay;

@WrapMethod(method = "run")
private void onRun(Operation<Void> original) {
private void onRun(Operation<Void> original) throws Throwable {
if (ThreadingImpl.isClientRunning) {
throw new IllegalStateException("Client is already running");
}
@@ -61,12 +61,21 @@ private void onRun(Operation<Void> original) {
try {
original.call();
} finally {
ThreadingImpl.clientCanAcceptTasks = false;
ThreadingImpl.PHASER.arriveAndDeregister();
ThreadingImpl.isClientRunning = false;
deregisterClient();

if (ThreadingImpl.testFailureException != null) {
throw ThreadingImpl.testFailureException;
}
}
}

@Inject(method = "cleanUpAfterCrash", at = @At("HEAD"))
private void deregisterAfterCrash(CallbackInfo ci) {
// Deregister a bit earlier than normal to allow for the integrated server to stop without waiting for the client
ThreadingImpl.setGameCrashed();
deregisterClient();
}

@Inject(method = "tick", at = @At("HEAD"))
private void onTick(CallbackInfo ci) {
if (!startedClientGametests && overlay == null) {
@@ -152,4 +161,13 @@ private static void checkThreadOnGetInstance(CallbackInfoReturnable<MinecraftCli
"MinecraftClient.getInstance() cannot be called from the gametest thread. Try using ClientGameTestContext.runOnClient or ClientGameTestContext.computeOnClient"
);
}

@Unique
private static void deregisterClient() {
if (ThreadingImpl.isClientRunning) {
ThreadingImpl.clientCanAcceptTasks = false;
ThreadingImpl.PHASER.arriveAndDeregister();
ThreadingImpl.isClientRunning = false;
}
}
}
Original file line number Diff line number Diff line change
@@ -19,10 +19,12 @@
import com.llamalad7.mixinextras.injector.wrapmethod.WrapMethod;
import com.llamalad7.mixinextras.injector.wrapoperation.Operation;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.Unique;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;

import net.minecraft.client.MinecraftClient;
import net.minecraft.server.MinecraftServer;

import net.fabricmc.fabric.impl.client.gametest.ThreadingImpl;
@@ -41,12 +43,21 @@ private void onRunServer(Operation<Void> original) {
try {
original.call();
} finally {
ThreadingImpl.serverCanAcceptTasks = false;
ThreadingImpl.PHASER.arriveAndDeregister();
ThreadingImpl.isServerRunning = false;
deregisterServer();
}
}

@Inject(method = "runServer", at = @At(value = "INVOKE", target = "Lnet/minecraft/server/MinecraftServer;setCrashReport(Lnet/minecraft/util/crash/CrashReport;)V", shift = At.Shift.AFTER))
protected void onCrash(CallbackInfo ci) {
if (ThreadingImpl.testFailureException == null) {
ThreadingImpl.testFailureException = new Throwable("The server crashed");
}

MinecraftClient.getInstance().scheduleStop();
ThreadingImpl.setGameCrashed();
deregisterServer();
}

@Inject(method = "runServer", at = @At(value = "INVOKE", target = "Lnet/minecraft/server/MinecraftServer;runTasksTillTickEnd()V"))
private void preRunTasks(CallbackInfo ci) {
ThreadingImpl.enterPhase(ThreadingImpl.PHASE_SERVER_TASKS);
@@ -78,4 +89,11 @@ private void postRunTasks(CallbackInfo ci) {

ThreadingImpl.enterPhase(ThreadingImpl.PHASE_TICK);
}

@Unique
private void deregisterServer() {
ThreadingImpl.serverCanAcceptTasks = false;
ThreadingImpl.PHASER.arriveAndDeregister();
ThreadingImpl.isServerRunning = false;
}
}