Weak References to Event Listeners #7

New Issue

delvh · 2021-02-17T12:28:52+01:00

delvh commented

2021-02-17 12:28:52 +01:00

Currently, if the GC would otherwise garbage collect an object, it currently can't as it will still be referenced from event bus.
I doubt that every user (i.e. us) will dereference an object in EventBus when it should otherwise be GC'ed.

Any reference to the foreign object should be referenced using WeakReference or something similar.

Currently, if the GC would otherwise garbage collect an object, it currently can't as it will still be referenced from event bus. I doubt that every user (i.e. **us**) will dereference an object in `EventBus` when it should otherwise be GC'ed. Any reference to the foreign object should be referenced using `WeakReference` or something similar.

kske was assigned by delvh

2021-02-17 12:28:52 +01:00

kske changed title from ~~[Bug] Only listen weakly on objects to avoid memory leaks~~ to Only listen weakly on objects to avoid memory leaks

2021-02-17 12:36:26 +01:00

kske added the

bug

label 2021-02-17 12:36:32 +01:00

kske commented

2021-02-17 12:41:38 +01:00

For that reason, EventBus#removeListener exists. If we no longer need a listener, we remove it and then it's available for garbage collection.

If, however, a dedicated event listener is defined (nothing special, there definitely are use cases for that), it isn't necessarily referenced from anywhere outside EventBus. Using a weak reference here would cause the listener to be garbage collected and thus fail to handle incoming events.

Do you see a solution for this? We could introduce some kind of parameter for this, as in EventBus#registerWeakly, but admittedly that is quite verbose.

For that reason, `EventBus#removeListener` exists. If we no longer need a listener, we remove it and then it's available for garbage collection. If, however, a dedicated event listener is defined (nothing special, there definitely are use cases for that), it isn't necessarily referenced from anywhere outside `EventBus`. Using a weak reference here would cause the listener to be garbage collected and thus fail to handle incoming events. Do you see a solution for this? We could introduce some kind of parameter for this, as in `EventBus#registerWeakly`, but admittedly that is quite verbose.

delvh commented

2021-02-17 13:06:30 +01:00

No.
How often have we used removeListener until now? Never.
Do you actually think that will be any different for almost everyone else?
I doubt so. It's much more wanted that objects that should be gced actually get gced.

No. How often have we used removeListener until now? Never. Do you actually think that will be any different for almost everyone else? I doubt so. It's much more wanted that objects that should be gced actually get gced.

delvh commented

2021-02-17 13:16:13 +01:00

WeakReference contains the method get which returns the object or null if it has been gced. By simply checking that we can implement it.

`WeakReference` contains the method get which returns the object or null if it has been gced. By simply checking that we can implement it.

kske commented

2021-02-17 13:22:28 +01:00

This only proves that the need for garbage collection is actually negligibe as most event listeners have the same lifetime as the entire application.

If a listener with a restricted lifetime exists, it can be unregistered, although it's unlikely that this will happen often, as you just mentioned.

But what about dedicated listeners? Most of those would arguably have application lifetime, so garbage collection wouldn't be an issue in the first place. However, with weak references such a listener would get GC'd almost immediately after creation and thus the user would be forced to keep an external reference to it that wouldn't have been necessary otherwise.

This only proves that the need for garbage collection is actually negligibe as most event listeners have the same lifetime as the entire application. If a listener with a restricted lifetime exists, it can be unregistered, although it's unlikely that this will happen often, as you just mentioned. But what about dedicated listeners? Most of those would arguably have application lifetime, so garbage collection wouldn't be an issue in the first place. However, with weak references such a listener would get GC'd almost immediately after creation and thus the user would be forced to keep an external reference to it that wouldn't have been necessary otherwise.

delvh commented

2021-02-17 21:52:24 +01:00

Well that was once.
Because then we used actual EventListeners.
But the average lifespan of any object is lower than that of an actual EventListener.
As we now allow any object, and as we now are trying to open eventbus to more people (maven), I think changing this behavior should be implemented.

Well that was once. Because then we used actual `EventListener`s. But the average lifespan of any object is lower than that of an actual `EventListener`. As we now allow any object, and as we now are trying to open eventbus to more people (maven), I think changing this behavior should be implemented.

kske commented

2021-02-18 08:01:09 +01:00

Alright, so as discussed I will make weak references the default for event listeners, while adding an annotation which enforces a regular reference.

kske added the

core

label 2021-02-20 22:21:27 +01:00

kske changed title from ~~Only listen weakly on objects to avoid memory leaks~~ to Weak References to Event Listeners

2021-02-20 22:25:55 +01:00

kske added the

34

could have

labels 2021-03-14 14:08:34 +01:00