On Mon, 2 Jan 2023 09:26:14 GMT, Florian Kirmaier <fkirma...@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
