RFR: [WIP] 8319779: SystemMenu: memory leak due to listener never being removed [v4]
John Hendrikx
jhendrikx at openjdk.org
Tue Dec 19 10:02:53 UTC 2023
On Mon, 18 Dec 2023 13:18:02 GMT, Johan Vos <jvos at openjdk.org> wrote:
>> A listener was added but never removed.
>> This patch removes the listener when the menu it links to is cleared. Fix for https://bugs.openjdk.org/browse/JDK-8319779
>
> Johan Vos has updated the pull request incrementally with one additional commit since the last revision:
>
> Fix more memoryleaks due to listeners never being unregistered.
Would it be an idea to do deterministic clean-up?
The JIRA tickets involved are light on details, and I don't have a Mac, so I can't test the leak (I think?), but it seems to me that you will want to remove the listeners when the menu bar isn't visible (and re-add them if it becomes visible again?).
>From what I understand, these are normal JavaFX controls wrapped into adapters, and so any listeners installed through the adapter interface will end up on controls. The `GlobalMenuAdapter` seems to be tracking the `showing` property, and calls `show` / `hide` accordingly.
The ticket says:
> when an application loses focus, the SystemMenu is removed. When it gets focus again, the SystemMenu is "back".
My question is, is there any event that can be used to trigger clean-up when this happens? Do the controls go invisible, are they no longer showing (is `hide` called?) There must be something. If there is something, then some kind of disposal can be triggered for deterministic clean-up (ie. add `dispose` method).
If you can go further and distill this information into a boolean property then a pattern like this can be used:
mb.textProperty()
.when(showingProperty) // assumes showing property becomes false when focus is lost...
.addListener(valueModel -> glassMenu.setTitle(parseText(mb)));
The `showingProperty` can be fed from the hide/show event handlers to set it to the right value, perhaps combined with focus information.
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1283#issuecomment-1862456106
More information about the openjfx-dev
mailing list