GeyserMC / Geyser

A bridge/proxy allowing you to connect to Minecraft: Java Edition servers with Minecraft: Bedrock Edition.
https://geysermc.org
MIT License
4.75k stars 686 forks source link

Geyser Memory Leak | Player Entities map. #5052

Closed PedroMPagani closed 2 months ago

PedroMPagani commented 2 months ago

Describe the bug

The map is: private final Map<UUID, PlayerEntity> playerEntities = new Object2ObjectOpenHashMap<>();

Also, i think the entity id also a mem leak too, i think they should just .clear(). and same with tickable entities aswell.,

EntityCache image image

To Reproduce

*

Expected behaviour

*

Screenshots / Videos

*

Server Version and Plugins

*

Geyser Dump

*

Geyser Version

*

Minecraft: Bedrock Edition Device/Version

*

Additional Context

The map needs to be cleared aswell otherwise it's just going to accumulate more and more until people just leave, or it crashes.

Camotoy commented 2 months ago

removeEntity shown in the screenshot removes an entity from both lists: https://github.com/GeyserMC/Geyser/blob/669a76c628f6a99eb3304b3505331e1609373111/core/src/main/java/org/geysermc/geyser/session/cache/EntityCache.java#L99-L103

Player entities are cleared whenever a FinishConfigurationPacket is sent, which should be sent on most dimension switches (admittedly I'm not too sure how the configuration system works especially on the different proxies): https://github.com/GeyserMC/Geyser/blob/669a76c628f6a99eb3304b3505331e1609373111/core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaFinishConfigurationPacketTranslator.java#L47

Ultimately, we can't just "clear" the map unless we know the client context has reset the known player entities. There could be an issue, but we need to know how to replicate it.

PedroMPagani commented 2 months ago

But in order, the first packet that is going to get sent to the client where you already clear most caches is through the login. I don't see why you have to clear the entities on a finish configuration, which is sent by the backend right? before the login packet is sent. no entities are shown to the client before.

Camotoy commented 2 months ago

I joined your server and was hovering with ~160 player entities. Swapped around dimensions and also just idled for a bit and the number remained around the same. If 160 Bedrock players have 160 player entities in the sidebar each, that's going to be 25600 player entities in your cache, and I'll assume I'm joining at a bit of a low point. Especially seeing as your graph isn't continually going up (which would indicate a memory leak).

PedroMPagani commented 2 months ago

no what you see is the tablist tho? our tab is limited

PedroMPagani commented 2 months ago

image this is the cache rn a simple math would make hte players shown to be 200, but its not that? tab-wise its one thing but this is not tab right its the entities

Camotoy commented 2 months ago

We create a player entity for each tab entry (it gets reused when the player entity actually spawns).

PedroMPagani commented 2 months ago

ah, i see, that makes more sense! thanks mate :)