On Wed, 6 Jul 2022 22:24:36 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> John Hendrikx has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 27 additional >> commits since the last revision: >> >> - Merge branch 'openjdk:master' into feature/fluent-bindings >> - Add null checks in Subscription >> - Update copyrights >> - Move private binding classes to com.sun.javafx.binding package >> - Add note to Bindings#select to consider ObservableValue#flatMap >> - Fix bug invalidation bug in FlatMappedBinding >> >> Also fixed a secondary issue where the indirect source of the binding >> was unsubscribed and resubscribed each time its value was recomputed. >> >> Add additional comments to clarify how FlatMappedBinding works. >> >> Added test cases for these newly discovered issues. >> - Fix typos in LazyObjectBinding >> - Rename observeInputs to observeSources >> - Expand flatMap javadoc with additional wording from Optional#flatMap >> - Add since tags to all new API >> - ... and 17 more: https://git.openjdk.org/jfx/compare/a2456300...d66f2ba6 > >> Let's say we introduce a marker interface that represents a thread-safety >> guarantee with regards to `addListener` and `removeListener`: >> >> ```java >> interface ConcurrentListenerAccess {} >> ``` >> >> Now we can use the existing `Disposer` mechanism to automatically schedule >> listeners to be removed from `ConcurrentListenerAccess` implementations when >> the bound property is eligible for collection. >> >> It's sufficient to implement `ConcurrentListenerAccess` for >> `LazyObjectBinding` to make `map` and `flatMap` work the way I'd expect it >> to work. > > You mean by `Disposer` something like `com.sun.prism.impl.Disposer`? I see > that it has a `ReferenceQueue` in it, so I'm guessing that's the mechanism > you are referring to. How would you want this triggered? I think a thread > blocking on the queue might be best. > > I think this is a very nice idea to potentially tackle this problem. > @hjohn Thanks for your detailed analysis of the bindings GC situation. The > conclusion I take from all of this is that the current implementation is > fine, but we might consider a follow-up issue to clean up dead listener stubs. >From @mstr2 analysis, it is slightly more than just the stub in the case of >the fluent bindings. See illustration below, assuming `caption` is the only >hard reference, the dashed boxes represent things that are eligible for GC, >dashed lines represent weak references. If `caption` is modified, triggering >a clean-up, then everything is eligible for GC aside from `caption`:  It might be good to fix this right away. ------------- PR: https://git.openjdk.org/jfx/pull/675