search

neoforged / Bus

Event firing and listening framework, based on the event bus concept
GNU Lesser General Public License v2.1
3 stars 8 forks source link

Potential memory leak when registering object on Event Bus #34

Open pietro-lopes opened 3 weeks ago

pietro-lopes commented 3 weeks ago

Minecraft Version: 1.21.1

NeoForge Version: 21.1.22

Logs: image

Steps to Reproduce:

  1. Join some world with Spark installed
  2. Leave to Main Menu
  3. Join world again
  4. Profile memory and look for NeoForgeServerSparkPlugin
  5. You will see 2 living instances, consequently 2 IntegratedServer instances

Description of issue: When you unregister from the event bus, it marks the ListenerList to be rebuilt

The rebuild process is triggered when you call getListeners, which happens the next time this event is posted.

For this Spark case this ServerStoppingEvent will be only called again when you leave the server for a second time, making you always have at least 1 leaked object if you leave and join a world.

With that said, should Spark clear their object and do not rely on Event Bus to release their object or there is something that can be fixed on NF side?

Technici4n commented 3 weeks ago

Since unregister is a relatively uncommon operation, I think it would be reasonable to rebuild the list immediately. Alternatively, we could rebuild it with some delay on a background thread. Finally, it would also be possible to use a WeakReference on spark's end to avoid keeping the server reference alive.

sciwhiz12 commented 3 weeks ago

As this issue is mostly unrelated to NeoForge and more to do with how https://github.com/neoforged/Bus handles deregistration of listeners, I'm moving this to that repo.