RFR: 8318841: macOS: Memory leak with MenuItem when Menu.useSystemMenuBar(true) is used [v5]

[Kevin Rushforth]

On Thu, 9 Nov 2023 12:46:35 GMT, Johan Vos <jvos at openjdk.org> wrote:

>> When the Java layer removes a systemmenu, release the native resources related to this systemmenu.
>> This removes the strong JNI Global ref, which prevents its references from being gc'ed.
>> 
>> The current implementation for the mac-specific system menu creates a menu, but never releases its resources. In the `dealloc` of this menu, the strong jni refs are deleted.
>> With this PR, we now release the native resources associated with a menuItem when that one is removed from a menu. A consequence is that this menuItem should never be used after being removed from its current menu (e.g. it should not be re-added, or its text/shortcut should not be altered). 
>> The current implementation will create a new MacMenuDelegate every time a menuItem is inserted into a menu, so there should be no references to the native resources lingering.
>
> Johan Vos has updated the pull request incrementally with one additional commit since the last revision:
> 
>   countdown in case of a failure as well
>   use jmemorybuddy to detect unreachable weakrefs.

The test changes look good. One thing I noticed is that it now takes ~ 50 seconds to time out if there is a failure (a call to checkCollectable that returns false takes ~ 1 second, which is needed to ensure that it is a robust check).

Maybe cut down the number of menu objects from 50 to, say, 10? I'll approve it as is, since the only way this will ever be a problem is if we reintroduce this bug, and it's still under 1 minute so OK.

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

Marked as reviewed by kcr (Lead).

PR Review: https://git.openjdk.org/jfx/pull/1277#pullrequestreview-1722491475