On Thu, 13 Oct 2022 15:52:46 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> John Hendrikx has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains ten commits:
>>
>> - Merge remote-tracking branch 'origin/master' into
>> feature/conditional-bindings
>>
>> # Conflicts:
>> #
>> modules/javafx.base/src/test/java/test/javafx/beans/value/LazyObjectBindingTest.java
>> - Fix review comments
>> - Add missing new line
>> - Small wording fixes
>> - Update javadoc of "when" with better phrasing
>> - Rename showing property to shown as it is already used in subclasses
>> - Add conditional bindings to ObservableValue
>> - Change explanatory comment to block comment
>> - Fix bug where ObjectBinding returns null when revalidated immediately
>>
>> Introduced with the fluent bindings PR
>
> modules/javafx.base/src/main/java/com/sun/javafx/binding/ConditionalBinding.java
> line 19:
>
>> 17:
>> 18: // condition is always observed and never unsubscribed
>> 19: Subscription.subscribe(nonNullCondition, current -> {
>
> do I understand this correctly - if the transient Nodes are created and
> hooked up with ConditionalBinding and then discarded, we'll have a memory
> leak?
I'm not sure I fully understand what you mean here with the transient nodes, so
if you have an example where you think this doesn't work as you'd like, I'd be
happy to take a look at it. However, I'll try explain though what's going on
as best I can:
There are two important things at play here:
1) The bindings resulting from `map`, `when`, etc. are lazy bindings because
they're created with the fluent system. This means that if they're not
observed themselves, they remove their listeners to their source. For example,
a chain of values (with arrows pointing from referrer to referent):
A <-- B <-- C <-- D
Or in Java:
b = a.map(...); c = b.map(...); d = c.map(...);
Here B observes A, C observes B and D observes C. These will only create
listeners on their source when they are observed themselves. In their
unobserved state, D can be GC'd, then C, then B, then A. When the listeners are
present, the chain looks like:
A <-> B <-> C <-> D <----> something is observing D
Now nothing can be GC'd as long as whatever is observing D is strongly
referenced. However, as soon as the observer on D disappears, the chain reverts
back to single directional pointers because all listeners are removed when
unobserved.
2) The condition for `when` acts as an additional requirement for an observable
value to create a listener on its source. If it is `false`, no listener on its
source is created, even when observed itself. If it is `true`, then a listener
is only created on its source if it is observed itself, otherwise still no
listener is created.
This means when you have:
ObservableValue<Boolean> active = new SimpleBooleanProperty(true);
And then use it as the condition for another property:
ObservableValue<String> x = label.textProperty().when(active);
x.addListener(xyz);
Then the `ObservableValue` that results from the call to `when` ("x") will
always be subscribed to the `active` value. That means as long as `active` is
referred, that `x` cannot be GC'd -- this is what you want, as when I toggle
`active` back and forth, the thing we're toggling should not disappear.
In this case, there is a listener AND the condition is `true`, so label's text
property is referencing `x` as `x` will have added a listener to label's text
property.
label.text <-> x <-----> listener
x <-> active
If the condition is `false`, the graph instead looks like:
label.text <-- x <-----> listener
x <-> active
This is a very subtle difference, but it does mean that the `label` no longer
has any influence on keeping these objects alive. If only `label` is strongly
referenced, then toggling `active` to `false`, and then purposely removing all
references to `active` would mean the system can freely remove all these
observables, including the listener as there is no way you could turn the
switch back on again...
If you however kept a reference to one of these (either `listener`, `x` or
`active`) then they all are referenced, as they should be, as I could toggle
`active` back to `true` and expect the whole chain to work again as before.
The fact that the lifecycle of the condition and its resulting observable value
are tied to each other should not matter, and in fact, has to work that way or
switching on/off wouldn't do what you'd expect anymore if the properties being
switched could be GC'd.
In an earlier example I showed this:
ObservableValue<Boolean> active = new SimpleBooleanProperty(true);
MySkin() {
getSkinnable().textProperty().when(active).addListener(this::updateSkinStuffs);
getSkinnable().fontProperty().when(active).addListener(this::updateSkinStuffs);
getSkinnable().wrapTextProperty().when(active).addListener(this::updateSkinStuffs);
}
void dispose() {
active.set(false); // kills all listeners/bindings/etc in a single
line
}
Assuming the Skin itself is not strongly referenced at some point, the
listeners in the above example are all still referenced by the control
resulting from `getSkinnable` and can't be GC'd. Skinnable refers to its text
property, the text property refers to the `when` observable, the `when`
observable refers to the `updateSkinStuffs` method, and that method pointer
(but also the `when` observable which refers to `active`) keeps the entire Skin
and all its listeners alive.
However, if you set `active` to false, that chain is broken. Specifically, the
text property no longer references the `when` observable, because the `when`
observable will unsubscribe itself. Since there are no other references to the
skin anymore, the entire skin, including the `active` observable boolean is
GC'd. The fact that `when` and `active` reference each other makes no
difference.
-------------
PR: https://git.openjdk.org/jfx/pull/830
