On Thu, 5 Jan 2023 16:15:38 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
>> I've tested this version with explicit GC, and it still worked. So i guess
>> it's stable.
>>
>> Ok, so now there are multiple Solutions:
>>
>> 1. Revert to the previous version, and remove the WeakReference again.
>> 2. Add an explicit reference to the Callable in the MacMenuDelegate. This
>> would make proof reading much easier.
>> 3. Keep it as is.
>>
>> I guess I would prefer 2. Would that be ok for everyone?
>>
>> @mstr2
>> I guess this would have to be done in this method:
>>
>>
>> - (void)action:(id)sender
>> {
>> if (self->jCallback != NULL)
>> {
>> GET_MAIN_JENV;
>> if (env != NULL)
>> {
>> (*env)->CallVoidMethod(env, self->jCallback, jMenuActionMethod,
>> NULL);
>> }
>> }
>> }
>>
>> Do you have an example, of how this should look like? I'm neither familiar
>> with C, Objective-C, or this JNI code.
>
>> I've tested this version with explicit GC, and it still worked. So i guess
>> it's stable.
>>
>> Ok, so now there are multiple Solutions:
>>
>> 1. Revert to the previous version, and remove the WeakReference again.
>> 2. Add an explicit reference to the Callable in the MacMenuDelegate. This
>> would make proof reading much easier.
>> 3. Keep it as is.
>>
>> I guess I would prefer 2. Would that be ok for everyone?
>>
>> @mstr2 I guess this would have to be done in this method:
>>
>> ```
>>
>> - (void)action:(id)sender
>> {
>> if (self->jCallback != NULL)
>> {
>> GET_MAIN_JENV;
>> if (env != NULL)
>> {
>> (*env)->CallVoidMethod(env, self->jCallback, jMenuActionMethod,
>> NULL);
>> }
>> }
>> }
>> ```
>>
>> Do you have an example, of how this should look like? I'm neither familiar
>> with C, Objective-C, or this JNI code.
>
> The `action` method would look something like this:
> ```objective-c
> - (void)action:(id)sender
> {
> GET_MAIN_JENV;
> if (env != NULL) {
> jobject jCallback = (*env)->NewLocalRef(env, self->jCallback);
> if (jCallback != NULL) {
> (*env)->CallVoidMethod(env, jCallback, jMenuActionMethod, NULL);
> (*env)->DeleteLocalRef(env, jCallback);
> }
> }
> }
>
>
> I've found another use of `jCallback` in `validateMenuItem`.
@mstr2
I've added the suggested nullcheck to the 2 places!
Adding a hard reference to the callback, in the MacMenuDelegate doesn't work
without reverting to my original change, which was way more complicated. So I
left this part.
As mentioned, I've tested this code with regularly forced "System.gc" and
didn't find any issues.
-------------
PR: https://git.openjdk.org/jfx/pull/987
