Use WeakHashMaps in CollectiveEvents
shartte opened this issue ยท 4 comments
While fixing an AE2 issue related to collective, I came across the two hash maps in CollectiveEvents.
See:
https://github.com/ricksouth/serilum-mc-mods/blob/76f9ced01759b5d1296c5c8105f2aa212801e562/sources/Collective/src/main/java/com/natamus/collective/events/CollectiveEvents.java#L51
This is bad. Since the world unload event isn't used to clean up, collective will keep levels alive forever while the game is still running. This can add up if people hop between their single player worlds.
Potential Fix: Use WeakHashMap, allowing Java to clean up the keys.
Appreciate you letting me know, that's not something I've come across yet. Do you recommend using WeakHashMap for any HashMap that references a ServerLevel? I believe I've got other mods which use the same principle.
Yeah. You can also subscribe to ServerWorldEvents.UNLOAD and remove the map-entry in that event.
Appreciate it. Is both making it into a WeakHashMap and subscribing to the unload event to remove the entry a good idea? Or would you suggest to pick one of the two?