From 51bb3eec3d51f1e26bd498c773628b7bc6bf992c Mon Sep 17 00:00:00 2001 From: TomTheFurry Date: Sun, 16 Jul 2023 17:23:31 +0800 Subject: [PATCH] Fix f3 msg causing mem leaks & render file throwing (harmless) exceptions on shutdown --- .../file/fullDatafile/FullDataMetaFile.java | 10 +---- .../file/renderfile/RenderMetaDataFile.java | 7 +++- .../renderfile/RenderSourceFileHandler.java | 2 +- .../core/generation/WorldGenerationQueue.java | 8 +--- .../core/level/ClientLevelModule.java | 4 +- .../core/logging/f3/F3Screen.java | 38 ++++++++++--------- .../distanthorizons/core/util/LodUtil.java | 14 ++++++- .../UncheckedInterruptedException.java | 8 ++-- .../core/world/DhClientServerWorld.java | 4 +- 9 files changed, 55 insertions(+), 40 deletions(-) diff --git a/core/src/main/java/com/seibel/distanthorizons/core/file/fullDatafile/FullDataMetaFile.java b/core/src/main/java/com/seibel/distanthorizons/core/file/fullDatafile/FullDataMetaFile.java index 150acb499..8cd060da7 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/file/fullDatafile/FullDataMetaFile.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/file/fullDatafile/FullDataMetaFile.java @@ -205,16 +205,10 @@ public class FullDataMetaFile extends AbstractMetaDataContainerFile implements I }) .whenComplete((fullDataSource, ex) -> { - if (ex instanceof CompletionException) { - ex = ex.getCause(); - } - if (ex instanceof InterruptedException || ex instanceof RejectedExecutionException) - { - // this exception can be ignored - } - else if (ex != null) { + if (ex != null && !LodUtil.isInterruptOrReject(ex)) { LOGGER.error("Error updating file "+this.file+": ", ex); } + if (fullDataSource != null) { new DataObjTracker(fullDataSource); new DataObjSoftTracker(this, fullDataSource); diff --git a/core/src/main/java/com/seibel/distanthorizons/core/file/renderfile/RenderMetaDataFile.java b/core/src/main/java/com/seibel/distanthorizons/core/file/renderfile/RenderMetaDataFile.java index 16b5b3641..4d8d9dce4 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/file/renderfile/RenderMetaDataFile.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/file/renderfile/RenderMetaDataFile.java @@ -14,6 +14,7 @@ import com.seibel.distanthorizons.core.level.IDhClientLevel; import com.seibel.distanthorizons.core.render.renderer.IDebugRenderable; import com.seibel.distanthorizons.core.util.AtomicsUtil; import com.seibel.distanthorizons.core.util.LodUtil; +import com.seibel.distanthorizons.core.util.objects.UncheckedInterruptedException; import com.seibel.distanthorizons.core.util.objects.dataStreams.DhDataInputStream; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.Nullable; @@ -222,7 +223,8 @@ public class RenderMetaDataFile extends AbstractMetaDataContainerFile implements { if (ex != null) { - LOGGER.error("Uncaught error on creation {}: ", this.file, ex); + if (!LodUtil.isInterruptOrReject(ex)) + LOGGER.error("Uncaught error on creation {}: ", this.file, ex); cachedRenderDataSource = new SoftReference<>(null); renderSourceLoadFutureRef.set(null); future.complete(null); @@ -263,7 +265,8 @@ public class RenderMetaDataFile extends AbstractMetaDataContainerFile implements { if (ex != null) { - LOGGER.error("Error loading file {}: ", this.file, ex); + if (!LodUtil.isInterruptOrReject(ex)) + LOGGER.error("Error loading file {}: ", this.file, ex); cachedRenderDataSource = new SoftReference<>(null); renderSourceLoadFutureRef.set(null); future.complete(null); diff --git a/core/src/main/java/com/seibel/distanthorizons/core/file/renderfile/RenderSourceFileHandler.java b/core/src/main/java/com/seibel/distanthorizons/core/file/renderfile/RenderSourceFileHandler.java index 43b6e7fac..fd3fd431b 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/file/renderfile/RenderSourceFileHandler.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/file/renderfile/RenderSourceFileHandler.java @@ -418,7 +418,7 @@ public class RenderSourceFileHandler implements ILodRenderSourceProvider int ignoreEmptyWarning = 0; } - else if (!UncheckedInterruptedException.isThrowableInterruption(ex)) + else if (!UncheckedInterruptedException.isInterrupt(ex)) { LOGGER.error("Exception when updating render file using data source: ", ex); } diff --git a/core/src/main/java/com/seibel/distanthorizons/core/generation/WorldGenerationQueue.java b/core/src/main/java/com/seibel/distanthorizons/core/generation/WorldGenerationQueue.java index 11f79963f..0e003dd77 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/generation/WorldGenerationQueue.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/generation/WorldGenerationQueue.java @@ -14,13 +14,9 @@ import com.seibel.distanthorizons.core.pos.DhSectionPos; import com.seibel.distanthorizons.core.config.Config; import com.seibel.distanthorizons.core.dataObjects.transformers.LodDataBuilder; import com.seibel.distanthorizons.core.file.fullDatafile.FullDataFileHandler; -import com.seibel.distanthorizons.core.generation.tasks.*; -import com.seibel.distanthorizons.core.pos.*; import com.seibel.distanthorizons.core.render.renderer.DebugRenderer; import com.seibel.distanthorizons.core.render.renderer.IDebugRenderable; import com.seibel.distanthorizons.core.util.ThreadUtil; -import com.seibel.distanthorizons.core.util.objects.quadTree.QuadNode; -import com.seibel.distanthorizons.core.util.objects.quadTree.QuadTree; import com.seibel.distanthorizons.core.util.objects.UncheckedInterruptedException; import com.seibel.distanthorizons.core.util.LodUtil; import com.seibel.distanthorizons.core.wrapperInterfaces.IWrapperFactory; @@ -430,7 +426,7 @@ public class WorldGenerationQueue implements Closeable, IDebugRenderable if (exception != null) { // don't log the shutdown exceptions - if (!UncheckedInterruptedException.isThrowableInterruption(exception) + if (!UncheckedInterruptedException.isInterrupt(exception) && !(exception instanceof CancellationException || exception.getCause() instanceof CancellationException)) { LOGGER.error("Error generating data for section "+taskPos, exception); @@ -566,7 +562,7 @@ public class WorldGenerationQueue implements Closeable, IDebugRenderable exception = exception.getCause(); } - if (!UncheckedInterruptedException.isThrowableInterruption(exception) && !(exception instanceof CancellationException)) + if (!UncheckedInterruptedException.isInterrupt(exception) && !(exception instanceof CancellationException)) { LOGGER.error("Error when terminating data generation for section "+runningTaskGroup.group.pos, exception); } diff --git a/core/src/main/java/com/seibel/distanthorizons/core/level/ClientLevelModule.java b/core/src/main/java/com/seibel/distanthorizons/core/level/ClientLevelModule.java index c4f84e7cb..ba0499ccd 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/level/ClientLevelModule.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/level/ClientLevelModule.java @@ -23,10 +23,11 @@ import com.seibel.distanthorizons.core.wrapperInterfaces.world.ILevelWrapper; import com.seibel.distanthorizons.coreapi.util.math.Mat4f; import org.apache.logging.log4j.Logger; +import java.io.Closeable; import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicReference; -public class ClientLevelModule { +public class ClientLevelModule implements Closeable { private static final Logger LOGGER = DhLoggerBuilder.getLogger(); private static final IMinecraftClientWrapper MC_CLIENT = SingletonInjector.INSTANCE.get(IMinecraftClientWrapper.class); private final IDhClientLevel parent; @@ -192,6 +193,7 @@ public class ClientLevelModule { ClientRenderState.close(); } } + f3Message.close(); } diff --git a/core/src/main/java/com/seibel/distanthorizons/core/logging/f3/F3Screen.java b/core/src/main/java/com/seibel/distanthorizons/core/logging/f3/F3Screen.java index f17d99f72..f5ab0e804 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/logging/f3/F3Screen.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/logging/f3/F3Screen.java @@ -3,10 +3,7 @@ package com.seibel.distanthorizons.core.logging.f3; import com.seibel.distanthorizons.coreapi.ModInfo; import java.io.Closeable; -import java.util.Arrays; -import java.util.Iterator; -import java.util.LinkedList; -import java.util.List; +import java.util.*; import java.util.function.Supplier; public class F3Screen @@ -15,22 +12,25 @@ public class F3Screen "", // blank line for spacing ModInfo.READABLE_NAME + " version: " + ModInfo.VERSION }; - private static final LinkedList SELF_UPDATE_MESSAGE_LIST = new LinkedList<>(); + private static final List SELF_UPDATE_MESSAGE_LIST = Collections.synchronizedList(new LinkedList<>()); public static void addStringToDisplay(List list) { list.addAll(Arrays.asList(DEFAULT_STRING)); - Iterator iterator = SELF_UPDATE_MESSAGE_LIST.iterator(); - while (iterator.hasNext()) - { - Message message = iterator.next(); - if (message == null) + synchronized (SELF_UPDATE_MESSAGE_LIST) + { + Iterator iterator = SELF_UPDATE_MESSAGE_LIST.iterator(); + while (iterator.hasNext()) { - iterator.remove(); - } - else - { - message.printTo(list); + Message message = iterator.next(); + if (message == null) + { + iterator.remove(); + } + else + { + message.printTo(list); + } } } } @@ -45,12 +45,16 @@ public class F3Screen // and because this class shouldn't be used in a try {} block. public static abstract class Message implements Closeable { - protected Message() { SELF_UPDATE_MESSAGE_LIST.add(this); } + protected Message() { + SELF_UPDATE_MESSAGE_LIST.add(this); + } public abstract void printTo(List output); @Override - public void close() { SELF_UPDATE_MESSAGE_LIST.remove(this); } + public void close() { + boolean removed = SELF_UPDATE_MESSAGE_LIST.remove(this); + } } public static class StaticMessage extends Message diff --git a/core/src/main/java/com/seibel/distanthorizons/core/util/LodUtil.java b/core/src/main/java/com/seibel/distanthorizons/core/util/LodUtil.java index 56885f1ab..02bad3b97 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/util/LodUtil.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/util/LodUtil.java @@ -20,6 +20,8 @@ package com.seibel.distanthorizons.core.util; import java.util.Iterator; +import java.util.concurrent.CompletionException; +import java.util.concurrent.RejectedExecutionException; import com.seibel.distanthorizons.api.enums.config.EVanillaOverdraw; import com.seibel.distanthorizons.core.dependencyInjection.SingletonInjector; @@ -29,6 +31,7 @@ import com.seibel.distanthorizons.core.pos.Pos2D; import com.seibel.distanthorizons.core.render.vertexFormat.DefaultLodVertexFormats; import com.seibel.distanthorizons.core.render.vertexFormat.LodVertexFormat; import com.seibel.distanthorizons.core.util.gridList.EdgeDistanceBooleanGrid; +import com.seibel.distanthorizons.core.util.objects.UncheckedInterruptedException; import com.seibel.distanthorizons.core.wrapperInterfaces.minecraft.IMinecraftRenderWrapper; import com.seibel.distanthorizons.core.wrapperInterfaces.minecraft.IMinecraftClientWrapper; import com.seibel.distanthorizons.core.wrapperInterfaces.world.IDimensionTypeWrapper; @@ -297,6 +300,15 @@ public class LodUtil public static void assertToDo() { throw new AssertFailureException("TODO!"); } - + + public static Throwable ensureUnwrap(Throwable t) { + return t instanceof CompletionException ? ensureUnwrap(t.getCause()) : t; + } + + public static boolean isInterruptOrReject(Throwable t) { + Throwable unwrapped = LodUtil.ensureUnwrap(t); + return UncheckedInterruptedException.isInterrupt(unwrapped) || + unwrapped instanceof RejectedExecutionException; + } } diff --git a/core/src/main/java/com/seibel/distanthorizons/core/util/objects/UncheckedInterruptedException.java b/core/src/main/java/com/seibel/distanthorizons/core/util/objects/UncheckedInterruptedException.java index c385e88fa..f88ad376a 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/util/objects/UncheckedInterruptedException.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/util/objects/UncheckedInterruptedException.java @@ -1,5 +1,7 @@ package com.seibel.distanthorizons.core.util.objects; +import com.seibel.distanthorizons.core.util.LodUtil; + import java.util.concurrent.CompletionException; public class UncheckedInterruptedException extends RuntimeException { @@ -38,8 +40,8 @@ public class UncheckedInterruptedException extends RuntimeException { rethrowIfIsInterruption(t.getCause()); } } - public static boolean isThrowableInterruption(Throwable t) { - return t instanceof InterruptedException || t instanceof UncheckedInterruptedException - || (t instanceof CompletionException && isThrowableInterruption(t.getCause())); + public static boolean isInterrupt(Throwable t) { + Throwable unwrapped = LodUtil.ensureUnwrap(t); + return unwrapped instanceof InterruptedException || unwrapped instanceof UncheckedInterruptedException; } } diff --git a/core/src/main/java/com/seibel/distanthorizons/core/world/DhClientServerWorld.java b/core/src/main/java/com/seibel/distanthorizons/core/world/DhClientServerWorld.java index 728a82cb4..6c38ee926 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/world/DhClientServerWorld.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/world/DhClientServerWorld.java @@ -101,7 +101,9 @@ public class DhClientServerWorld extends AbstractDhWorld implements IDhClientWor } else { - // TODO why is this called here? + // If the level wrapper is a Client Level Wrapper, then that means the client side leaves the level, + // but note that the server side still has the level loaded. So, we don't want to unload the level, + // we just want to stop rendering it. this.levelObjMap.remove(wrapper).stopRenderer(); // Ignore resource warning. The level obj is referenced elsewhere. } }