On Mon, 18 Dec 2023 13:18:02 GMT, Johan Vos <j...@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. As for the memory leak issue: there were several. The first commit in this PR fixed a clear memory leak, but the one that is still left is not described in the issue. It occurs because whenever the SystemMenuBar is shown after it was not shown, all MenuItems will be readded using `GlassSystemMenu.setMenu`. This will invoke `GlassSystemMenu.insertMenuItem(... MenuItemBase menuItem...)` Inside this method, platform-specific menuItems will be created, that will adapt when the `MenuItemBase` properties are changing. The annoying thing, as stated in the first comment of that method, is that we don't know if the passed `MenuItemBase` instance was previously used in this method (in which case the listeners are regisered to this instance) or not. That is why for example the `visibilityListener` is unconditionally removed (even if we don't know if it was ever added) and then added. We can do the same for the other properties (preventing the use of weak listeners), but it would be nice if we could address this case using the api introduced in the deterministic listeners management (I would very much prefer that over weak listeners). I'm not too familiar with the new API though, but it seems difficult to do this _inside_ this method, as we would still add a listener each time the method is invoked with an item that already has a listener registered? ------------- PR Comment: https://git.openjdk.org/jfx/pull/1283#issuecomment-1862664763
