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

[Michael Strauß]

On Tue, 7 Dec 2021 15:21:56 GMT, Florian Kirmaier <fkirmaier at openjdk.org> wrote:

>> Making the initial listener of the ListProperty weak fixes the problem.
>> The same is fixed for Set and Map.
>> Due to a smart implementation, this is done without any performance drawback.
>> (The trick is to have an object, which is both the WeakReference and the Changelistener)
>> By implying the same trick to the InvalidationListener, this should even improve the performance of the collection properties.
>
> 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);
        }
    }
}

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

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