Skip to content

Commit

Permalink
Fixed the gui system closing the whole game if the forked gui process…
Browse files Browse the repository at this point in the history
… encountered an error, rather than returning the error to the original process.
  • Loading branch information
AlexIIL committed Jun 2, 2024
1 parent 2116c56 commit f6ca0bb
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 6 deletions.
12 changes: 10 additions & 2 deletions src/main/java/org/quiltmc/loader/impl/gui/AbstractWindow.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.function.Supplier;

import org.quiltmc.loader.api.LoaderValue;
import org.quiltmc.loader.api.LoaderValue.LObject;
Expand Down Expand Up @@ -126,15 +127,22 @@ void onClosed() {
sendSignal("closed");
}

void waitUntilClosed() throws LoaderGuiException {
void waitUntilClosed(Supplier<Error> errorChecker) throws LoaderGuiException {
try {
while (true) {
try {
onClosedFuture.get(1, TimeUnit.SECONDS);
break;
} catch (TimeoutException e) {
if (QuiltForkComms.getCurrentComms() == null) {
throw new LoaderGuiException("Forked communication failure; check the log for details!", e);
Error error = errorChecker.get();
if (error == null) {
throw new LoaderGuiException("Forked communication failure; check the log for details!", e);
} else {
LoaderGuiException ex = new LoaderGuiException("Forked communication failure!", error);
ex.addSuppressed(e);
throw ex;
}
}
}
}
Expand Down
19 changes: 15 additions & 4 deletions src/main/java/org/quiltmc/loader/impl/gui/QuiltFork.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public class QuiltFork {
private static final QuiltForkComms COMMS;
private static final LoaderValueHelper<IOException> HELPER = LoaderValueHelper.IO_EXCEPTION;
private static final IOException FORK_EXCEPTION;
private static Error previousServerError;

static {
GameProvider provider = QuiltLoaderImpl.INSTANCE.getGameProvider();
Expand Down Expand Up @@ -130,13 +131,18 @@ public static void open(QuiltLoaderWindow<?> window, boolean shouldWait) throws
return;
}

if (previousServerError != null) {
// Gui NOT disabled, but it previously crashed.
throw new LoaderGuiException("The gui server encountered an error!", previousServerError);
}

AbstractWindow<?> realWindow = (AbstractWindow<?>) window;

realWindow.send();
realWindow.open();

if (shouldWait) {
realWindow.waitUntilClosed();
realWindow.waitUntilClosed(() -> previousServerError);
}
}

Expand Down Expand Up @@ -187,10 +193,15 @@ private static void handleMessageFromServer0(LoaderValue msg) throws IOException
switch (type) {
case ForkCommNames.ID_EXCEPTION: {
// The server encountered an exception
// We should really store the exception, but for now just exit
LoaderValue detail = packet.get("detail");
if (detail == null || detail.type() != LType.STRING) {
Log.error(LogCategory.COMMS, "The gui-server encountered an unknown error!");
previousServerError = new Error("[Unknown error: the gui server didn't send a detail trace]");
} else {
Log.error(LogCategory.COMMS, "The gui-server encountered an error:\n" + detail.asString());
previousServerError = new Error("Previous error stacktrace:\n" + detail.asString());
}
COMMS.close();
Log.error(LogCategory.COMMS, "The gui-server encountered an error!");
System.exit(1);
return;
}
case ForkCommNames.ID_GUI_OBJECT_UPDATE: {
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/org/quiltmc/loader/impl/gui/QuiltForkComms.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.PrintStream;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.lang.ProcessBuilder.Redirect;
import java.net.InetAddress;
import java.net.Socket;
Expand Down Expand Up @@ -380,6 +383,14 @@ private void readMessage(LoaderValue value) {
} catch (Throwable t) {
Map<String, LoaderValue> map = new HashMap<>();
map.put("__TYPE", lvf().string(ForkCommNames.ID_EXCEPTION));

StringWriter sw = new StringWriter();
try (PrintWriter printer = new PrintWriter(sw)) {
t.printStackTrace(printer);
printer.flush();
}
map.put("detail", lvf().string(sw.toString()));

send(lvf().object(map));
throw t;
}
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/changelog/0.26.1.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Bug Fixes:

- Fixed crash reports being printed to system out, rather than the log. (This is a problem for launchers which don't replay sysout, like Modrinth)
- Fixed the gui system closing the whole game if the forked gui process encountered an error, rather than returning the error to the original process.

0 comments on commit f6ca0bb

Please sign in to comment.