From c820e051b33630c0896e8efbb09b6645d581b785 Mon Sep 17 00:00:00 2001 From: ToadDev <748280+Toad-Dev@users.noreply.github.com> Date: Sat, 22 May 2021 12:43:22 -0700 Subject: [PATCH] Fix: Release monitor buffers on *client* thread These buffers were being released on the server thread, causing the logical server in a single player session to crash just before shutting down. It seems the crash happened late enough to not cause world corruption issues, and so went unnoticed. It was however causing a memory leak when quitting/rejoining worlds, and it left the save file lock in limbo. --- .../proxy/ComputerCraftProxyClient.java | 23 ++++--------------- .../events/ClientUnloadWorldEvent.java | 17 ++++++++++++++ ...aftGame.java => MixinMinecraftClient.java} | 16 ++++++++++++- src/main/resources/computercraft.mixins.json | 2 +- 4 files changed, 37 insertions(+), 21 deletions(-) create mode 100644 src/main/java/dan200/computercraft/events/ClientUnloadWorldEvent.java rename src/main/java/dan200/computercraft/mixin/{MixinMinecraftGame.java => MixinMinecraftClient.java} (52%) diff --git a/src/main/java/dan200/computercraft/client/proxy/ComputerCraftProxyClient.java b/src/main/java/dan200/computercraft/client/proxy/ComputerCraftProxyClient.java index 9365806b0..9983e6a08 100644 --- a/src/main/java/dan200/computercraft/client/proxy/ComputerCraftProxyClient.java +++ b/src/main/java/dan200/computercraft/client/proxy/ComputerCraftProxyClient.java @@ -20,6 +20,7 @@ import dan200.computercraft.client.render.TileEntityMonitorRenderer; import dan200.computercraft.client.render.TileEntityTurtleRenderer; import dan200.computercraft.client.render.TurtleModelLoader; import dan200.computercraft.client.render.TurtlePlayerRenderer; +import dan200.computercraft.events.ClientUnloadWorldEvent; import dan200.computercraft.shared.ComputerCraftRegistry; import dan200.computercraft.shared.common.ContainerHeldItem; import dan200.computercraft.shared.common.IColouredItem; @@ -47,14 +48,13 @@ import net.fabricmc.fabric.api.client.rendereregistry.v1.BlockEntityRendererRegi import net.fabricmc.fabric.api.client.rendereregistry.v1.EntityRendererRegistry; import net.fabricmc.fabric.api.client.screenhandler.v1.ScreenRegistry; import net.fabricmc.fabric.api.event.client.ClientSpriteRegistryCallback; -import net.fabricmc.fabric.api.event.lifecycle.v1.ServerWorldEvents; import net.fabricmc.fabric.mixin.object.builder.ModelPredicateProviderRegistrySpecificAccessor; @Environment (EnvType.CLIENT) public final class ComputerCraftProxyClient implements ClientModInitializer { - public static void initEvents() { - + private static void initEvents() { + ClientUnloadWorldEvent.EVENT.register( () -> ClientMonitor.destroyAll() ); } @Override @@ -96,13 +96,9 @@ public final class ComputerCraftProxyClient implements ClientModInitializer { () -> ComputerCraftRegistry.ModItems.POCKET_COMPUTER_ADVANCED); ClientRegistry.onItemColours(); - // TODO Verify this does things properly - ServerWorldEvents.UNLOAD.register(((minecraftServer, serverWorld) -> { - ClientMonitor.destroyAll(); - })); + initEvents(); } - // My IDE doesn't think so, but we do actually need these generics. private static void registerContainers() { ScreenRegistry.>register(ComputerCraftRegistry.ModContainers.COMPUTER, GuiComputer::create); @@ -125,15 +121,4 @@ public final class ComputerCraftProxyClient implements ClientModInitializer { ModelPredicateProviderRegistrySpecificAccessor.callRegister(item.get(), id, getter); } } - - // @Mod.EventBusSubscriber (modid = ComputerCraft.MOD_ID, value = Dist.CLIENT) - // public static final class ForgeHandlers { - // @SubscribeEvent - // public static void onWorldUnload(WorldEvent.Unload event) { - // if (event.getWorld() - // .isClient()) { - // ClientMonitor.destroyAll(); - // } - // } - // } } diff --git a/src/main/java/dan200/computercraft/events/ClientUnloadWorldEvent.java b/src/main/java/dan200/computercraft/events/ClientUnloadWorldEvent.java new file mode 100644 index 000000000..081997c68 --- /dev/null +++ b/src/main/java/dan200/computercraft/events/ClientUnloadWorldEvent.java @@ -0,0 +1,17 @@ +package dan200.computercraft.events; + +import net.fabricmc.fabric.api.event.Event; +import net.fabricmc.fabric.api.event.EventFactory; + +@FunctionalInterface +public interface ClientUnloadWorldEvent +{ + Event EVENT = EventFactory.createArrayBacked( ClientUnloadWorldEvent.class, + callbacks -> () -> { + for( ClientUnloadWorldEvent callback : callbacks) { + callback.onClientUnloadWorld(); + } + }); + + void onClientUnloadWorld(); +} diff --git a/src/main/java/dan200/computercraft/mixin/MixinMinecraftGame.java b/src/main/java/dan200/computercraft/mixin/MixinMinecraftClient.java similarity index 52% rename from src/main/java/dan200/computercraft/mixin/MixinMinecraftGame.java rename to src/main/java/dan200/computercraft/mixin/MixinMinecraftClient.java index 1dc5fa12f..c468eabbe 100644 --- a/src/main/java/dan200/computercraft/mixin/MixinMinecraftGame.java +++ b/src/main/java/dan200/computercraft/mixin/MixinMinecraftClient.java @@ -7,6 +7,9 @@ package dan200.computercraft.mixin; import dan200.computercraft.client.FrameInfo; +import dan200.computercraft.events.ClientUnloadWorldEvent; +import net.minecraft.client.gui.screen.Screen; +import net.minecraft.client.world.ClientWorld; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.injection.At; import org.spongepowered.asm.mixin.injection.Inject; @@ -15,9 +18,20 @@ import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; import net.minecraft.client.MinecraftClient; @Mixin (MinecraftClient.class) -public abstract class MixinMinecraftGame { +public abstract class MixinMinecraftClient +{ @Inject (method = "render", at = @At ("HEAD")) private void onRender(CallbackInfo info) { FrameInfo.onRenderFrame(); } + + @Inject(method = "disconnect(Lnet/minecraft/client/gui/screen/Screen;)V", at = @At ("RETURN")) + private void disconnectAfter(Screen screen, CallbackInfo info) { + ClientUnloadWorldEvent.EVENT.invoker().onClientUnloadWorld(); + } + + @Inject(method = "joinWorld", at = @At("RETURN")) + private void joinWorldAfter(ClientWorld world, CallbackInfo info) { + ClientUnloadWorldEvent.EVENT.invoker().onClientUnloadWorld(); + } } diff --git a/src/main/resources/computercraft.mixins.json b/src/main/resources/computercraft.mixins.json index e5771bd01..2d45964e1 100644 --- a/src/main/resources/computercraft.mixins.json +++ b/src/main/resources/computercraft.mixins.json @@ -20,7 +20,7 @@ "HeldItemRendererAccess", "MixinHeldItemRenderer", "MixinItemFrameEntityRenderer", - "MixinMinecraftGame", + "MixinMinecraftClient", "MixinScreen", "MixinWorldRenderer" ],