RFR(S): JDK-8030708: warnings from b119 for jdk/src/share/back: JNI exception pending

[David Holmes]

Hi Dmitry,

Sorry this slipped through the cracks.

On 21/01/2015 11:05 PM, Dmitry Samersoff wrote:
> David,
>
> Please, take a look at updated webrev
>
> http://cr.openjdk.java.net/~dsamersoff/JDK-8030708/webrev.03/

src/jdk.jdwp.agent/share/native/libjdwp/StringReferenceImpl.c.

Don't you need to execute the END_WITH_LOCAL_REFS before you can return?

Otherwise seems okay.

Thanks,
David

> -Dmitry
>
>
> On 2014-10-16 16:07, David Holmes wrote:
>> 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
>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>
>>>
>
>