RFR: 8349756: Memory leak in PaginationSkin when setting page count / index
Kevin Rushforth
kcr at openjdk.org
Tue Feb 11 22:14:16 UTC 2025
On Tue, 11 Feb 2025 15:46:17 GMT, Andy Goryachev <angorya at openjdk.org> wrote:
> ## Root Cause
>
> Each time a `PaginationSkin.IndicatorButton` gets created it adds two listeners (to the control's `styleClass` property and to the skin's tooltipVisible property) via `ListenerHelper`. This was not detected by the `SkinMemoryLeakTest` because all listeners got removed correctly upon skin change.
>
> ## Solution
>
> Add a single listener to the control's `styleClass` property at the skin level, and insert the call to the skin's `tooltipVisible` property both of which iterate over indicator button to do their thing.
The fix looks good to me. I tested it in a couple of different ways, and everything works as expected.
With this fix and the fix for PR #1698 with the call to `System.gc()` removed, the test passes with no OOM.
modules/javafx.controls/src/main/java/javafx/scene/control/skin/PaginationSkin.java line 201:
> 199:
> 200: navigation = new NavigationControl();
> 201: navigation.initializePageIndicators();
Minor: would it make sense to move this back to the`NavigationControl` constructor (at the end)?
-------------
Marked as reviewed by kcr (Lead).
PR Review: https://git.openjdk.org/jfx/pull/1705#pullrequestreview-2610114615
PR Review Comment: https://git.openjdk.org/jfx/pull/1705#discussion_r1951646931
More information about the openjfx-dev
mailing list