From ff6c4e227b4f4b5d604d46586c7b8300955cf6c1 Mon Sep 17 00:00:00 2001 From: James Seibel Date: Sat, 29 Mar 2025 18:18:23 -0500 Subject: [PATCH] level wrapper weak refs to fix leak on bad shutdown --- .../wrappers/world/ClientLevelWrapper.java | 34 +++++++++++++++---- .../wrappers/world/ServerLevelWrapper.java | 34 +++++++++++++++---- coreSubProjects | 2 +- 3 files changed, 56 insertions(+), 14 deletions(-) diff --git a/common/src/main/java/com/seibel/distanthorizons/common/wrappers/world/ClientLevelWrapper.java b/common/src/main/java/com/seibel/distanthorizons/common/wrappers/world/ClientLevelWrapper.java index a9039973d..b4e2ffc4b 100644 --- a/common/src/main/java/com/seibel/distanthorizons/common/wrappers/world/ClientLevelWrapper.java +++ b/common/src/main/java/com/seibel/distanthorizons/common/wrappers/world/ClientLevelWrapper.java @@ -25,7 +25,6 @@ import net.minecraft.server.level.ServerLevel; import net.minecraft.world.level.block.state.BlockState; import net.minecraft.world.level.chunk.ChunkAccess; import net.minecraft.world.level.chunk.ChunkSource; -import net.minecraft.world.phys.Vec3; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -33,6 +32,10 @@ import org.jetbrains.annotations.Nullable; import java.awt.*; import java.io.File; import java.io.IOException; +import java.lang.ref.WeakReference; +import java.util.Collections; +import java.util.Map; +import java.util.WeakHashMap; import java.util.concurrent.ConcurrentHashMap; #if MC_VER <= MC_1_20_4 @@ -49,7 +52,12 @@ import com.seibel.distanthorizons.core.util.ColorUtil; public class ClientLevelWrapper implements IClientLevelWrapper { private static final Logger LOGGER = DhLoggerBuilder.getLogger(ClientLevelWrapper.class.getSimpleName()); - private static final ConcurrentHashMap LEVEL_WRAPPER_BY_CLIENT_LEVEL = new ConcurrentHashMap<>(); // TODO can leak + /** + * weak references are to prevent rare issues + * where, upon world closure, some levels aren't shutdown/removed properly + * and/or for servers were the level object isn't consistent + */ + private static final Map> LEVEL_WRAPPER_REF_BY_CLIENT_LEVEL = Collections.synchronizedMap(new WeakHashMap<>()); private static final IKeyedClientLevelManager KEYED_CLIENT_LEVEL_MANAGER = SingletonInjector.INSTANCE.get(IKeyedClientLevelManager.class); private static final Minecraft MINECRAFT = Minecraft.getInstance(); @@ -72,9 +80,9 @@ public class ClientLevelWrapper implements IClientLevelWrapper - //===============// - // wrapper logic // - //===============// + //==================// + // instance methods // + //==================// public static IClientLevelWrapper getWrapper(@NotNull ClientLevel level) { return getWrapper(level, false); } @@ -96,7 +104,19 @@ public class ClientLevelWrapper implements IClientLevelWrapper } } - return LEVEL_WRAPPER_BY_CLIENT_LEVEL.computeIfAbsent(level, ClientLevelWrapper::new); + return LEVEL_WRAPPER_REF_BY_CLIENT_LEVEL.compute(level, (newLevel, levelRef) -> + { + if (levelRef != null) + { + ClientLevelWrapper oldLevelWrapper = levelRef.get(); + if (oldLevelWrapper != null) + { + return levelRef; + } + } + + return new WeakReference<>(new ClientLevelWrapper(newLevel)); + }).get(); } @Nullable @@ -265,7 +285,7 @@ public class ClientLevelWrapper implements IClientLevelWrapper @Override public void onUnload() { - LEVEL_WRAPPER_BY_CLIENT_LEVEL.remove(this.level); + LEVEL_WRAPPER_REF_BY_CLIENT_LEVEL.remove(this.level); this.parentDhLevel = null; } diff --git a/common/src/main/java/com/seibel/distanthorizons/common/wrappers/world/ServerLevelWrapper.java b/common/src/main/java/com/seibel/distanthorizons/common/wrappers/world/ServerLevelWrapper.java index 12756a199..b6264192f 100644 --- a/common/src/main/java/com/seibel/distanthorizons/common/wrappers/world/ServerLevelWrapper.java +++ b/common/src/main/java/com/seibel/distanthorizons/common/wrappers/world/ServerLevelWrapper.java @@ -20,6 +20,10 @@ package com.seibel.distanthorizons.common.wrappers.world; import java.io.File; +import java.lang.ref.WeakReference; +import java.util.Collections; +import java.util.Map; +import java.util.WeakHashMap; import java.util.concurrent.ConcurrentHashMap; import com.seibel.distanthorizons.api.enums.worldGeneration.EDhApiLevelType; @@ -57,7 +61,11 @@ import org.apache.logging.log4j.Logger; public class ServerLevelWrapper implements IServerLevelWrapper { private static final Logger LOGGER = DhLoggerBuilder.getLogger(); - private static final ConcurrentHashMap LEVEL_WRAPPER_BY_SERVER_LEVEL = new ConcurrentHashMap<>(); + /** + * weak references are to prevent rare issues + * where, upon world closure, some levels aren't shutdown/removed properly + */ + private static final Map> LEVEL_WRAPPER_REF_BY_SERVER_LEVEL = Collections.synchronizedMap(new WeakHashMap<>()); private final ServerLevel level; @Deprecated // TODO circular references are bad @@ -70,15 +78,29 @@ public class ServerLevelWrapper implements IServerLevelWrapper //==============// public static ServerLevelWrapper getWrapper(ServerLevel level) - { return LEVEL_WRAPPER_BY_SERVER_LEVEL.computeIfAbsent(level, ServerLevelWrapper::new); } + { + return LEVEL_WRAPPER_REF_BY_SERVER_LEVEL.compute(level, (newLevel, levelRef) -> + { + if (levelRef != null) + { + ServerLevelWrapper oldLevelWrapper = levelRef.get(); + if (oldLevelWrapper != null) + { + return levelRef; + } + } + + return new WeakReference<>(new ServerLevelWrapper(newLevel)); + }).get(); + } public ServerLevelWrapper(ServerLevel level) { this.level = level; } - //=========// - // methods // - //=========// + //==================// + // instance methods // + //==================// @Override public File getMcSaveFolder() @@ -179,7 +201,7 @@ public class ServerLevelWrapper implements IServerLevelWrapper public ServerLevel getWrappedMcObject() { return this.level; } @Override - public void onUnload() { LEVEL_WRAPPER_BY_SERVER_LEVEL.remove(this.level); } + public void onUnload() { LEVEL_WRAPPER_REF_BY_SERVER_LEVEL.remove(this.level); } @Override diff --git a/coreSubProjects b/coreSubProjects index 4aac61b37..5951f542a 160000 --- a/coreSubProjects +++ b/coreSubProjects @@ -1 +1 @@ -Subproject commit 4aac61b37f3348792ce5ae19a604e8c4809df895 +Subproject commit 5951f542ae46deba6c6336eb0adf8b7405d08907