[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR(S): JDK-8030708: warnings from b119 for jdk/src/share/back: JNI exception pending
From:       David Holmes <david.holmes () oracle ! com>
Date:       2014-10-16 12:07:11
Message-ID: 543FB4EF.5050706 () oracle ! com
[Download RAW message or body]

Hi Dmitry,

On 16/10/2014 8:08 PM, Dmitry Samersoff wrote:
> David,
>
> Changed. Thank you for review!
>
> please, see:
>
> http://cr.openjdk.java.net/~dsamersoff/JDK-8030708/webrev.02/

  102     if (weakRef == NULL || (*env)->ExceptionCheck(env)) {

Isn't the only time it will return NULL when an exception occurs? 
Conversely if an exception occurs then it must return NULL - so the 
exception check seems redundant.

But this also suggests you need similar logic at:

  182         weakRef = JNI_FUNC_PTR(env,NewWeakGlobalRef)(env, node->ref);

  456                 lref = JNI_FUNC_PTR(env,NewLocalRef)(env, node->ref);

Or more generally any JNI call from JVMTI should be wrapped in a way 
that checks for exceptions and clears them.

David

> -Dmitry
>
> On 2014-10-16 04:24, David Holmes wrote:
>> On 16/10/2014 12:33 AM, Dmitry Samersoff wrote:
>>> David,
>>>
>>> Sorry, copied wrong function!
>>>
>>> I mean this call
>>>
>>> weakRef = JNI_FUNC_PTR(env,NewWeakGlobalRef)(env, ref);
>>>
>>> that can post  OutOfMemoryError
>>
>> Okay, so shouldn't that be where the exception is cleared:
>>
>>     /* Create weak reference to make sure we have a reference */
>>      weakRef = JNI_FUNC_PTR(env,NewWeakGlobalRef)(env, ref);
>>      if (weakRef == NULL) {
>> +       // < clear exception here >
>>          jvmtiDeallocate(node);
>>          return NULL;
>>      }
>>
>> Thanks,
>> David
>> -----
>>
>>> commonRef_refToID() ->
>>>
>>> createNode(JNIEnv *env, jobject ref) ->
>>>
>>> weakRef = JNI_FUNC_PTR(env,NewWeakGlobalRef)(env, ref);
>>>
>>> -Dmitry
>>>
>>> On 2014-10-15 16:21, David Holmes wrote:
>>>> On 15/10/2014 8:39 PM, Dmitry Samersoff wrote:
>>>>> On 2014-10-15 14:27, David Holmes wrote:
>>>>>> On 15/10/2014 8:08 PM, Dmitry Samersoff wrote:
>>>>>>> Please review the fix:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8030708/webrev.01/
>>>>>>>
>>>>>>> Added missed exception checks.
>>>>>>
>>>>>> src/jdk.jdwp.agent/share/native/libjdwp/outStream.c
>>>>>>
>>>>>> What is potentially posting the exception?
>>>>>
>>>>> JvmtiEnv::GetTag(jobject object, jlong* tag_ptr) called from
>>>>> commonRef_refToID()
>>>>
>>>> You mean this call:
>>>>
>>>>      error = JVMTI_FUNC_PTR(gdata->jvmti,GetTag)(gdata->jvmti, ref,
>>>> &tag);
>>>
>>> x
>>>>
>>>> in findNodeByRef which is called by commonRef_refToID?  JVM TI doesn't
>>>> post exceptions.
>>>>
>>>> "JVM TI functions never throw exceptions; error conditions are
>>>> communicated via the function return value. Any existing exception state
>>>> is preserved across a call to a JVM TI function."
>>>>
>>>> http://docs.oracle.com/javase/8/docs/platform/jvmti/jvmti.html
>>>>
>>>> David
>>>>
>>>>> -Dmitry
>>>>>
>>>>>
>>>
>>>
>
>
[prev in list] [next in list] [prev in thread] [next in thread]