RFR[10] 8190984 : tools/launcher/TestXcheckJNIWarnings.java WARNING was found in the output

[Brent Christian]

What David said. :)

One other wrinkle: I ProblemList-ed the test a few days ago[1] in 
jdk/jdk.  AFAICT, that change is not yet in jdk/hs, so the changes 
David's pushing can't be used to take the test back off of the 
ProblemList.  I figure I'll have to file a new issue, to be fixed in 
jdk/jdk (unless someone has a better idea).

Thanks,
-Brent

1. http://hg.openjdk.java.net/jdk/jdk/rev/b9a19d1e61f2

On 12/12/17 4:18 PM, David Holmes wrote:
> 
> IIRC Brent instrumented the localref methods to see how many were being 
> created, and the "50" gives us sufficient capacity. The actual number of 
> local refs introduced seems to be platform and locale specific.
> 
> Note this only affects the warnings that -Xcheck:jni produces (at least 
> on hotspot). If more properties are added that take us beyond the new 
> capacity then we'll see test failures again and can increase it further.
> 
> Thanks,
> David
> 
>> Mandy
>>
>> On 12/12/17 4:00 PM, David Holmes wrote:
>>> Reviewed! :)
>>>
>>> Thanks Brent!
>>>
>>> I guess I can push these both now.
>>>
>>> David
>>>
>>> On 13/12/2017 9:38 AM, Brent Christian wrote:
>>>> Hi,
>>>>
>>>> Please review this small change to the System.initProperties() 
>>>> native method.
>>>>
>>>> The tools/launcher/TestXcheckJNIWarnings.java has begun to fail due 
>>>> to this warning being issued:
>>>>
>>>> WARNING: JNI local refs: 41, exceeds capacity: 40
>>>>          at java.lang.System.initProperties(java.base/Native Method)
>>>>          at java.lang.System.initPhase1(java.base/System.java:1943)
>>>>
>>>> This has only been seen on solaris-sparc with LANG=C.  In fact, the 
>>>> warning can also be seen with a simple HelloWorld program, run with 
>>>> -Xcheck:jni.  This started happening as of JDK-8043224 ("-Xcheck:jni 
>>>> improvements to exception checking and excessive local refs").
>>>>
>>>> The following change should be made to prevent the warning:
>>>>
>>>> diff -r ed1bb7743b3e src/java.base/share/native/libjava/System.c
>>>> --- a/src/java.base/share/native/libjava/System.c    Tue Dec 12 
>>>> 19:05:02 2017 +0100
>>>> +++ b/src/java.base/share/native/libjava/System.c    Tue Dec 12 
>>>> 15:21:03 2017 -0800
>>>> @@ -186,6 +186,9 @@
>>>>       jobject ret = NULL;
>>>>       jstring jVMVal = NULL;
>>>>
>>>> +    if ((*env)->EnsureLocalCapacity(env, 50) < 0) {
>>>> +        return NULL;
>>>> +    }
>>>>       sprops = GetJavaProperties(env);
>>>>       CHECK_NULL_RETURN(sprops, NULL);
>>>>
>>>>
>>>> Of note: an additional change is needed, to EnsureLocalCapacity itself:
>>>>
>>>> 8193222 : EnsureLocalCapacity() should maintain capacity requests 
>>>> through multiple calls
>>>>
>>>> That fix has already been proposed and reviewed.
>>>> These two fixes should be pushed together via hs, which David has 
>>>> kindly offered to do, pending code review approval of my change here.
>>>>
>>>> Thanks,
>>>> -Brent
>>>>
>>