Hi,
While building the J9 JVM for the IBM SDK for Java(*), we've run across
what we believe is a race condition in initializeEncoding(JNIEnv*) in
jni_util.c.
(*) The IBM SDK for Java is comprised of the J9 JVM and OpenJDK
class library.
We've encountered a situation where (*env)->CallObjectMethod() in
JNU_GetStringPlatformChars() is invoked with an uninitialized method ID
(the static variable "String_getBytes_ID").
The function initializeEnconding() initializes 4 static variables, but only
one of them ("fastEncoding") is checked to see if initializeEnconding()
should be called in JNU_NewStringPlatform() and JNU_GetStringPlatformChars
(). Further, "fastEncoding" is the first of the 4 static variables set by
initializeEncoding() leaving a race condition where another thread sees
"fastEncoding" set but "String_getBytes_ID" is not yet initialized.
I believe that moving the initialization of "String_getBytes_ID" and
"String_init_ID" to the top of the function would eliminate the impact of
the race condition.
I'd also recommend moving the initialization of "jnuEncoding" up one line,
but this may lead to leaking a global ref.
We are able to reproduce the failure in JNU_GetStringPlatformChars() due to
using an null String_getBytes_ID variable for a method id race condition in
our environment 1 in 20. With the following patch applied, the race
condition can no longer be reproduced.
Do you agree there is a race condition? If so, would the patch be an
appropriate fix? We could introduce locking, but that feels like
overkill.
Ron.
diff -r efa71dc820eb src/java.base/share/native/libjava/jni_util.c
--- a/src/java.base/share/native/libjava/jni_util.c Mon Nov 07 13:10:42
2016 -0400
+++ b/src/java.base/share/native/libjava/jni_util.c Wed Nov 09 17:19:16
2016 -0500
@@ -688,6 +688,15 @@
strClazz = JNU_ClassString(env);
CHECK_NULL(strClazz);
+
+ /* Initialize method-id cache */
+ String_getBytes_ID = (*env)->GetMethodID(env, strClazz,
+ "getBytes",
"(Ljava/lang/String;)[B");
+ CHECK_NULL(String_getBytes_ID);
+ String_init_ID = (*env)->GetMethodID(env, strClazz,
+ "<init>",
"([BLjava/lang/String;)V");
+ CHECK_NULL(String_init_ID);
+
propname = (*env)->NewStringUTF(env, "sun.jnu.encoding");
if (propname) {
@@ -727,8 +736,8 @@
strcmp(encname, "utf-16le") == 0)
fastEncoding = FAST_CP1252;
else {
+ jnuEncoding = (jstring)(*env)->NewGlobalRef(env,
enc);
fastEncoding = NO_FAST_ENCODING;
- jnuEncoding = (jstring)(*env)->NewGlobalRef(env,
enc);
}
(*env)->ReleaseStringUTFChars(env, enc, encname);
}
@@ -741,13 +750,6 @@
}
(*env)->DeleteLocalRef(env, propname);
(*env)->DeleteLocalRef(env, enc);
-
- /* Initialize method-id cache */
- String_getBytes_ID = (*env)->GetMethodID(env, strClazz,
- "getBytes",
"(Ljava/lang/String;)[B");
- CHECK_NULL(String_getBytes_ID);
- String_init_ID = (*env)->GetMethodID(env, strClazz,
- "<init>",
"([BLjava/lang/String;)V");
}
