Hi Mandy,
On 13/12/2017 10:12 AM, mandy chung wrote:
How do we pick the value 50? The JNI local refs are deleted in some
cases and it's unclear how 50 is computed. What if new properties are
added in the future?
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
