RFR: 8307538: Memory leak in TreeTableView when calling refresh [v7]

[Andy Goryachev]

On Tue, 6 Jun 2023 09:36:37 GMT, Marius Hanl <mhanl at openjdk.org> wrote:

>> Andy Goryachev 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 17 additional commits since the last revision:
>> 
>>  - Merge remote-tracking branch 'origin/master' into 8307538.refresh
>>  - review comments
>>  - John is right
>>  - review comments
>>  - one more test case
>>  - back to register listener
>>  - weak lambdas are getting collected
>>  - removed monkey tester changes
>>  - Merge remote-tracking branch 'origin/master' into 8307538.refresh
>>  - whitespace
>>  - ... and 7 more: https://git.openjdk.org/jfx/compare/c419be41...f68de127
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java line 117:
> 
>> 115:         TreeTableView<T> treeTableView = getSkinnable().getTreeTableView();
>> 116:         if (treeTableView == null) {
>> 117:             registerInvalidationListener(getSkinnable().treeTableViewProperty(), (x) -> {
> 
> One question here: Why does this prevent the leak but the ListenerHelper does not?

The difference is that registerInvalidationListener() adds a weak listener, while ListenerHelper adds a strong listener.

It is possible to use ListenerHelper here, at the expense of more complicated code since we'd need to explicitly disconnect the listener when tableViewProperty value gets set.  

Another solution would involve adding a method to add a weak listener to the ListenerHelper to avoid explicit cleanup, or

Go back to the original code which used register/unregister*Listener **in this particular case**.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1129#discussion_r1219912085