RFR (S): JDK-8164086: Checked JNI pending exception check should be cleared when returning to Java frame

[David Holmes]

On 6/09/2016 5:51 PM, David Simms wrote:
>
> Updated webrev: http://cr.openjdk.java.net/~dsimms/8164086/webrev3/

No further comments from me, but note my original comment re the 
platform specific code.

More below.

> On 06/09/16 01:20, David Holmes wrote:
>> Hi David,
>>
>> On 5/09/2016 6:02 PM, David Simms wrote:
>>> Hi David,
>>>
>>> I can make the checks silent, but the launcher itself produces warnings
>>> from checked JNI (it's use of unchecked Java method invocations), and
>>> this causes the test
>>> (http://cr.openjdk.java.net/~dsimms/8164086/webrev2/hotspot/test/runtime/jni/checked/TestCheckedJniExceptionCheck.java.html)
>>>
>>> to fail, with unchecked exception warnings popping up in the output.
>>
>> I've looked back at your original changes in this area but I'm still
>> not seeing exactly where the unchecked back-to-back JNI calls are
>> happening. For example NewPlatformString can return with an exception
>> pending, but will return NULL, and the calling code has checks for NULL.
>>
>
> LoadMainClass:
>
> http://hg.openjdk.java.net/jdk9/hs/jdk/file/090cbd92c744/src/java.base/share/native/libjli/java.c#l1530
>
>
> Calls "CallStaticObjectMethod" from "NewPlatformString", then on line
> 1531 make another call "CallStaticObjectMethod".
>
> No exception check is made. The Xcheck:jni code cannot interpreter the
> return value from JNI "Call<X>Method" functions, so it triggers the
> "unchecked" exception warning at line 1531.

Ah I see. No bug in the launcher code though as it only makes the second 
JNI call if the original did not return NULL and hence there can be no 
exception pending.

This is a limitation of the unchecked warning facility as it can't take 
control flow into account. :(


>
>>> But yeah, I could adjust the test the ignore any start-up warnings and
>>> drop the changes to the launcher...
>>
>> Unless we can more clearly identify exactly where the launcher has a
>> problem I would prefer not to involve it in these changes. But I would
>> like to understand exactly why the launcher is triggering the check.
>>
>
> Dropped launcher changes, adjusted the test to ignore any previous
> warnings.

Yep - that seems best: only monitor the test itself not the overall VM 
operation.

Thanks,
David H.
-------

> Cheers
> /David Simms