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

Johan Vos jvos at openjdk.org
Mon Nov 20 08:00:59 UTC 2023


On Fri, 10 Nov 2023 12:37:13 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> Johan Vos has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   process reviewers comments
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassSystemMenu.java line 63:
> 
>> 61:     private MenuBar             glassSystemMenuBar = null;
>> 62:     private final Map <Menu, ListChangeListener> menuListeners = new HashMap<>();
>> 63:     private final Map <ListChangeListener, ObservableList> listenerItems  = new HashMap<>();
> 
> minor:
> Suggestion:
> 
>     private final Map<Menu, ListChangeListener> menuListeners = new HashMap<>();
>     private final Map<ListChangeListener, ObservableList> listenerItems = new HashMap<>();

done

> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassSystemMenu.java line 189:
> 
>> 187: 
>> 188:     private ListChangeListener createListener(final Menu glassMenu) {
>> 189:         ListChangeListener<MenuItemBase> answer = ((ListChangeListener.Change<? extends MenuItemBase> change) -> {
> 
> minor: I see extra parenthesis around the whole, and I think you can just `return` this immediately (no need for `answer` local)

I typically add a local variable for this (which is removed by javac) to make it easier to add debugging, but I'll remove it.

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

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


More information about the openjfx-dev mailing list