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

Johan Vos jvos at openjdk.org
Sat Jan 20 20:37:37 UTC 2024


On Tue, 16 Jan 2024 10:16:08 GMT, Florian Kirmaier <fkirmaier at openjdk.org> wrote:

>> 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.

@FlorianKirmaier Thanks for the additional test. There was already a test in the SystemMenuBarTest, but it's really good to have an additional test for this -- I added it.

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

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


More information about the openjfx-dev mailing list