RFR: 8277848 Binding and Unbinding to List leads to memory leak [v2]

[Florian Kirmaier]

On Fri, 31 Dec 2021 12:53:54 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> Florian Kirmaier has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   JDK-8277848
>>   Added missing change
>
> Why are the new listener imlementations called `BaseChangeListener` and `BaseInvalidationListener`, i.e. why the _Base_?
> 
> Also, if you're going to the trouble of refactoring the existing listener implementation, have you considered merging the very similar implementations into a single class? You can then re-use the listener instance and save another object allocation in this way:
> 
> 
> private static class Listener<E> extends WeakReference<ListPropertyBase<E>>
>         implements InvalidationListener, ListChangeListener<E>, WeakListener {
>     Listener(ListPropertyBase<E> ref) {
>         super(ref);
>     }
> 
>     @Override
>     public boolean wasGarbageCollected() {
>         return get() == null;
>     }
> 
>     @Override
>     public void onChanged(Change<? extends E> change) {
>         ListPropertyBase<E> ref = get();
>         if(ref != null) {
>             ref.invalidateProperties();
>             ref.invalidated();
>             ref.fireValueChangedEvent(change);
>         }
>     }
> 
>     @Override
>     public void invalidated(Observable observable) {
>         ListPropertyBase<E> ref = get();
>         if (ref == null) {
>             observable.removeListener(this);
>         } else {
>             ref.markInvalid(ref.value);
>         }
>     }
> }

@mstr2 
Great point. The chosen name was just because I needed a name. I switched now to the name "Listener".

I've now merged the ChangeListener with the Invalidation Listener, as you've suggested.
The PR should now improve the performance while fixing a bug. It might even be quite relevant because these classes are used very often.

-------------

PR: https://git.openjdk.java.net/jfx/pull/689