Code Review for jdk

Prepared by:simonis on Tue Jun 2 17:52:19 CEST 2015
Workspace:/net/usr.work/d046063/OpenJDK/jdk9-dev/jdk
Compare against: http://hg.openjdk.java.net/jdk9/dev/jdk
Compare against version:12061
Summary of changes: 39 lines changed: 31 ins; 0 del; 8 mod; 5102 unchg
Changeset: jdk.changeset
Bug id: JDK-8081674 : EmptyStackException at startup if running with extended or unsupported charset
Author comments:

Using an unsupported character encoding (e.g. vi_VN.TCVN on Linux) results in an immediate VM failure with jdk 8 and 9:

export LANG=vi_VN.TCVN
java -version
Error occurred during initialization of VM
java.util.EmptyStackException
	at java.util.Stack.peek(Stack.java:102)
	at java.lang.ClassLoader$NativeLibrary.getFromClass(ClassLoader.java:1751)
	at java.lang.ClassLoader$NativeLibrary.findBuiltinLib(Native Method)
	at java.lang.ClassLoader.loadLibrary0(ClassLoader.java:1862)
	at java.lang.ClassLoader.loadLibrary(ClassLoader.java:1835)
	at java.lang.Runtime.loadLibrary0(Runtime.java:870)
	at java.lang.System.loadLibrary(System.java:1119)
	at java.lang.System.initializeSystemClass(System.java:1194)

This is a consequence of "8005716: Enhance JNI specification to allow support of static JNI libraries in Embedded JREs".

With jdk 9 we get this error even if we're running with a supported charset which is in the ExtendedCharsets (as opposed to being in the StandardCharsets) class which is a consequence of delegating the loading of the ExtendedCharsets class to the ServiceLoader in jdk 9.

export LANG=eo.iso-8859-3
output-jdk9/images/jdk/bin/java -version
Error occurred during initialization of VM
java.util.EmptyStackException
	at java.util.Stack.peek(Stack.java:102)
	at java.lang.ClassLoader$NativeLibrary.getFromClass(ClassLoader.java:1737)
	at java.lang.ClassLoader$NativeLibrary.findBuiltinLib(Native Method)
	at java.lang.ClassLoader.loadLibrary0(ClassLoader.java:1866)
	at java.lang.ClassLoader.loadLibrary(ClassLoader.java:1840)
	at java.lang.Runtime.loadLibrary0(Runtime.java:874)
	at java.lang.System.loadLibrary(System.java:1111)
	at java.lang.System.initializeSystemClass(System.java:1186)

Here's why the exception happens for an unsupported charset (see the mixed stack trace below for the full details):

java.lang.System.loadLibrary() wants to load libzip.so. It calls java.lang.Runtime.loadLibrary0() which at the very beginning calls the native method ClassLoader$NativeLibrary.findBuiltinLib() which checks if the corresponding library is already statically linked into the VM (introduced by 8005716). Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib(), the native implementation of findBuiltinLib() in Classloader.c calls GetStringPlatformChars() to convert the library name into the native platform encoding. GetStringPlatformChars() calls the helper function jnuEncodingSupported() to check if the platform encoding which is stored in the property "sun.jnu.encoding" is supported by Java. jnuEncodingSupported() is implemented as follows:

static jboolean isJNUEncodingSupported = JNI_FALSE;
static jboolean jnuEncodingSupported(JNIEnv *env) {
    jboolean exe;
    if (isJNUEncodingSupported == JNI_TRUE) {
        return JNI_TRUE;
    }
    isJNUEncodingSupported = (jboolean) JNU_CallStaticMethodByName (
                                    env, &exe,
                                    "java/nio/charset/Charset",
                                    "isSupported",
                                    "(Ljava/lang/String;)Z",
                                    jnuEncoding).z;
    return isJNUEncodingSupported;
}

Once the function finds that the platform encoding is supported (by calling java.nio.charset.Charset.isSupported()) it caches this value and always returns it on following calls. However if the platform encoding is not supported, it ALWAYS calls java.nio.charset.Charset.isSupported() an every subsequent invocation.

In order to call the Java method Charset.isSupported() (in JNU_CallStaticMethodByName() in file jni_util.c), we have to call jni_FindClass() to convert the symbolic class name "java.nio.charset.Charset" into a class reference.

But unfortunately, jni_FindClass() (from jni.cpp in libjvm.so) has a special handling if called from java.lang.ClassLoader$NativeLibrary to ensure that JNI_OnLoad/JNI_OnUnload are executed in the correct class context:

  instanceKlassHandle k (THREAD, thread->security_get_caller_class(0));
  if (k.not_null()) {
    loader = Handle(THREAD, k->class_loader());
    // Special handling to make sure JNI_OnLoad and JNI_OnUnload are executed
    // in the correct class context.
    if (loader.is_null() &&
        k->name() == vmSymbols::java_lang_ClassLoader_NativeLibrary()) {
      JavaValue result(T_OBJECT);
      JavaCalls::call_static(&result, k,
                                      vmSymbols::getFromClass_name(),
                                      vmSymbols::void_class_signature(),
                                      thread);
      if (HAS_PENDING_EXCEPTION) {
        Handle ex(thread, thread->pending_exception());
        CLEAR_PENDING_EXCEPTION;
        THROW_HANDLE_0(ex);
      }

So if that's the case and jni_FindClass() was reallycalled from ClassLoader$NativeLibrary, then jni_FindClass() calles ClassLoader$NativeLibrary().getFromClass() to find out the corresponding context class which is supposed to be saved there in a field of type java.util.Stack named "nativeLibraryContext":

// Invoked in the VM to determine the context class in
// JNI_Load/JNI_Unload
static Class<?> getFromClass() {
    return ClassLoader.nativeLibraryContext.peek().fromClass;
}

Unfortunately, "nativeLibraryContext" doesn't contain any entry at this point and the invocation of Stack.peek() will throw the exception shown before. In general, the "nativeLibraryContext" stack will be filled later on in Runtime.loadLibrary0() like this:

NativeLibrary lib = new NativeLibrary(fromClass, name, isBuiltin);
nativeLibraryContext.push(lib);
try {
    lib.load(name, isBuiltin);
} finally {
    nativeLibraryContext.pop();
}

such that it always contains at least one element later when jni_FindClass() will be invoked.

So in summary, the problem is that the implementors of 8005716 didn't took into account that calling ClassLoader$NativeLibrary.findBuiltinLib() may trigger a call to jni_FindClass() if we are running on a system with an unsupported character encoding.

I'd suggest the following fix for this problem:

Change ClassLoader$NativeLibrary().getFromClass() to return null if the stack is empty instead of throwing an exception:

static Class<?> getFromClass() {
    return ClassLoader.nativeLibraryContext.empty() ?
        null : ClassLoader.nativeLibraryContext.peek().fromClass;
}

Unfortunately this also requires a HotSpot change in jni_FindClass() in order to properly handle the new 'null' return value:

  if (k.not_null()) {
    loader = Handle(THREAD, k->class_loader());
    // Special handling to make sure JNI_OnLoad and JNI_OnUnload are executed
    // in the correct class context.
    if (loader.is_null() &&
        k->name() == vmSymbols::java_lang_ClassLoader_NativeLibrary()) {
      JavaValue result(T_OBJECT);
      JavaCalls::call_static(&result, k,
                                      vmSymbols::getFromClass_name(),
                                      vmSymbols::void_class_signature(),
                                      thread);
      if (HAS_PENDING_EXCEPTION) {
        Handle ex(thread, thread->pending_exception());
        CLEAR_PENDING_EXCEPTION;
        THROW_HANDLE_0(ex);
      }
      oop mirror = (oop) result.get_jobject();
      if (oopDesc::is_null(mirror)) {
        loader = Handle(THREAD, SystemDictionary::java_system_loader());
      } else {
        loader = Handle(THREAD,
          InstanceKlass::cast(java_lang_Class::as_Klass(mirror))->class_loader());
        protection_domain = Handle(THREAD,
          InstanceKlass::cast(java_lang_Class::as_Klass(mirror))->protection_domain());
      }
    }
  } else {
    // We call ClassLoader.getSystemClassLoader to obtain the system class loader.
    loader = Handle(THREAD, SystemDictionary::java_system_loader());
  }

These changes are sufficient to solve the problem in Java 8. Unfortunately, that's still not enough in Java 9 because there the loading of the extended charsets has been delegated to ServiceLoader. But ServiceLoader calls ClassLoader.getBootstrapResources() which calls sun.misc.Launcher.getBootstrapClassPath(). This leads to another problem during class initialization of sun.misc.Launcher if running on an unsupported locale.

The first thing done in sun.misc.Launcher.<clinit> is the initialisation of the bootstrap URLClassPath in the Launcher. However this initialisation will eventually call Charset.isSupported() and if we are running on an unsupported locale this will inevitably end in another recursive call to ServiceLoader. But as explained below, ServiceLoader will query the Launcher's bootstrap URLClassPath which will be still uninitialized at that point.

So we'll have to additionally guard guard against this situation on JDK 9 like this:

private static Charset lookupExtendedCharset(String charsetName) {
    if (!sun.misc.VM.isBooted() ||             // see lookupViaProviders()
        sun.misc.Launcher.getBootstrapClassPath() == null)
        return null;

This fixes the crashes, but still at the price of not having the extended charsets available during initialization until Launcher.getBootstrapClassPath is set up properly. This may be still a problem if the jdk is installed in a directory which contains characters specific to an extended encoding or if we have such characters in the command line arguments.

Mixed stack trace of the initial EmptyStackException for unsupported charsets described before:

Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
j  java.util.Stack.peek()Ljava/lang/Object;+1
j  java.lang.ClassLoader$NativeLibrary.getFromClass()Ljava/lang/Class;+3
v  ~StubRoutines::call_stub
V  [libjvm.so+0x9d279a]  JavaCalls::call_helper(JavaValue*, methodHandle*, JavaCallArguments*, Thread*)+0x6b4
V  [libjvm.so+0xcad591]  os::os_exception_wrapper(void (*)(JavaValue*, methodHandle*, JavaCallArguments*, Thread*), JavaValue*, methodHandle*, JavaCallArguments*, Thread*)+0x45
V  [libjvm.so+0x9d20cf]  JavaCalls::call(JavaValue*, methodHandle, JavaCallArguments*, Thread*)+0x8b
V  [libjvm.so+0x9d1d3b]  JavaCalls::call_static(JavaValue*, KlassHandle, Symbol*, Symbol*, JavaCallArguments*, Thread*)+0x139
V  [libjvm.so+0x9d1e3f]  JavaCalls::call_static(JavaValue*, KlassHandle, Symbol*, Symbol*, Thread*)+0x9d
V  [libjvm.so+0x9e6588]  jni_FindClass+0x428
C  [libjava.so+0x20208]  JNU_CallStaticMethodByName+0xff
C  [libjava.so+0x21cae]  jnuEncodingSupported+0x61
C  [libjava.so+0x22125]  JNU_GetStringPlatformChars+0x125
C  [libjava.so+0xedcd]  Java_java_lang_ClassLoader_00024NativeLibrary_findBuiltinLib+0x8b
j  java.lang.ClassLoader$NativeLibrary.findBuiltinLib(Ljava/lang/String;)Ljava/lang/String;+0
j  java.lang.ClassLoader.loadLibrary0(Ljava/lang/Class;Ljava/io/File;)Z+4
j  java.lang.ClassLoader.loadLibrary(Ljava/lang/Class;Ljava/lang/String;Z)V+228
j  java.lang.Runtime.loadLibrary0(Ljava/lang/Class;Ljava/lang/String;)V+54
j  java.lang.System.loadLibrary(Ljava/lang/String;)V+7
j  java.lang.System.initializeSystemClass()V+113
v  ~StubRoutines::call_stub
V  [libjvm.so+0x9d279a]  JavaCalls::call_helper(JavaValue*, methodHandle*, JavaCallArguments*, Thread*)+0x6b4
V  [libjvm.so+0xcad591]  os::os_exception_wrapper(void (*)(JavaValue*, methodHandle*, JavaCallArguments*, Thread*), JavaValue*, methodHandle*, JavaCallArguments*, Thread*)+0x45
V  [libjvm.so+0x9d20cf]  JavaCalls::call(JavaValue*, methodHandle, JavaCallArguments*, Thread*)+0x8b
V  [libjvm.so+0x9d1d3b]  JavaCalls::call_static(JavaValue*, KlassHandle, Symbol*, Symbol*, JavaCallArguments*, Thread*)+0x139
V  [libjvm.so+0x9d1e3f]  JavaCalls::call_static(JavaValue*, KlassHandle, Symbol*, Symbol*, Thread*)+0x9d
V  [libjvm.so+0xe3cceb]  call_initializeSystemClass(Thread*)+0xb0
V  [libjvm.so+0xe44444]  Threads::initialize_java_lang_classes(JavaThread*, Thread*)+0x21a
V  [libjvm.so+0xe44b12]  Threads::create_vm(JavaVMInitArgs*, bool*)+0x4a6
V  [libjvm.so+0xa19bd7]  JNI_CreateJavaVM+0xc7
C  [libjli.so+0xa520]  InitializeJVM+0x154
C  [libjli.so+0x8024]  JavaMain+0xcc
Legend: Modified file
Deleted file
New file

Cdiffs Udiffs Sdiffs Frames Old New ----- Raw src/java.base/share/classes/java/lang/ClassLoader.java

rev 12062 : 8081674: EmptyStackException at startup if running with extended or unsupported charset
2 lines changed: 1 ins; 0 del; 1 mod; 2206 unchg

Cdiffs Udiffs Sdiffs Frames Old New ----- Raw src/java.base/share/classes/java/nio/charset/Charset.java

rev 12062 : 8081674: EmptyStackException at startup if running with extended or unsupported charset
10 lines changed: 7 ins; 0 del; 3 mod; 918 unchg

Cdiffs Udiffs Sdiffs Frames Old New ----- Raw src/java.base/share/native/libjava/jni_util.c

rev 12062 : 8081674: EmptyStackException at startup if running with extended or unsupported charset
12 lines changed: 11 ins; 0 del; 1 mod; 1342 unchg

Cdiffs Udiffs Sdiffs Frames Old New ----- Raw src/java.base/unix/native/libjava/java_props_md.c

rev 12062 : 8081674: EmptyStackException at startup if running with extended or unsupported charset
15 lines changed: 12 ins; 0 del; 3 mod; 636 unchg

This code review page was prepared using /usr/work/d046063/OpenJDK/webrev/webrev.ksh (vers 25.8-hg+openjdk.java.net).