RFR: 8241737: TabPaneSkin memory leak on replacing selectionModel

Jeanette Winzenburg fastegal at openjdk.java.net
Mon Apr 13 16:04:18 UTC 2020

On Mon, 13 Apr 2020 08:03:09 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:

> `TabPaneSkin` adds a listener to `SelectionModel.selectedItemProperty()` which holds the `SelectionModel` from being
> GCed. Fix is to add and remove the listener when a `SelectionModel` is changed.
> If the fix looks good, We can change all the `getSkinnable().getSelectionModel()` calls to use the new class member
> `selectionModel`.
> The fix seems safe to cause any regression. Enabled an ignored test that was added as part of fix for
> [JDK-8241455](https://bugs.openjdk.java.net/browse/JDK-8241455).

Changes requested by fastegal (Author).

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TabPaneSkin.java line 270:

> 269:         }
> 270:
> 271:         super.dispose();

wondering if the removal is really needed? The memory leak seems to be fixed without explicit removing. Did you
experience any side-effects if not?

If so, a unit test covering that would be cool. There are other (mostly) skins that have a weak "path listener"  which
is not removed on dispose, they need a re-visit to see if the side-effect applies to them as well. Or the other way
round: without a detectable side-effect, not doing it might be okay.

Whatever the outcome, this looks like a pattern to remember when skins are listening weakly :)


PR: https://git.openjdk.java.net/jfx/pull/175

More information about the openjfx-dev mailing list