Memory leak of HawkPlayer

[adiantek]

Hi,

There are a lot of HawkPlayer instances in ConcurrentHashMap with attribute online=false. Server 1.8. Tbh don't know how to fix it correctly (I could make some type of GC, eg. removing online=false if smbd is offline longer than 30 secs). ConcurrentHashMap<CraftPlayer, HawkPlayer> doesn't have sense, bcuz HawkPlayer keeps a reference to CraftPlayer.

[Islandscout]

Yes, this is a problem. You are probably the second person who reported this (you're good!)

Yeah, I couldn't figure out a good fix for this but I do like how you mentioned that HawkPlayer keeps a reference to CraftPlayer. Right now I don't have the incentive to keep working on Hawk since I sort of moved on with my life, but I guess this could serve as coding practice. I have no guarantee when this will be fixed.

[Islandscout]

Actually, I don't think there is any Map in the project whose key is CraftPlayer. The HawkPlayers are stored in a map that holds UUID as a key. Also, the HawkPlayer is being removed from the map when a player leaves (via PlayerQuitEvent), so the only way the entry remains is if somehow the player leaves without that event being triggered.

I don't know whether you are using the build from SpigotMC or built the latest version from GitHub. It could simply be that you're using a build that doesn't have these changes.

[adiantek]

ConcurrentHashMap<CraftPlayer, HawkPlayer> doesn't have sense, bcuz HawkPlayer keeps a reference to CraftPlayer

It was only my PoC. It's not in this project.

Also, the HawkPlayer is being removed from the map when a player leaves (via PlayerQuitEvent), so the only way the entry remains is if somehow the player leaves without that event being triggered

You're adding HawkPlayer to the map if netty calls getHawkPlayer? I don't know. I know only that PlayerQuitEvent is not called if PlayerLoginEvent is disallowed - but according to hprof it's not a reason, bcuz in hprof I saw players that were allowed. I think it could be due to calling it async, eg. from netty thread (like netty parsing packet after player is kicked from the server).

If you wanna know - I restart the server only if I need. Sometimes the spigot has 2 weeks uptime, every day one instance has 50 - 100 players. Maybe that's why I saw this memory leak xD (-XX:+HeapDumpOnOutOfMemoryError -XX:+CrashOnOutOfMemoryError helps too)

[Islandscout]

You must have a lot of unique players. The map uses UUID as a key so unless I have a big misconception about hash maps, there shouldn't be duplicate UUIDs as an existing map entry would be overwritten when the player joins. However, this doesn't address the fundamental problem that some entries are supposedly not getting cleared when the player leaves. I will have to find another way to determine when a player leaves the server.

Also, yes, if a HawkPlayer doesn't exist when getHawkPlayer() is called, it will create one, add it to the map, and return it. It could very well be a race condition. Some sort of GC system would be able to handle this, but I really would like to fix the underlying problem.