search

AlexModGuy / Citadel

A Library containing shared code used throughout most of my projects. Can be considered a continuation of LLibrary, of which this borrows the majority of its source code from.
25 stars 29 forks source link

Memory Leak: `EntityProperties` prevent Entitys from being garbage collected. #38

Closed MaPePeR closed 3 years ago

MaPePeR commented 3 years ago

ResourcefulBees' apiary breeder creates temporary entitys to do it's breeding. That can create a lot of Entitys. That wouldn't be as bad in itself, if Citadel wouldn't prevent the entitys from being garbage collected. See Resourceful-Bees/ResourcefulBees#266

This is one of the hard references of an ResourcefulBee: image

The EntityDataHandler creates a WeakIdentityHashMap<Entity, List<IEntityData<?>>>, but the WeakIdentityHashMap is only Weak-Referenced in its keys. EntityProperties (implementing IEntityData) and all subclasses keep a hard reference to the Entity Object preventing it from being garbage collected causing a memory leak.

AlexModGuy commented 3 years ago

Does the config option in citadel.cfg not work?

MaPePeR commented 3 years ago

Setting "Track Entities" = false did indeed fix the memory leak. Not sure what's the other consequences of that are, though. I guess you do Entity-Tracking for a reason? Didn't expect to find a "don't memory leak"-option, so I didn't really look, to be honest.

A real fix would probably be to also store a WeakReference in EntityProperties? Since entity is already private the subclasses should go through getEntity() anyway, which could then resolve the WeakReference. Subclasses would need to be aware of the possibility of getting null from getEntity(), though.

Edit: also didn't find any reference to the config in the code, so I assume the latest version is not uploaded here?

AlexModGuy commented 3 years ago

Got it, from 1.7.2 onwards entity will be a WeakReference.