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

[Dmitry Samersoff]

David,

Agree! Will change it.

-Dmitry

On 2015-01-29 10:26, David Holmes wrote:
> On 29/01/2015 12:00 AM, Dmitry Samersoff wrote:
>> 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/
> 
> Not quite how I envisaged the fix:
> 
> JNI_FUNC_PTR(env,PopLocalFrame)(env, NULL); // END_WITH_LOCAL_REFS
> 
> inlining the macro defeats the purpose the macro to some extent - if it
> were changed then this code would not get the change. I was thinking
> more along the lines of:
> 
>  WITH_LOCAL_REFS(env, 1) {
> 
>      char *utf;
> 
>      utf = (char *)JNI_FUNC_PTR(env,GetStringUTFChars)(env, string, NULL);
>      if (!(*env)->ExceptionCheck(env)) {
>         (void)outStream_writeString(out, utf);
>         JNI_FUNC_PTR(env,ReleaseStringUTFChars)(env, string, utf);
>      }
> 
> } END_WITH_LOCAL_REFS(env);
> 
> David
> 
>> -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.