RFR: 8189633: Missing -Xcheck:jni checking for DeleteWeakGlobalRef

[Daniel D. Daugherty]

On 8/19/19 9:10 PM, Kim Barrett wrote:
>> On Jan 5, 2018, at 1:04 AM, Kim Barrett <kim.barrett at oracle.com> wrote:
>>
>>> On Jan 3, 2018, at 12:31 AM, David Holmes <david.holmes at oracle.com> wrote:
>>>
>>> Hi Kim,
>>>
>>> On 3/01/2018 9:30 AM, Kim Barrett wrote:
>>>> Please review this change to the checking form of DeleteWeakGlobalRef
>>>> to actually perform some checking.  The checking that is now being
>>>> done was cribbed from DeleteGlobalRef.
>>>> CR:
>>>> https://bugs.openjdk.java.net/browse/JDK-8189633
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~kbarrett/8189633/open.00/
>>> Doesn't the fact it is a weak-ref mean that resolving it could return NULL, in which case validate_object will trigger a JNI fatal error:
>>> […]
>> You’re right.  I thought I’d used validate_handle, but obviously not.
>>
>> I think I should leave this issue alone for now, and come back to it after JDK-8194312 is done.
>> There are some changes to jniCheck there already, and I think some more, like here, can
>> be simplified or improved by relying on the JDK-8194312 changes to JNIHandles.
> Finally getting back to this.
>
> full: http://cr.openjdk.java.net/~kbarrett/8189633/open.01/

src/hotspot/share/prims/jniCheck.cpp
     L1931:       if (ref && !JNIHandles::is_weak_global_handle(ref)) {
         Possible implied boolean. Perhaps:

                  if (ref != NULL && 
!JNIHandles::is_weak_global_handle(ref)) {

Thumbs up! Your call on whether to fix the possible implied boolean.
No need for another webrev if you do change it.

Dan


> incr: http://cr.openjdk.java.net/~kbarrett/8189633/open.01.inc/
>
> Testing:
> mach5 tier1-3.
> local tier1 with -Xcheck:jni (thanks for fixing JDK-8189766, David).
> Reported JDK-8229900 (a tier1 failure with -Xcheck:jni that is unrelated to this change).
>
>
>