EntityPlayerMP leak in SyncHandler

[fraenkelc]

Versions:

• SimplyJetpacks2-1.12.2-2.2.3.33.jar
• forge-14.23.3.2678

Issue:
I'm currently analyzing a heap dump from a private server. It seems as if EntityPlayerMP objects are all being kept alive by tonius.simplyjetpacks.handler.SyncHandler.descendKeyState.

The affected lines in the source:
Player is added here:
https://github.com/Tomson124/SimplyJetpacks-2/blob/1.12/src/main/java/tonius/simplyjetpacks/handler/SyncHandler.java#L63-L72

Player is not removed here:
https://github.com/Tomson124/SimplyJetpacks-2/blob/1.12/src/main/java/tonius/simplyjetpacks/handler/SyncHandler.java#L91-L107

furthermore we've got some Players (with huge amounts of pending packets) being kept alive by rightKeyState. According to the code this should be cleaned up though. Could it be that the key handler can be called after a the onPlayerLoggedOut() event is being called by forge?

[thundr]

I added the keystate removal, could you be more specific with the second issue?

[fraenkelc]

Well, when looking at the heap dump I see the player objects in both descendKeyState and rightKeyState. You fixed the descendKeyState case in fe6c365. But there's still some objects left for disconnected players in rightKeyState. I was just guessing at possible reasons why they could be there as the code to clean them up is there.

I haven't tried reproducing the issue so far. from what I've seen in the server logs the players got disconnected when the players change dimensions and the server chugged on worldgen lag