Advanced-Rocketry / AdvancedRocketry

Space mod for minecraft
http://arwiki.dmodoomsirius.me/
MIT License
217 stars 273 forks source link

[BUG]Memory Leak #2228

Open ginyai opened 2 years ago

ginyai commented 2 years ago

Version of Advanced Rocketry

AdvancedRocketry-1.12.2-1.7.0-240

Have you verified this is an issue in the latest unstable build

Version of LibVulpes

LibVulpes-1.12.2-0.4.2-75

Version of Minecraft

Minecraft 1.12.2

Does this occur without other mods installed

Description of the problem

Memory is used up and cannot be GC when played for a long time. I make a heap dump, analyze it with Eclipse Memory Analyzer and find 215 instances of "net.minecraft.client.multiplayer.WorldClient". Select one and use Path To GC Roots, get this. 20210919095731

I think the issue is here. https://github.com/Advanced-Rocketry/AdvancedRocketry/blob/8ddea81a529868aaaa929225338928532342bba6/src/main/java/zmaster587/advancedRocketry/tile/TileRocketAssemblingMachine.java#L114-L142 The tile will be registered to the Forge EVENT_BUS at init, and be unregistered when method invalidate or onChunkUnload is called. But at world unload both two methods will not be called, which leads to the memory leak.

zmaster587 commented 2 years ago

Looking at minecraft's world handling code, it appears that worlds unload only when there's no chunks loaded still, in fact that appears to be THE trigger for world unload. Chunk unload appears to be called before the world is unloaded. In situations where the server isn't being shut down.

See net.minecraft.world.gen.ChunkProviderServer

Correct me if I'm wrong, but what I'm seeing is something like this:

on every server tick:   (minecraftServer.java:updateTimeLightAndEntities)
  for every world:         (minecraftServer.java:updateTimeLightAndEntities)
    tick chunk provider:  (WorldServer.java:tick)
       unload relevent chunks (ChunkProviderServer:tick)
       if no chunks are loaded: add world to unload queue (ChunkProviderServer:tick)
  for every world in the unload world queue:  (minecraftServer.java:updateTimeLightAndEntities)
     unload world   (DimensionManager.java:unloadWorlds)

Are you seeing a codepath somewhere that the world is getting unloaded without the chunks being unloaded first that occurs outside server shutdown? If so can you point it out because I'm having difficulty finding one.

I've also noticed you've got mods installed with the memory analyzer running. Could you show an instance where there's no other mods just to rule out the possibility of other mods interfering with chunk loading and unloading?

ginyai commented 2 years ago

It's the client-side's world. SPacketRespawn -> NetHandlerPlayClient:handleRespawn -> Minecraft:loadWorld