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

[Dmitry Samersoff]

Serguei,

Thanks.

-Dmitry

On 2015-01-29 11:10, serguei.spitsyn at oracle.com wrote:
> 
> 
> On 1/29/15 12:06 AM, serguei.spitsyn at oracle.com wrote:
>> Hi Dmitry,
>>
>>
>> It looks good in general.
> 
> In fact, I've reviewed the webrev.3 (but replied on the wrong email with
> webrev.2):
>   http://cr.openjdk.java.net/~dsamersoff/JDK-8030708/webrev.03/
> 
> 
> Thanks,
> Serguei
>>
>> src/jdk.jdwp.agent/share/native/libjdwp/StringReferenceImpl.c
>>
>> I agree with David, the fix needs the "END_WITH_LOCAL_REFS(env);"
>> before the line 50.
>>
>> src/jdk.jdwp.agent/share/native/libjdwp/commonRef.c 
>>  159          * It never throws OOM
>>
>> Minor: Could you, please, add a dot at the end of the comment statement?
>>
>> No need to re-review after the above is fixed.
>>
>> Thanks,
>> Serguei
>>
>>
>> On 10/16/14 3:08 AM, Dmitry Samersoff wrote:
>>> David,
>>>
>>> Changed. Thank you for review!
>>>
>>> please, see:
>>>
>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8030708/webrev.02/
>>>
>>> -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.