RFR: 8299423: JavaFX Mac system menubar leaks [v2]

[Kevin Rushforth]

On Mon, 2 Jan 2023 09:26:14 GMT, Florian Kirmaier <fkirmaier at openjdk.org> wrote:

>> This PR fixes the leak in the mac system menu bar.
>> 
>> Inside the native code, NewGlobalRef is called for the callable.
>> Which makes it into a "GC-Root" until DeleteGlobalRef is called.
>> 
>> The DeleteGlobalRef is never called for the MenuEntry, if it's removed from the menu without removing it's callable.
>> This PR adds logic, whether the Menu is inserted. If it's not inserted in a Menu anymore, then DeleteGlobalRef is called, by calling `_setCallback` with the callable "null".
>> 
>> The unit test verifies, that this bug happened without this change, but no longer happens with this change.
>
> Florian Kirmaier has updated the pull request incrementally with one additional commit since the last revision:
> 
>   JDK-8299423
>   Simplified the fix for the mac-menu-bar based on code review

> This PR adds logic, whether the Menu is inserted. If it's not inserted in a Menu anymore, then DeleteGlobalRef is called, by calling _setCallback with the callable "null".

That would be one approach, but it's not what this PR currently does. Instead, I see that you changed the JNI code to use weak global references. Unless there is code on the Java side that holds a strong reference, using a weak reference for the callback can cause the opposite problem of premature GC, such that the callback stops working in some cases. How certain are you that this can't happen here?

modules/javafx.graphics/src/main/java/com/sun/glass/ui/mac/MacMenuDelegate.java line 66:

> 64:                                             boolean enabled, boolean checked) {
> 65:         ptr = _createMenuItem(title, (char)shortcutKey, shortcutModifiers,
> 66:                 pixels, enabled, checked, callback);

This file now has only white-space changes, which should be reverted.

tests/system/src/test/java/test/javafx/scene/control/SystemMenuBarTest.java line 1:

> 1: package test.javafx.scene.control;

This file needs a copyright header.

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

PR: https://git.openjdk.org/jfx/pull/987