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

[David Holmes]

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
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>
>>>
>
>