RFR: 8319779: SystemMenu: memory leak due to listener never being removed [v8]

Florian Kirmaier fkirmaier at openjdk.org
Tue Jan 16 10:18:37 UTC 2024


On Thu, 11 Jan 2024 14:04:36 GMT, Johan Vos <jvos at openjdk.org> wrote:

>> Johan Vos has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Revert some of the conditional bindings.
>>   Clear menu construction when an menuitem that is a menu needs to be removed
>>   Add a test for the latter
>
> A few things about the latest commit:
> 
> 1. The usage of the `active` property caused regression: the memoryLeak test that was introduced in the fix for JDK-8318841 now failed. A number of listeners were hard referenced from the `active` property.
> The complexity is that there are different entry points by which a listener should be removed. What we try to do in this PR, is making sure we remove listeners after a defocus/focus operation that would otherwise stay referenced. A focus operation will result in GlassSystemMenu.setMenus() being called. 
> I reverted some of the dependencies on the `active` property, and kept those that were directly created as a consequence of the setMenus call.
> 
> 2. I added a test that demonstrates the issue when not removing menu's completely inside the listener, as reported by @jperedadnr and this issue is fixed as well now.
> 
> 3. All tests now pass, but I noticed that in some cases, the systemtests do not correctly work with the application lifecycle management (see https://mail.openjdk.org/pipermail/openjfx-dev/2024-January/044516.html). For now, I consider this anomaly to be independent from JDK-8319779

@johanvos 
**ticket**
We found a bug, which is also fixed by this PR. This is the ticket: https://bugs.openjdk.org/browse/JDK-8323787
It's an IndexOutOfBounds exception, related to the visible flag/listeners.

**unittest**
We wrote a unit test, which goes green with this PR.
Can you do us a favor, and add the test from the ticket to your PR?
Then we can be sure, that this issue will be solved and never reappears.

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

PR Comment: https://git.openjdk.org/jfx/pull/1283#issuecomment-1893444554


More information about the openjfx-dev mailing list