search

mobends / MoBends

A Minecraft mod that adds more realistic looking animations to the inhabitants of your blocky world.
MIT License
138 stars 30 forks source link

(1.12) Memory Leak: EntityDatabase#entryMap #262

Open Meldexun opened 2 years ago

Meldexun commented 2 years ago

Describe the bug The EntityDatabase#entryMap map stores LivingEntityData as values which have a reference to their entity instance. This map should be cleared in EntityDatabase#updateClient() but I think it would be better to clear the map with an IWorldEventListener#onEntityRemoved(Entity) and the PlayerLoggedOutEvent. Otherwise while in the main menu a WorldClient will still be loaded in memory.

https://github.com/mobends/MoBends/blob/3b56b02e59fcf2d8571d3fedb6ceee8f8824ba05/src/main/java/goblinbob/mobends/core/data/EntityDatabase.java#L22

To Reproduce Steps to reproduce the behavior:

  1. Join a world
  2. Leave the world
  3. See WorldClient instance not being garbage collected

Context (please complete the following information):

iwoplaza commented 2 years ago

Finally got around to this issue, thanks for the report. Learned about weak references recently, and about WeakHashMap. I think this should eliminate the issue entirely. What do you think?

Meldexun commented 2 years ago

WeakHashMap: "Hash table based implementation of the Map interface, with weak keys." (https://docs.oracle.com/javase/8/docs/api/java/util/WeakHashMap.html)

So you would need to change it to use the entity as the key. Then this should work fine.

Edit: Keep in mind that the values of a WeakHashMap are held by strong references. This means the entries of the map won't be cleared if the value prevent the keys from being garbage collected. This is the case here since LivingEntityData holds a reference to the Entity object.

zOnlyKroks commented 11 months ago

Fixed in my fork if this was not fixed already, alongside with another huge one (i talk about 12gb in a few minutes)