On Thu, 13 Oct 2022 22:17:09 GMT, Andy Goryachev <[email protected]> wrote:
> Also, using your example above, it might be too easy to create a memory leak
> by inadvertently referring to a long lived object:
>
> ```
> getSkinnable().textProperty().when(active).addListener((src,prev,current) -> {
> getSkinnable().setSomething(window.getSomething() + "..."); // window
> reference makes the listener lambda uncollectable, resulting in a memory leak
> });
> ```
I'm not sure I follow your meaning. Why would referring to `window` make the
lambda uncollectable? The lambda is referring to `window`, not the other way
around.
The current "alternative", which is weak listeners, is not what it is cracked
up to be. A weak listener that is not strongly referenced will be collected at
a random time, including never, (almost) immediately or in 5 minutes. All that
time, the listener still functions, responds to potential events and property
changes, even though the reason for its existence has since disappeared.
When a weak reference is collected depends on many things, including the chosen
GC, its parameters, JDK versions, memory pressure, etc. This proposal is
specifically tailored to make the process of removing listeners deterministic
without having to manually unregister listeners. Weak listeners are completely
unpredictable and could potentially live much longer than expected on certain
GC's (or future improvements of GC's) that have different strategies for
cleaning weak references.
A weak reference specifically is NOT something that gets cleaned up immediately
upon the last reference disappearing. This can cause all kinds of phantom
effects in JavaFX applications, if your listener does something like persisting
state or printing something to the console. For example, a long lived model
gets a weak listener attached to it when a dialog opens; the listener persists
the last selected item. The dialog is closed and reopened. Now there might be
two listeners, both persisting the last selected item. This may show up on your
console as two database calls where one is expected.
You can argue this is bad programming and that those listeners should have been
cleaned up specifically with a dispose method, and I agree, you get predictable
behavior that way. And that's exactly what this proposal also brings,
predictable behavior, using a syntax that doesn't require you to keep track of
your listeners explicitely:
longLivedModel.indexProperty().when(dialog::shownProperty).addListener(persistPosition);
This detaches the listener immediately after the dialog is closed. Not in 3
seconds or 5 minutes, but as soon as it is no longer visible. Opening a new
dialog will not reinstate this listener. Reusing the same dialog however would,
and it would start working again exactly as you'd expect as soon as the dialog
is visible again.
-------------
PR: https://git.openjdk.org/jfx/pull/830
