RFR 8030115: [parfait] warnings from b119 for jdk.src.share.native.sun.tracing.dtrace: JNI exception pending

[serguei.spitsyn at oracle.com]

Thanks, Dmitry!

I reviewed it again. Now I think the fix is good.
Sorry for the wrong opinion and confusion.
One nit is about the line:
   205 noErrors &= readProviderData(env, provider, p);

The ampersand '&' is not necessary here.
The loop is stopped as soon as the noErrors is equal to zero, so there 
is no need to accumulate it.

Thanks,
Serguei

On 7/25/14 4:16 PM, Dmitry Samersoff wrote:
> Serguei,
>
> Previous fix get offset to pointer passed as parameter without bounds
> check. It's safe here, but not the best solution in general and cause a
> warning.
>
> Also readProviderData checks for exceptions, so I see no reason to check
> it again.
>
> *In this version:*
>
> we don't need to count errors and can abort after first one.
>
> CHECK_(0)L at 204 cause the function to return, but left allocated memory.
>
> -Dmitry
>
> On 2014-07-26 01:34, serguei.spitsyn at oracle.com wrote:
>> Previous fix was more elegant.
>> In fact, I do not like new one.
>>
>> Thanks,
>> Serguei
>>
>> On 7/25/14 6:04 AM, Jaroslav Bachorik wrote:
>>> On 07/25/2014 01:40 PM, Dmitry Samersoff wrote:
>>>> Jaroslav,
>>>>
>>>> readProviderData already use CHECK
>>>>
>>>> So it might be better to:
>>>>
>>>> 1. change readProviderData to return 0 on error and 1 on success.
>>>>
>>>> 2. add check for exception to ll 201 and abort the loop if at least
>>>>      one of providers is not read.
>>> Ok, here is another attempt, now without introducing a new function.
>>>
>>> http://cr.openjdk.java.net/~jbachorik/8030115/webrev.01
>>>
>>> -JB-
>>>
>>>>
>>>> -Dmitry
>>>>
>>>>
>>>> On 2014-07-24 15:07, Jaroslav Bachorik wrote:
>>>>> Please, review the change for the fix of the following problem
>>>>>
>>>>> In jdk/src/share/native/sun/tracing/dtrace/JVM.c the following code is
>>>>> invoked in loop
>>>>>
>>>>> 198     for (i = 0; i < num_providers; ++i) {
>>>>> 199         JVM_DTraceProvider* p = &(jvm_providers[i]);
>>>>> 200         jobject provider = (*env)->GetObjectArrayElement(
>>>>> 201             env, providers, i);
>>>>> 202         readProviderData(env, provider, p);
>>>>> 203     }
>>>>>
>>>>> The first problem is that GetGetObjectArrayElement() call on L200 may
>>>>> throw an exception which is not checked subsequently. On L202 the call
>>>>> to readProviderData() can also raise a Java exception without
>>>>> appropriate after-check. When getting back to the beginning of the loop
>>>>> GetObjectArrayElement() may be called with a pending exception which is
>>>>> deemed unsafe.
>>>>>
>>>>> The fix extracts the inner part of the loop into a separate function
>>>>> where the result of each step is properly checked for pending
>>>>> exceptions. If there is a pending exception the loop will be
>>>>> interrupted, resources cleaned and
>>>>> Java_sun_tracing_dtrace_JVM_activate0() function will return 0 with the
>>>>> pending exception recorded in env.
>>>>>
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~jbachorik/8030115/webrev.00
>>>>>
>>>>> Thanks,
>>>>>
>>>>> -JB-
>>>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20140726/ede071b6/attachment.html>