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

[John Hendrikx]

On Wed, 10 Jan 2024 19:55:35 GMT, Johan Vos <jvos at openjdk.org> wrote:

>> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassSystemMenu.java line 98:
>> 
>>> 96:             active.set(false);
>>> 97:         }
>>> 98:         active = new SimpleBooleanProperty(true);
>> 
>> should this be
>> 
>> if(active == null) {
>>   active = new ...
>> } else {
>>  active.set(false);
>> }
>> 
>> instead?
>
> No, the explicit goal of this construction is to set active to false (in case it exists) so that existing listeners can be released; followed by creating a new active property that is by default set to true until `setMenus` is called again (e.g. after unfocus/focus events).
> I noticed however that it is not as simple as that, and the current PR introduces regression, as the `SystemMenuBarTest.testMemoryLeak` test is now failing. Listeners are not only registered when `setMenus` is called, but also when menu(Item)s are added to menus. I'm not sure if a single `active` property can address this, as we don't want to remove all listeners for all menuitems in case a particular menuitem is added to a particular menu.
> @hjohn does that make sense?

@johanvos I don't think what I wrote above (option 1) will work -- there will still be a reference from the `active` property that keeps the menu item alive; a reference is indeed removed when `hasParent` becomes `false`, but there is another (same reason we had to recreate the active property)... I will think on this some more over the weekend.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1448132951