On Wed, 20 Jan 2021 06:02:55 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
>> This removes desktop module usage of the JNF JNI reference convenience APIs
>> These are simply a direct conversion
>> JNFNewGlobalRef
>> JNFDeleteGlobalRef
>> JNFNewWeakGlobalRef
>> JNFDeleteWeakGlobalRef
>>
>> These two
>> JNFJObjectWrapper
>> JNFWeakJObjectWrapper
>> exist to allow clean up of the refs when a Cocoa wrapper object is released.
>> However in all cases there are more direct ways to clean it up and in at
>> least one usage
>> the existing code directly releases it with the comment that this is more
>> efficient.
>>
>> All our automated regression and JCK tests pass with this change.
>
> src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m line 1547:
>
>> 1545: AWTWindow *awtWindow = [AWTWindow getTopmostWindowUnderMouse];
>> 1546: if (awtWindow != nil) {
>> 1547: topmostWindowUnderMouse = awtWindow.javaPlatformWindow;
>
> I wonder what type of "reference" we should return here, I do not remember
> how the "NewWeakGlobalRef" behave when returned to java.
It is no different than other cases. Java will get a new strong ref to the
object
and expect it to be of the return type of this native method.
> src/java.desktop/macosx/native/libawt_lwawt/awt/CDragSourceContextPeer.m line
> 58:
>
>> 56: jobject gTriggerEvent = (*env)->NewGlobalRef(env, jtrigger);
>> 57: jlongArray gFormats = (*env)->NewGlobalRef(env, jformats);
>> 58: jobject gFormatMap = (*env)->NewGlobalRef(env, jformatmap);
>
> All above should be checked for NULL since OOM may occur, but it looks like
> it does not throw OOM?
> https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/functions.html#NewGlobalRef
It is the case that per even the latest spec. none of these throw any exception.
https://docs.oracle.com/en/java/javase/15/docs/specs/jni/functions.html#newglobalref
So I think the existing code doesn't do anything in the event of a NULL return.
And if you want to check for NULL here and return NULL from the native method
there's semantic implications that require the caller never pass NULL for any
of these.
It is not illegal to pass NULL to NewGlobalRef.
Investigating and confirming that is beyond the scope of this change.
Or we make the code a bit more complex here and check that we get back non-null
for a non-null arg. But once again nothing new here.
> src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.h line 55:
>
>> 53: @property (nonatomic, retain) NSWindow *nsWindow;
>> 54:
>> 55: @property (nonatomic) jobject javaPlatformWindow;
>
> I think it will be useful to have a comment here that this value is a weak
> reference and should be copied to the local ref before usage.
I suppose I can add a comment ... but it is already done in the couple of dozen
usages so any one adding a new usage who misses that will likely miss the
comment too.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2133
