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

[Dmitry Samersoff]

David,

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

done.

http://cr.openjdk.java.net/~dsamersoff/JDK-8030708/webrev.04/

-Dmitry

On 2015-01-28 10:24, David Holmes wrote:
> 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
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>
>>


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.