When a binding is created, a weak listener is added to the source property:
target.bind(source);
The binding is weak to allow `target` to be garbage collected in the event that the binding (from source to target) is the only reference left. If `source` however is a long lived property, this mechanism can leave behind many dead listeners (stubs) in the listener list of `source`.
These stubs are cleaned up currently under two circumstances:
1) When `source` changes value
This will call `changed` or `invalidated` on each listener in the list, including any left over stubs -- these stubs will then remove themselves from the list as part of their `invalidated` / `changed` code as their target is no longer available (note: this leads to concurrent modification of the listener list while it is being iterated).
2) When the listener list of `source` is resized
`ExpressionHelper` sometimes makes copies of a listener list. While it is making the copy, it will remove any listeners for which `wasGarbageCollected` returns `true`.
Normally, the above two safe guards are quite sufficient (although in stress tests you can produce odd results), however, for the recently added Fluent bindings a bit more than just the stub can get left behind. As the stub isn't removed immediately from a chain of mappings, the entire chain remains in memory until the chain is unsubscribed by removing the stub. This then in turn triggers the chain to unsubscribe its chain of source(s) which may include a long lived object. Once the unsubscription completes, the chain is eligible for GC.
See the discussion on https://github.com/openjdk/jfx/pull/675 for more details.
Suggested Fix
-----------------
Use Java `Cleaner`s to pro-actively clean up dead stubs when they're GC'd instead of relying on the two mechanisms described above. The advantage of a `Cleaner` is that no further action is required on the `source` property to trigger the removal of the stub.
Modifying listener lists however is not thread-safe, and it is the user's responsibility to avoid concurrent modification of the listeners for a property. Cleaners run outside the user's control, which means that they should only attempt clean-up when the listener list can be modified safely.
To get the desired effect of cleaning up Fluent bindings, only `LazyObjectBinding` needs to support thread safe changes to its listeners. Cleaners can detect whether an `ObservableValue` supports this by using a marker interface (`ConcurrentListenerAccess` was suggested as a name for this interface).
All classes that create bindings should be updated to create cleaners if this concurrent listener access is allowed (All *PropertyBase classes).
Optionally (to be discussed), we could mark more classes to allow eager clean-up of stubs, not just `LazyObjectBinding`.
target.bind(source);
The binding is weak to allow `target` to be garbage collected in the event that the binding (from source to target) is the only reference left. If `source` however is a long lived property, this mechanism can leave behind many dead listeners (stubs) in the listener list of `source`.
These stubs are cleaned up currently under two circumstances:
1) When `source` changes value
This will call `changed` or `invalidated` on each listener in the list, including any left over stubs -- these stubs will then remove themselves from the list as part of their `invalidated` / `changed` code as their target is no longer available (note: this leads to concurrent modification of the listener list while it is being iterated).
2) When the listener list of `source` is resized
`ExpressionHelper` sometimes makes copies of a listener list. While it is making the copy, it will remove any listeners for which `wasGarbageCollected` returns `true`.
Normally, the above two safe guards are quite sufficient (although in stress tests you can produce odd results), however, for the recently added Fluent bindings a bit more than just the stub can get left behind. As the stub isn't removed immediately from a chain of mappings, the entire chain remains in memory until the chain is unsubscribed by removing the stub. This then in turn triggers the chain to unsubscribe its chain of source(s) which may include a long lived object. Once the unsubscription completes, the chain is eligible for GC.
See the discussion on https://github.com/openjdk/jfx/pull/675 for more details.
Suggested Fix
-----------------
Use Java `Cleaner`s to pro-actively clean up dead stubs when they're GC'd instead of relying on the two mechanisms described above. The advantage of a `Cleaner` is that no further action is required on the `source` property to trigger the removal of the stub.
Modifying listener lists however is not thread-safe, and it is the user's responsibility to avoid concurrent modification of the listeners for a property. Cleaners run outside the user's control, which means that they should only attempt clean-up when the listener list can be modified safely.
To get the desired effect of cleaning up Fluent bindings, only `LazyObjectBinding` needs to support thread safe changes to its listeners. Cleaners can detect whether an `ObservableValue` supports this by using a marker interface (`ConcurrentListenerAccess` was suggested as a name for this interface).
All classes that create bindings should be updated to create cleaners if this concurrent listener access is allowed (All *PropertyBase classes).
Optionally (to be discussed), we could mark more classes to allow eager clean-up of stubs, not just `LazyObjectBinding`.
- relates to
-
JDK-8274771 Map, FlatMap and OrElse fluent bindings for ObservableValue
-
- Resolved
-
- links to
-
Review(master) openjdk/jfx/827