On Tue, 6 Jun 2023 09:36:37 GMT, Marius Hanl <mh...@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
