JavaBeanProperties fixed DescriptorListenerCleaner still causes memory leaks

The solution presented in RT-36087 still causes a memory leak, if a strong reference from the java bean to the JavaBeanProperty exists.

Assume a situation where we want to bind a PropertyChangeSupport supporting bean to some other Property.
The steps one would take are
* Create a JavaBeanProperty for the desired bean property
* Bind it using Bindings.bindBidirectional
* (Drop the reference to the JavaBeanProperty)

Unfortunately this would cause an issue, as the JavaBeanProperty is only weakly referenced from the binding created with Bindings.bindBidirectional, so eventually the garbage collector will break the binding.

To avoid this a strong reference to the JavaBeanProperty needs to be held someplace. This is where I don't see any other scenario than that we want our JavaBeanProperty lifecycle to be the same as the beans ones.
To achieve this one would probably
* use a WeakHashMap<Bean, Property>
* add a dummy propertyChangeSupport listener to the bean which only holds a reference to the property
* or simply add a field to the bean to store the reference to the property (As shown in the example).

Now we have created the situation which causes the memory leak.

The reason for the memory leak is the following:

The JavaBeanProperty creates a Cleaner with a strong reference to the ReadOnlyListener (which is the PropertyChangeListener added to the bean), which itself has a strong reference to the bean.
The Cleaner is statically referenced until it ran, so the GC won't remove the Cleaner before it ran.
The Cleaner strongly references the bean via the ReadOnyListener reference, so the bean can not be collected until the Cleaner ran.
The Cleaner won't run before the JavaBeanProperty is collected.
The bean strongly references the JavaBeanProperty, so the Cleaner will never run.

This problem is easily solved by changing the strong reference of the ReadOnlyListener to a weak reference. Weakly referencing the ReadOnlyListener doesn't demonstrate a problem for the Cleaner because the ReadOnlyListener is strongly referenced by the bean, therefore the ReadOnlyListener exists as long as the bean does.
Three scenarios are thinkable of:
* The reference to the JavaBeanProperty is dropped: The ReadOnlyListener is still referenced by the bean, therefore the Cleaner can still access it via the WeakReference and unregister it from the PropertyChangeSupport. The JavaBeanProperty is successfully removed.
* The reference to the bean is dropped: The bean is garbage collected, as the Cleaner doesn't hold a strong reference to it anymore. As the bean's reference to the JavaBeanProperty is gone the Cleaner will run, notice that the ReadOnlyListener is also garbage collected and do nothing (as there isn't anything to unbind anyways)
* Both are dereferenced. Same as the second scenario.

Added is a patch to fix this problem in the DescriptorListenerCleaner and a code example to reproduce the problem.


=== PATCH ===

diff -r 9e26a5eaed82 modules/base/src/main/java/javafx/beans/property/adapter/DescriptorListenerCleaner.java
--- a/modules/base/src/main/java/javafx/beans/property/adapter/DescriptorListenerCleaner.java Sat Jul 12 16:03:15 2014 +1200
+++ b/modules/base/src/main/java/javafx/beans/property/adapter/DescriptorListenerCleaner.java Mon Jul 14 15:49:10 2014 +0200
@@ -24,21 +24,25 @@
  */
 package javafx.beans.property.adapter;

+import java.lang.ref.WeakReference;
+
 import com.sun.javafx.property.adapter.ReadOnlyPropertyDescriptor;

 class DescriptorListenerCleaner implements Runnable{

     private final ReadOnlyPropertyDescriptor pd;
- private final ReadOnlyPropertyDescriptor.ReadOnlyListener<?> l;
+ private final WeakReference<ReadOnlyPropertyDescriptor.ReadOnlyListener<?>> lRef;

     DescriptorListenerCleaner(ReadOnlyPropertyDescriptor pd, ReadOnlyPropertyDescriptor.ReadOnlyListener<?> l) {
         this.pd = pd;
- this.l = l;
-
+ this.lRef = new WeakReference<ReadOnlyPropertyDescriptor.ReadOnlyListener<?>>(l);
     }

     @Override
     public void run() {
- pd.removeListener(l);
+ ReadOnlyPropertyDescriptor.ReadOnlyListener<?> l = lRef.get();
+ if (l != null) {
+ pd.removeListener(l);
+ }
     }
 }



=== EXAMPLE ===


import java.beans.PropertyChangeListener;
import java.beans.PropertyChangeSupport;
import java.util.Map;
import java.util.WeakHashMap;

import javafx.beans.property.Property;

import org.junit.Test;

import com.eon.pegasus.plus.client.commons.ui.binding.adapter.JavaBeanObjectProperty;
import com.eon.pegasus.plus.client.commons.ui.binding.adapter.JavaBeanObjectPropertyBuilder;

public class BindTest {

public class FaceBean {
private int mouthWidth = 90;
private final PropertyChangeSupport mPcs = new PropertyChangeSupport(this);
private Property<Integer> property;

public int getMouthWidth() {
return mouthWidth;
}

public void setMouthWidth(final int mw) {
int oldMouthWidth = mouthWidth;
mouthWidth = mw;
mPcs.firePropertyChange("mouthWidth", oldMouthWidth, mw);
}

public Property<Integer> getProperty() {
return property;
}

public void setProperty(final Property<Integer> property) {
this.property = property;
}

public void addPropertyChangeListener(final PropertyChangeListener listener) {
mPcs.addPropertyChangeListener(listener);
}

public void removePropertyChangeListener(final PropertyChangeListener listener) {
mPcs.removePropertyChangeListener(listener);
}
}

@Test
public void produceMemoryLeak() throws NoSuchMethodException {

Map<FaceBean, Void> map = new WeakHashMap<>();

int created = 0;

while (true) {
FaceBean faceBean = new FaceBean();
map.put(faceBean, null);
created++;
JavaBeanObjectPropertyBuilder<?> builder = new JavaBeanObjectPropertyBuilder<>();
JavaBeanObjectProperty<Integer> mouthWidthProperty = builder.bean(faceBean).name("mouthWidth").build();
faceBean.setProperty(mouthWidthProperty);

if (created % 1000 == 0) {
System.gc();
System.out.println(String.format("Created %d, in map %d", created, map.size()));
}
}
}

}