Possible Memory Leak

[EmanYm]

With this plugin installed on a Purpur server, the RAM usage quickly reaches the cap, and then makes the server unplayable until it crashes for OOM. The RAM usage cap only started to happen after installing this plugin. I have had this plugin disabled for well over 12 hours and have returned to the ~50% RAM usage.

I can provide environment and plugin list if required

[Bram1903]

Hello,

Thanks for reporting this issue. Based upon your information you are most probably correct. I am not too sure how this can happen, because I am removing entities from the cache that are being unloaded or that died, however, this could be through a variety of reasons.

1. Purper perhaps changed something with the EntityUnload event
2. There is a general memory leak across all server software, because Bukkit events can act completely retarded across versions 😅

I understand this is extremely frustrating. I am currently rewriting this plugin into a more modular approach, where I have a common module containing the basic logic, and then some platform-dependent logic for each platform (Bukkit, Velocity, etc).

Because I am switching to a common module, I can't use the Bukkit events any longer, and therefore I am completely rewriting it using nothing more than packets. I will also switch the cache from not storing the complete entity object, but only its ID and type, as that's mostly the only thing I need. (I need the type to determine if the given entity should get its health spoofed or not).

I hope that once the rewrite is finished that will automatically also fix the memory leak, as I am no longer dependent on Bukkit events, but rather native packets.

This issue will be kept open until I rewrite the plugin, and we can re-evaluate the possible memory leak once again! Feel free to react to this message.

Kind regards, Bram

[EmanYm]

1. Purper perhaps changed something with the `EntityUnload` event

I would look at Paper for this. Purpur just takes Paper and adds a few config options and Pufferfish things. If anything is changed it would be on the Paper level

2. There is a general memory leak across all server software, because Bukkit events can act completely retarded across versions 😅

I don't disagree there... 😆

Do you have a ko-fi or patreon that I may use to offer monetary encouragement?

[Bram1903]

Do you have a ko-fi or patreon that I may use to offer monetary encouragement?

Thanks for the offer, but I don't have any of that 😄. I already get my joy from seeing other people use stuff I develop! Thanks anyway!

The rewrite is going quite fast at the moment. I only have to fix a few more things before I can release version 2.0

1. When wolves are excluded from the health spoofer when tamed or owned, once hit their health is being spoofed again.
2. On Minecraft version 1.8 I don't properly detect players riding on a vehicle, so the health of the mounted vehicle is still being spoofed.

I am going to fix those issues later this week, and once they are fixed I can release version 2.0!

[Bram1903]

Update

The earlier-mentioned bugs have been fixed. I now have to solve an issue where I am dependent on the DESTROY_ENTITIES packet that results in entities being removed from the cache if another client is getting out of render distance. This results in the player standing next to those entities in seeing their actual health again.

[EmanYm]

You can do it! I believe in you!

[Bram1903]

You can do it! I believe in you!

Haha, thanks. I did fix this yesterday. Now I have to make sure I minimize the chance of a memory leak occurring by trying to think of every possible way an entity might get removed in a way I am currently not detecting (so I can remove it from my cache). Technically it doesn't really matter anymore as I no longer store the entire entity object, but rather only a few fields.

[HydrolienF]

I can confirm that I have the same issue. Tested with Folia 1.20.4. With 30 players online, after 30 min I had 1 milion element in entityData in the ConcurentHashMap of com.deathmotion.antihealthindicator.managers.CacheManager#1

[ewof]

for us we fixed it with event priority, if theres a plugin that disables spawning of zombies the cache will get filled with all the zombies that never spawn (and never despawn) @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true) should fix

[Bram1903]

Hellow @HydrolienF and @ewof,

Thanks for confirming this issue and mentioning a possible fix! To be honest, I thought that since I had my event priority set to Monitor it would automatically not trigger for canceled events, but apparently, I was mistaken! Thanks for pointing that out!

Version 2.0.0 is almost finished and I hope that in that version all the above issues are fixed! I'll create a beta release in the following days so you guys can check it out yourself! (or you compile the code from the source)

In the new versions, I created a scheduled task that double-checks each entity in the cache if it still exists in the actual world (this is all being done async, so it won't hurt server performance).

Besides that from version 2.0.0, PacketEvents will no longer be shaded, as the library is growing insanely quickly in popularity favoring the performance of the server. For me, it's much easier to just shade the library into my plugin, so you guys don't have to install a dependency, however, this means that if multiple plugins depend on PacketEvents and they all shade their version, that would mean PacketEvents is being run 3/4 times more than necessary. This would hurt the server's performance a lot.

I also removed Adventure and Gson as a dependency. This means that the total size of the plugin has been brought down from 2.9MB to 58KB. Quite an improvement if I say so ;-)

If you do want to test the latest version make sure that you installed the latest dev build from PacketEvents which can be found here: https://ci.codemc.io/job/retrooper/job/packetevents/

Kind regards, Bram

[Bram1903]

Hello @EmanYm @HydrolienF @ewof,

I just created a new pre-release (2.0.0-SNAPSHOT). The Memory leak should be completely fixed now. I added a listener that checks when an entity in the cache has been accessed for the last time, and if it hasn't been for more than a minute I check if the entity still exists in the world. This is all still being done asynchronously.

I also added an option players-only so only players will be cached (PvP-based servers might benefit from this, as the spoofing of animals and mobs isn't relevant for them). Make sure that you delete your current config.yml.

I would greatly appreciate it if you guys could run the latest release and share your thoughts about it! https://github.com/Bram1903/AntiHealthIndicator/releases/tag/v2.0.0-SNAPSHOT

Make sure that you download the latest version of PacketEvents here: https://ci.codemc.io/job/retrooper/job/packetevents/ I decided that from version 2.0.0 I will no longer shade PacketEvents as the library is greatly growing in popularity, and if every plugin will shade their version of PacketEvents the performance of the server will be impacted.

Kind regards, Bram

[HydrolienF]

I will update it on our server and let you know.

[Bram1903]

@HydrolienF any updates? 😅

[ewof]

works great, memory leak fixed

[Bram1903]

The latest release (v2.0.0) solves this issue. Thanks, everyone for their involvement!