Another investigation into dynamic bind/unbinding of dependencies in Bindings led me to a discovery of a potentially nasty bug in the low level Binding classes in JavaFX.
The "unbind" method in for example IntegerBinding will assume that all bindings are unbound and clear an internal helper "BindingHelperObserver", resulting in any further "unbind" calls to become no-ops.
The result is that properties that have been unbound can still invalidate the binding.
Test case:
@Test
public void test() {
IntegerProperty p1 = new SimpleIntegerProperty(10);
IntegerProperty p2 = new SimpleIntegerProperty(20);
IntegerProperty p3 = new SimpleIntegerProperty(40);
IntegerBinding binding = new IntegerBinding() {
{
bind(p1, p2, p3);
unbind(p2); // clears BindingHelperObserver inside IntegerBinding
unbind(p1); // does nothing, so the listener is still present
}
@Override
protected int computeValue() {
return p3.get();
}
};
assertTrue(binding.isValid());
p1.set(2); // unrelated change
assertTrue(binding.isValid()); // Fails !!!!!
}
Potential Solution:
The most trivial fix is to never clear the BindingHelperObserver once it has been created.
A more involved fix would be to track the bindings made and clear the BindingHelperObserver once all bindings are removed. This would also allow for default implementations of "getDependencies" and "dispose" methods (but those would be functional changes).
For this fix, a Binding would keep references to its dependencies (which is already the case as otherwise computeValue could not be implemented.) The binding would still be eligible for garbage collection if nothing else referred to it anymore.
The "unbind" method in for example IntegerBinding will assume that all bindings are unbound and clear an internal helper "BindingHelperObserver", resulting in any further "unbind" calls to become no-ops.
The result is that properties that have been unbound can still invalidate the binding.
Test case:
@Test
public void test() {
IntegerProperty p1 = new SimpleIntegerProperty(10);
IntegerProperty p2 = new SimpleIntegerProperty(20);
IntegerProperty p3 = new SimpleIntegerProperty(40);
IntegerBinding binding = new IntegerBinding() {
{
bind(p1, p2, p3);
unbind(p2); // clears BindingHelperObserver inside IntegerBinding
unbind(p1); // does nothing, so the listener is still present
}
@Override
protected int computeValue() {
return p3.get();
}
};
assertTrue(binding.isValid());
p1.set(2); // unrelated change
assertTrue(binding.isValid()); // Fails !!!!!
}
Potential Solution:
The most trivial fix is to never clear the BindingHelperObserver once it has been created.
A more involved fix would be to track the bindings made and clear the BindingHelperObserver once all bindings are removed. This would also allow for default implementations of "getDependencies" and "dispose" methods (but those would be functional changes).
For this fix, a Binding would keep references to its dependencies (which is already the case as otherwise computeValue could not be implemented.) The binding would still be eligible for garbage collection if nothing else referred to it anymore.