RFR: 8242621: TabPane: Memory leak when switching skin
Kevin Rushforth
kcr at openjdk.java.net
Thu Oct 15 23:32:15 UTC 2020
On Tue, 13 Oct 2020 12:56:10 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:
> `TabPaneSkin` installs some listeners that are not removed when `TabPaneSkin` is changed.
> The fix converts listeners to WeakListeners and also removes them on dispose.
>
> There is a NPE check change needed in `isHosrizontal()`. Without this check it causes NPE if pulse is in progress while
> TabPaneSkin is getting disposed.
> `SkinMemoryLeakTest` already had a test which only needed to be enabled.
> Test fails before and passes after this change.
modules/javafx.controls/src/main/java/javafx/scene/control/skin/TabPaneSkin.java line 274:
> 272: getSkinnable().getTabs().removeListener(weakTabsListener);
> 273:
> 274: getChildren().remove(tabHeaderArea);
As mentioned offline, can you check whether removing `tabHeaderArea` is necessary?
modules/javafx.controls/src/main/java/javafx/scene/control/skin/TabPaneSkin.java line 716:
> 714:
> 715: private boolean isHorizontal() {
> 716: Side tabPosition = getSkinnable() != null ? getSkinnable().getSide() : Side.TOP;
I agree with @kleopatra This null check suggests that someone (a listener perhaps?) is calling into the skin after it
has been disposed.
modules/javafx.controls/src/main/java/javafx/scene/control/skin/TabPaneSkin.java line 1226:
> 1224:
> 1225: void removeListeners() {
> 1226: for(Node child : headersRegion.getChildren()) {
Minor: space after the `for`
-------------
PR: https://git.openjdk.java.net/jfx/pull/318
More information about the openjfx-dev
mailing list