RFR: 8290040: Provide simplified deterministic way to manage listeners [v6]

John Hendrikx jhendrikx at openjdk.org
Thu Oct 13 21:05:44 UTC 2022


On Thu, 13 Oct 2022 20:42:31 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> 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.

Since this was quite wordy, and since pictures are worth a thousand words...

Here is the situation when you have a Label with its Skin:

![image](https://user-images.githubusercontent.com/995917/195709660-0bdad680-9240-48e9-9f8e-ae4b40ab7047.png)

When that Skin is replaced, dispose is called which toggles `active` to `false`.  Furthermore, the Label will no longer reference the Skin.  This image then shows the state of the references, and which objects can be GC'd:

![image](https://user-images.githubusercontent.com/995917/195710017-413af0ec-1c0f-4329-adb5-d5c3072c0008.png)

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

PR: https://git.openjdk.org/jfx/pull/830


More information about the openjfx-dev mailing list