RFR: JDK-8304439: Subscription based listeners [v7]

[John Hendrikx]

On Sun, 9 Jul 2023 18:50:34 GMT, Jose Pereda <jpereda at openjdk.org> wrote:

>> John Hendrikx has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add newline at end of ConditionalBinding file
>
> modules/javafx.base/src/main/java/javafx/beans/Observable.java line 110:
> 
>> 108:     default Subscription subscribe(Runnable subscriber) {
>> 109:         Objects.requireNonNull(subscriber, "subscriber cannot be null");
>> 110:         InvalidationListener listener = obs -> subscriber.run();
> 
> In the `subscribe` implementations, Invalidation/ChangeListeners are added as local variables. Have you run any test to check that they don't end up being gc'ed, because no strong references being held?
> 
> And also related to this, how would you add a WeakInvalidation/WeakChangeListener? Maybe, that's up to the developer, overriding the default implementation of `subscribe`?

There is no need, the returned `Subscription` is implemented using a lambda that captures the listener (as it needs it to call `removeListener`).  As long as you have the `Subscription` referenced, it can't get GC'd.  If you let the `Subscription` get GC'd, then you'd lose your means to unsubscribe the listener, just like if you lost the reference to the listener itself, you would never be able to remove it anymore.

Weak listeners I would not recommend to use in new code if you can avoid it.  They're unpredictable as to when they're removed exactly, and can still receive notifications when your application may be in a state where you don't expect them anymore.

The `Subscription` interface is in fact intended to make management of listeners easier so you can rely less on weak listeners.  One pattern I can really recommend to use is to automatically disable listeners when the UI they belong with becomes invisible, for example:

    private Subscription subscription = Subscription.EMPTY;

    node.sceneProperty()
      .flatMap(Scene::windowProperty)
      .flatMap(Window::showingProperty)
      .subscribe(visible -> {
            if (visible) {
                  subscription = Subscription.combine(
                       // create subscriptions (to external resources) here as we just became visible!
                  );
            }
            else {
                  // we became invisible, remove listeners (from external resources) to remove hard references that 
                  // may keep the UI referenced; the other branch will restore them if the UI becomes visible again
                  subscription.unsubscribe();  // unsubscribes everything in one go
            }
      });
      
Or you can use `ObservableValue#when` to do something similar, not requiring subscriptions.

I suspect it may also prove a useful pattern for `Skin`s that have a specific lifecycle.  They can register listeners (and combine in a single subscription) in their constructor / initializer, and remove them all in their `dispose` method.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1069#discussion_r1257540910