Fix minor memory leaks

[embeddedt]

ItemMobSpawner uses a hash map to cache entity instances for rendering them inside spawners. These entities hold a reference to a client world in order to render correctly. However, based on reading that code, that reference only appears to be updated when the spawners are visible in NEI. This leads to the spawners retaining an extra, unnecessary WorldClient instance when the player changes dimensions.

I've fixed this problem by simply removing and recreating the entities if the client world changes.

NEIController keeps a manager instance for a GUI even after it's closed. If the player then switches dimensions, the previous world is not able to be garbage collected till the GUI is reopened. This is fixed by setting manager back to null once the GUI is closed.

[eigenraven]

Can you use a WeakHashMap instead and lazily initialize the entities when they go missing? That way you won't have to rely on calling the cleanup function at the right time

[embeddedt]

Good idea. Thanks for that suggestion, in the process of verifying it worked I actually managed to find two more leaks: one in vanilla and another in NEI (fix included here).

[eigenraven]

Ah sorry, now looking through it I'm not sure a weak map is the best solution. The weak map is the only thing that ever holds references to these entities, as spawnEntityInWorld is never called, so they will be collected on every gc cycle, causing wasteful entity re-creating. The world object on these entities seems to be reset in the renderer class, maybe just set the world object to null after the constructors are run, and add setting it to null to SpawnerRenderer after the rendering is done?

[embeddedt]

I believe the entities are referenced by the world that contains them (the WorldClient, within loadedEntityList) and that is referenced by the Minecraft client class, therefore it shouldn't be possible for them to be collected until the player switches worlds.

[eigenraven]

I believe the entities are referenced by the world that contains them (the WorldClient, within loadedEntityList) and that is referenced by the Minecraft client class, therefore it shouldn't be possible for them to be collected until the player switches worlds.

The problem is that they're not added to that list - the Entity constructor does not put the entity in the loaded list. Only when the World.spawnEntityInWorld function is called that entity would be added there - but these are not "real" entities so NEI doesn't call that function (rightfully)

[embeddedt]

Interesting, I am pretty sure I saw them in loadedEntityList when I was investigating the heap dump, but perhaps I mixed them up with something else.

Should I just revert to my original solution then? I'm not sure how mods will react to the world object on the entity being null.

[eigenraven]

Yeah, the original should be ok

[embeddedt]

Updated.