[JAVA] Memory leak when cleaning room events with removeEventsForListener instead of removeEventsForListener

[plenorio]

So we had a memory leak, and i tracked the cause to be the room events.
In the room init we were declaring events like:

Code: Select all

public abstract class MyRoomExt extends BaseSFSExtension {
   private ISFSEventListener reconnectEventHandler = new ISFSEventListener() {
      @Override
      public void handleServerEvent(ISFSEvent event) {
         //to stuff
      }
   };
   
   @Override
   public void init() {
      addEventListener(SFSEventType.USER_DISCONNECT, reconnectEventHandler);
   }
   
   @Override
   public void destroy() {
      removeEventListener(SFSEventType.USER_DISCONNECT, reconnectEventHandler);
   }
}


After trial and error i found out that if i changed removeEventListener to removeEventsForListener, suddenly no more memory leak.

Code: Select all

public abstract class MyRoomExt extends BaseSFSExtension {
   private ISFSEventListener reconnectEventHandler = new ISFSEventListener() {
      @Override
      public void handleServerEvent(ISFSEvent event) {
         //to stuff
      }
   };
   
   @Override
   public void init() {
      addEventListener(SFSEventType.USER_DISCONNECT, reconnectEventHandler);
   }
   
   @Override
   public void destroy() {
      //removeEventListener(SFSEventType.USER_DISCONNECT, reconnectEventHandler);
      //no more leak suddenly!!
      removeEventsForListener(reconnectEventHandler);
   }
}


My problems are solved, so i can move on with my work, but i wonder why this behaviour was ocurring?
I am using server version 2.18, the only thing i can think is maybe the sfs2x.jar/sfs2x-core.jar is old(this project is going since 2018 and dont know since when the jar was updated)
At least learned to work with VisualVM with is amazing.

[Lapo]

Hi,
can I ask why you're extending BaseSFSExtension rather than the recommended SFSExtension?
Using the latter would take care of event listeners automatically and avoid memory management issues.

With this said the first method you were using removes a single Listener, while the second removes all listeners for a specific Event.

Cheers

[plenorio]

can I ask why you're extending BaseSFSExtension rather than the recommended SFSExtension?

Nice catch, guess there was no good reason for that huh

With this said the first method you were using removes a single Listener, while the second removes all listeners for a specific Event.

Yes, but it should be the same thing if i register once, and then removing once should be the same thing as register once and removing all.

Maybe i am missing something(somehow unknown to me registering multiple times?), later i am gonna build a simple example for some sanity check.

[Lapo]

Nice catch, guess there was no good reason for that huh

In that case I'd recommend switching.

Yes, but it should be the same thing if i register once, and then removing once should be the same thing as register once and removing all.

That's correct. We're not aware of any specific issue with that approach (register once, remove once), as long as the reference of the listener is the same when registering and removing.

Cheers