From 9fbb71e4e9d9f70e2256a73b45482208b01053cf Mon Sep 17 00:00:00 2001 From: PepperCode1 <44146161+PepperCode1@users.noreply.github.com> Date: Wed, 3 Aug 2022 22:36:14 -0700 Subject: [PATCH] Crash and leak fix - Fix crash when invoking ContraptionMovementSetting#get - Fix memory leak in WorldAttached by copying Flywheel's updated version - Fix stockpile switches not preserving settings after being printed --- .../redstone/StockpileSwitchTileEntity.java | 16 +++- .../config/ContraptionMovementSetting.java | 5 +- .../foundation/utility/WorldAttached.java | 75 ++++++++++++++++--- 3 files changed, 83 insertions(+), 13 deletions(-) diff --git a/src/main/java/com/simibubi/create/content/logistics/block/redstone/StockpileSwitchTileEntity.java b/src/main/java/com/simibubi/create/content/logistics/block/redstone/StockpileSwitchTileEntity.java index 150aa9494..6a467744d 100644 --- a/src/main/java/com/simibubi/create/content/logistics/block/redstone/StockpileSwitchTileEntity.java +++ b/src/main/java/com/simibubi/create/content/logistics/block/redstone/StockpileSwitchTileEntity.java @@ -59,17 +59,27 @@ public class StockpileSwitchTileEntity extends SmartTileEntity { super.read(compound, clientPacket); } - @Override - public void write(CompoundTag compound, boolean clientPacket) { + protected void writeCommon(CompoundTag compound) { compound.putFloat("OnAbove", onWhenAbove); compound.putFloat("OffBelow", offWhenBelow); + compound.putBoolean("Inverted", inverted); + } + + @Override + public void write(CompoundTag compound, boolean clientPacket) { + writeCommon(compound); compound.putFloat("Current", currentLevel); compound.putBoolean("Powered", redstoneState); - compound.putBoolean("Inverted", inverted); compound.putBoolean("PoweredAfterDelay", poweredAfterDelay); super.write(compound, clientPacket); } + @Override + public void writeSafe(CompoundTag compound) { + writeCommon(compound); + super.writeSafe(compound); + } + public float getStockLevel() { return currentLevel; } diff --git a/src/main/java/com/simibubi/create/foundation/config/ContraptionMovementSetting.java b/src/main/java/com/simibubi/create/foundation/config/ContraptionMovementSetting.java index c6d012ce0..f197203bd 100644 --- a/src/main/java/com/simibubi/create/foundation/config/ContraptionMovementSetting.java +++ b/src/main/java/com/simibubi/create/foundation/config/ContraptionMovementSetting.java @@ -31,7 +31,10 @@ public enum ContraptionMovementSetting { public static ContraptionMovementSetting get(Block block) { if (block instanceof IMovementSettingProvider provider) return provider.getContraptionMovementSetting(); - return SETTING_SUPPLIERS.get(block).get(); + Supplier supplier = SETTING_SUPPLIERS.get(block); + if (supplier == null) + return null; + return supplier.get(); } public static boolean allAre(Collection blocks, ContraptionMovementSetting are) { diff --git a/src/main/java/com/simibubi/create/foundation/utility/WorldAttached.java b/src/main/java/com/simibubi/create/foundation/utility/WorldAttached.java index 4986354f5..236d38d16 100644 --- a/src/main/java/com/simibubi/create/foundation/utility/WorldAttached.java +++ b/src/main/java/com/simibubi/create/foundation/utility/WorldAttached.java @@ -1,36 +1,50 @@ package com.simibubi.create.foundation.utility; +import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.BiConsumer; +import java.util.function.Consumer; +import java.util.function.Function; import javax.annotation.Nonnull; import net.minecraft.world.level.LevelAccessor; -import net.minecraftforge.common.util.NonNullFunction; public class WorldAttached { - static List> allMaps = new ArrayList<>(); - Map attached; - private final NonNullFunction factory; + // weak references to prevent leaking hashmaps when a WorldAttached is GC'd during runtime + static List>> allMaps = new ArrayList<>(); + private final Map attached; + private final Function factory; - public WorldAttached(NonNullFunction factory) { + public WorldAttached(Function factory) { this.factory = factory; attached = new HashMap<>(); - allMaps.add(attached); + allMaps.add(new WeakReference<>(attached)); } public static void invalidateWorld(LevelAccessor world) { - allMaps.forEach(m -> m.remove(world)); + var i = allMaps.iterator(); + while (i.hasNext()) { + Map map = i.next() + .get(); + if (map == null) { + // If the map has been GC'd, remove the weak reference + i.remove(); + } else { + // Prevent leaks + map.remove(world); + } + } } @Nonnull public T get(LevelAccessor world) { T t = attached.get(world); - if (t != null) - return t; + if (t != null) return t; T entry = factory.apply(world); put(world, entry); return entry; @@ -40,4 +54,47 @@ public class WorldAttached { attached.put(world, entry); } + /** + * Replaces the entry with a new one from the factory and returns the new entry. + */ + @Nonnull + public T replace(LevelAccessor world) { + attached.remove(world); + + return get(world); + } + + /** + * Replaces the entry with a new one from the factory and returns the new entry. + */ + @Nonnull + public T replace(LevelAccessor world, Consumer finalizer) { + T remove = attached.remove(world); + + if (remove != null) + finalizer.accept(remove); + + return get(world); + } + + /** + * Deletes all entries after calling a function on them. + * + * @param finalizer Do something with all of the world-value pairs + */ + public void empty(BiConsumer finalizer) { + attached.forEach(finalizer); + attached.clear(); + } + + /** + * Deletes all entries after calling a function on them. + * + * @param finalizer Do something with all of the values + */ + public void empty(Consumer finalizer) { + attached.values() + .forEach(finalizer); + attached.clear(); + } }