On Fri, 22 Nov 2024 17:10:32 GMT, Marius Hanl <mh...@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
>
> Marius Hanl has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8341687: Add more tests

thank you for adding the memory leak test!
the tests fail in the master, and pass with the fixes in this PR.

-------------

Marked as reviewed by angorya (Reviewer).

PR Review: https://git.openjdk.org/jfx/pull/1640#pullrequestreview-2455225720

Reply via email to