RFR: 8341687: Memory leak in TableView after interacting with TableMenuButton
Marius Hanl
mhanl at openjdk.org
Mon Nov 18 20:17:55 UTC 2024
On Mon, 18 Nov 2024 16:19:20 GMT, Andy Goryachev <angorya at openjdk.org> wrote:
>> There are multiple issues in the `TableMenu` logic that result in a memory leak.
>>
>> The following problems are now fixed with this PR:
>> - The listener, that is registered to the `col.visibleProperty()` was not weak nor was it ever unregistered
>> - The fix is to do pretty much the same that was already done with the `col.textProperty()` listener
>> - The `col.visibleProperty()` and `col.textProperty()` where added again and again and again to the column when the columns changed (which happens when toggling the visibility).
>> - The fix is to add the listeners once - when the `MenuItem` is created. This way, when the `TableMenu` is rebuild and the cached `MenuItem` is used, the whole listener logic is not run again for the column
>
> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableViewTableHeaderRowTest.java line 261:
>
>> 259: /**
>> 260: * Tests that toggling the column visibility does not cause memory leaks.
>> 261: * See also: <a href="https://bugs.openjdk.org/browse/JDK-8341687">JDK-8341687</a>.
>
> same comment - does not test for memory leak. the memory leak test might involve `JMemoryBuddy` - see for example `SystemMenuBarTest`.
I tried, but it isn't that easy - since the memory is not leaked because we keep some references we should not keep - rather we are adding listener again and again and again. So we can not assert things to be collectable, as they should not be, still they increase the memory footprint instead.
So I did not manage to make a test with `JMemoryBuddy`, but can prove the point with the amount of listener instead.
I wrote a test that checked, that the memory is similar high as before, that worked as well but I was afraid that is might be flaky, which I want to avoid.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1640#discussion_r1847209388
More information about the openjfx-dev
mailing list