Incorrect return value for Java_jdk_internal_loader_NativeLibraries_load() in java.base/share/native/libjava/NativeLibraries.c

[Cheng Jin]

Hi there,

The return value from Java_jdk_internal_loader_NativeLibraries_load() at https://github.com/openjdk/jdk/blob/master/src/java.base/share/native/libjava/NativeLibraries.c
seems incorrect according to the code logic in there.

Looking at the calling path from  loadLibrary() to load() at src/java.base/share/classes/jdk/internal/loader/NativeLibraries.java as follows:

    public NativeLibrary loadLibrary(Class<?> fromClass, String name) {
        ...
        NativeLibrary lib = findFromPaths(LibraryPaths.SYS_PATHS, fromClass, name); <----------
        if (lib == null && searchJavaLibraryPath) {
            lib = findFromPaths(LibraryPaths.USER_PATHS, fromClass, name); <----
        }
        return lib;
    }

      private NativeLibrary findFromPaths(String[] paths, Class<?> fromClass, String name) {
        for (String path : paths) {  <---------------- keep searching the library paths till the library is located at the correct lib path
            File libfile = new File(path, System.mapLibraryName(name));
            NativeLibrary nl = loadLibrary(fromClass, libfile);  ?
            if (nl != null) {
                return nl; <--------------------
            }
            libfile = ClassLoaderHelper.mapAlternativeName(libfile);
            if (libfile != null) {
                nl = loadLibrary(fromClass, libfile);
                if (nl != null) {
                    return nl;
                }
            }
        }
        return null;
}

    public NativeLibrary loadLibrary(Class<?> fromClass, File file) {
       // Check to see if we're attempting to access a static library
        String name = findBuiltinLib(file.getName());
        boolean isBuiltin = (name != null);
        ...
        return loadLibrary(fromClass, name, isBuiltin);  <------------
    }

   private NativeLibrary loadLibrary(Class<?> fromClass, String name, boolean isBuiltin) {
   ...
            NativeLibraryImpl lib = new NativeLibraryImpl(fromClass, name, isBuiltin, isJNI);

                       // load the native library
            NativeLibraryContext.push(lib);
            try {
                if (!lib.open()) {  <--------------------- call load()
                    return null;    // fail to open the native library
                }
             ...
           // register the loaded native library
            loadedLibraryNames.add(name);
            libraries.put(name, lib);
            return lib;  <--------------- return an invalid lib as lib.open() returns true
        } finally {
            releaseNativeLibraryLock(name);
        }
  ...
  }

        /*
         * Loads the named native library
         */
        boolean open() {
            ...
            return load(this, name, isBuiltin, isJNI, loadLibraryOnlyIfPresent);
        }

    private static native boolean load(NativeLibraryImpl impl, String name, boolean isBuiltin, boolean isJNI, boolean throwExceptionIfFail);

and then native code in java.base/share/native/libjava/NativeLibraries.c

Java_jdk_internal_loader_NativeLibraries_load
  (JNIEnv *env, jobject this, jobject lib, jstring name,
   jboolean isBuiltin, jboolean isJNI, jboolean throwExceptionIfFail)
{
    const char *cname;
    jint jniVersion;
    jthrowable cause;
    void * handle;
    jboolean loaded = JNI_FALSE;
   ...
    handle = isBuiltin ? procHandle : JVM_LoadLibrary(cname, throwExceptionIfFail);
    if (isJNI) {
       ...
    }
    (*env)->SetLongField(env, lib, handleID, ptr_to_jlong(handle));
    loaded = JNI_TRUE;  <----- always return true when isJNI is false and a null handle

done:
    JNU_ReleaseStringPlatformChars(env, name, cname);
    return loaded;
}

Assuming there are multiple library paths existing in the jdk/lib which are added to sun.boot.library.path and java.library.path
and the expected library (libnative.so) is only placed in jdk/lib. e.g.
lib/libA
lib/libB
lib/libC
lib/libnative.so

when the code in findFromPaths() searches the library paths set in sun.boot.library.path and java.library.path starting from lib/LibA,
it will end up with an invalid value if load() returns true in the case of a false isJNI and a null handle returned from JVM_LoadLibrary()
as the library doesn't exist in lib/libA passed to JVM_LoadLibrary().

To ensure findFromPaths() keeps searching the remaining lib paths,  the simple fix should look like:
Java_jdk_internal_loader_NativeLibraries_load( ...)
{
...
    (*env)->SetLongField(env, lib, handleID, ptr_to_jlong(handle));
    if (handle) {  <------------------------- return true only for a non-null handle
        loaded = JNI_TRUE;
    }
...
}

Please help check the code around there to see whether the fix is reasonable to avoid the unexpected situation as explained above (the problem was spotted since JDK17)

Best Regards
Cheng Jin