henkelmax / simple-voice-chat

A working voice chat in Minecraft!
https://modrepo.de/minecraft/voicechat/wiki
422 stars 108 forks source link

Fix Event System Memory Leak #671

Closed KaptainWutax closed 4 months ago

KaptainWutax commented 4 months ago

The event system keeps many objects in memory as the listeners are never deregistered. The biggest consequence has to do with PlayerStateManager's constructor. It registers a lambda onServerVoiceChatConnected which is never deregistered which means the garbage collector can't clean up PlayerStateManager which can't clean up Server which can't clean up MinecraftServer. This makes it impossible to load into more than a handful of worlds before requiring a restart.

This PR fixes the memory leak by registering the event only once globally, which won't prevent the garbage collector from discarding the world instances when they are no longer used. I have been unable to test the fix as my workspace is broken so please review carefully. It would be nice if the fix was backported too, I need it for 1.20.1.

henkelmax commented 4 months ago

Thank you for investigating this issue! The code looks flawless. I'm just testing it to be sure everything works. I'll backport this to all currently supported versions (including 1.20.1).

henkelmax commented 4 months ago

Alright, your fix is now published. Here a link to the 1.20.1 version: https://modrinth.com/plugin/simple-voice-chat/version/fabric-1.20.1-2.5.7

Thanks again for the PR @KaptainWutax!

KaptainWutax commented 4 months ago

No worries, I'm glad to hear it's working! Will it be on CurseForge soon as well?

henkelmax commented 4 months ago

CurseForge takes about 1-2 days to get approved.

KaptainWutax commented 4 months ago

I forgot about that. Thank you!