RFR: 8274771: Map, FlatMap and OrElse fluent bindings for ObservableValue [v15]
John Hendrikx
jhendrikx at openjdk.org
Thu Jun 30 04:31:48 UTC 2022
On Wed, 29 Jun 2022 23:39:14 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:
> This test fails for `assertCollectable(oldValueHolder)`.
>
> My assumption is that a mapped binding holds no strong references to objects that are no longer in the mapping chain (i.e. property values that have been replaced by other values).
>
> Is this a bug, or is this expected behavior?
This is indeed a bug, and quite a subtle one, so definitely a good catch!
The issue is that the flat mapped binding only switches its subscriptions during the computing of its value. However, since this test avoids querying the actual value, this never happens (no `getValue` is called on `mapped` and the listener involved is only an `InvalidationListener`).
Adding `mapped.getValue()` just before the asserts or using a `ChangeListener` makes the issue disappear.
The solution is for `FlatMappedBinding` to respond to invalidations by unsubscribing the mapped value it was observing; it normally does this in `computeValue` but it must do this a bit more eagerly.
My initial quick fix is to do this in `FlatMappedBinding`:
@Override
protected void onInvalidating() {
mappedSubscription.unsubscribe();
mappedSubscription = Subscription.EMPTY;
}
I'm seeing if this is the correct solution and adding some extra documentation to the class to more clearly explain what it should be doing. I will also add a test case to `ObservableValueFluentBindingsTest` to catch this one specifically.
> By the way, if I move `ObservableValue<String> mapped = ...` before `var oldValueHolder = ...`, the test passes.
I couldn't fix the issue with your suggestion of moving the `ObservableValue<String> mapped = ...` line -- perhaps you made some other changes as well that influenced the result. I suspect you also moved the `mapped.addListener(observable -> {});` line to the same location.
-------------
PR: https://git.openjdk.org/jfx/pull/675
More information about the openjfx-dev
mailing list