8282515: More clean up on NativeLibraries just for JNI library use

Reviewed-by: mcimadamore

src/java.base/share/classes/jdk/internal/loader/NativeLibraries.java

@@ -190,7 +190,7 @@ public final class NativeLibraries {
                 }
             }
 
-            NativeLibraryImpl lib = new NativeLibraryImpl(fromClass, name, isBuiltin, true);
+            NativeLibraryImpl lib = new NativeLibraryImpl(fromClass, name, isBuiltin);
             // load the native library
             NativeLibraryContext.push(lib);
             try {
@@ -290,21 +290,16 @@ public final class NativeLibraries {
         final String name;
         // Indicates if the native library is linked into the VM
         final boolean isBuiltin;
-        // Indicate if this is JNI native library
-        final boolean isJNI;
 
         // opaque handle to native library, used in native code.
         long handle;
         // the version of JNI environment the native library requires.
         int jniVersion;
 
-        NativeLibraryImpl(Class<?> fromClass, String name, boolean isBuiltin, boolean isJNI) {
+        NativeLibraryImpl(Class<?> fromClass, String name, boolean isBuiltin) {
-            assert !isBuiltin || isJNI : "a builtin native library must be JNI library";
-
             this.fromClass = fromClass;
             this.name = name;
             this.isBuiltin = isBuiltin;
-            this.isJNI = isJNI;
         }
 
         @Override
@@ -322,7 +317,7 @@ public final class NativeLibraries {
          * when this class loader becomes phantom reachable.
          */
         private Runnable unloader() {
-            return new Unloader(name, handle, isBuiltin, isJNI);
+            return new Unloader(name, handle, isBuiltin);
         }
 
         /*
@@ -333,14 +328,14 @@ public final class NativeLibraries {
                 throw new InternalError("Native library " + name + " has been loaded");
             }
 
-            return load(this, name, isBuiltin, isJNI, loadLibraryOnlyIfPresent);
+            return load(this, name, isBuiltin, loadLibraryOnlyIfPresent);
         }
 
         /*
          * Close this native library.
          */
         void close() {
-            unload(name, isBuiltin, isJNI, handle);
+            unload(name, isBuiltin, handle);
         }
     }
 
@@ -352,15 +347,13 @@ public final class NativeLibraries {
         // This represents the context when a native library is unloaded
         // and getFromClass() will return null,
         static final NativeLibraryImpl UNLOADER =
-                new NativeLibraryImpl(null, "dummy", false, false);
+                new NativeLibraryImpl(null, "dummy", false);
 
         final String name;
         final long handle;
         final boolean isBuiltin;
-        final boolean isJNI;
 
-        Unloader(String name, long handle, boolean isBuiltin, boolean isJNI) {
+        Unloader(String name, long handle, boolean isBuiltin) {
-            assert !isBuiltin || isJNI : "a builtin native library must be JNI library";
             if (handle == 0) {
                 throw new IllegalArgumentException(
                         "Invalid handle for native library " + name);
@@ -369,7 +362,6 @@ public final class NativeLibraries {
             this.name = name;
             this.handle = handle;
             this.isBuiltin = isBuiltin;
-            this.isJNI = isJNI;
         }
 
         @Override
@@ -377,12 +369,12 @@ public final class NativeLibraries {
             acquireNativeLibraryLock(name);
             try {
                 /* remove the native library name */
-                if (isJNI && !loadedLibraryNames.remove(name)) {
+                if (!loadedLibraryNames.remove(name)) {
                     throw new IllegalStateException(name + " has already been unloaded");
                 }
                 NativeLibraryContext.push(UNLOADER);
                 try {
-                    unload(name, isBuiltin, isJNI, handle);
+                    unload(name, isBuiltin, handle);
                 } finally {
                     NativeLibraryContext.pop();
                 }
@@ -524,12 +516,28 @@ public final class NativeLibraries {
         return NativeLibraryContext.peek().fromClass;
     }
 
-    // JNI FindClass expects the caller class if invoked from JNI_OnLoad
+    /*
-    // and JNI_OnUnload is NativeLibrary class
+     * Return true if the given library is successfully loaded.
+     * If the given library cannot be loaded for any reason,
+     * if throwExceptionIfFail is false, then this method returns false;
+     * otherwise, UnsatisfiedLinkError will be thrown.
+     *
+     * JNI FindClass expects the caller class if invoked from JNI_OnLoad
+     * and JNI_OnUnload is NativeLibrary class.
+     */
     private static native boolean load(NativeLibraryImpl impl, String name,
-                                       boolean isBuiltin, boolean isJNI,
+                                       boolean isBuiltin,
                                        boolean throwExceptionIfFail);
-    private static native void unload(String name, boolean isBuiltin, boolean isJNI, long handle);
+    /*
+     * Unload the named library.  JNI_OnUnload, if present, will be invoked
+     * before the native library is unloaded.
+     */
+    private static native void unload(String name, boolean isBuiltin, long handle);
     private static native String findBuiltinLib(String name);
-    private static native long findEntry0(NativeLibraryImpl lib, String name);
+
+    /*
+     * Returns the address of the named symbol defined in the given library.
+     * Returns 0 if not found.
+     */
+    static native long findEntry0(NativeLibrary lib, String name);
 }

src/java.base/share/classes/jdk/internal/loader/RawNativeLibraries.java

@@ -48,7 +48,7 @@ import static jdk.internal.loader.NativeLibraries.*;
  * 3. No relationship with class loaders.
  */
 public final class RawNativeLibraries {
-    final Map<String, NativeLibraryImpl> libraries = new ConcurrentHashMap<>();
+    final Map<String, RawNativeLibraryImpl> libraries = new ConcurrentHashMap<>();
     final Class<?> caller;
 
     private RawNativeLibraries(MethodHandles.Lookup trustedCaller) {
@@ -116,10 +116,10 @@ public final class RawNativeLibraries {
          return libraries.computeIfAbsent(pathname, this::get);
     }
 
-    private NativeLibraryImpl get(String pathname) {
+    private RawNativeLibraryImpl get(String pathname) {
-        NativeLibraryImpl lib = new NativeLibraryImpl(caller, pathname, false, false);
+        RawNativeLibraryImpl lib = new RawNativeLibraryImpl(caller, pathname);
         if (!lib.open()) {
-            return null;    // fail to open the native library
+            return null;
         }
         return lib;
     }
@@ -132,8 +132,49 @@ public final class RawNativeLibraries {
         if (!libraries.remove(lib.name(), lib)) {
             throw new IllegalArgumentException(lib.name() + " not loaded by this RawNativeLibraries instance");
         }
-        NativeLibraryImpl nl = (NativeLibraryImpl)lib;
+        RawNativeLibraryImpl nl = (RawNativeLibraryImpl)lib;
         nl.close();
     }
+
+    static class RawNativeLibraryImpl implements NativeLibrary {
+        // the name of the raw native library.
+        final String name;
+        // opaque handle to raw native library, used in native code.
+        long handle;
+
+        RawNativeLibraryImpl(Class<?> fromClass, String name) {
+            this.name = name;
+        }
+
+        @Override
+        public String name() {
+            return name;
+        }
+
+        @Override
+        public long find(String name) {
+            return findEntry0(this, name);
+        }
+
+        /*
+         * Loads the named native library.
+         */
+        boolean open() {
+            if (handle != 0) {
+                throw new InternalError("Native library " + name + " has been loaded");
+            }
+            return load0(this, name);
+        }
+
+        /*
+         * Close this native library.
+         */
+        void close() {
+            unload0(name, handle);
+        }
+    }
+
+    private static native boolean load0(RawNativeLibraryImpl impl, String name);
+    private static native void unload0(String name, long handle);
 }
 

src/java.base/share/native/libjava/NativeLibraries.c

@@ -25,13 +25,13 @@
 
 #include <stdlib.h>
 #include <assert.h>
+#include <string.h>
 
 #include "jni.h"
 #include "jni_util.h"
 #include "jlong.h"
 #include "jvm.h"
 #include "jdk_internal_loader_NativeLibraries.h"
-#include <string.h>
 
 typedef jint (JNICALL *JNI_OnLoad_t)(JavaVM *, void *);
 typedef void (JNICALL *JNI_OnUnload_t)(JavaVM *, void *);
@@ -40,18 +40,17 @@ static jfieldID handleID;
 static jfieldID jniVersionID;
 static void *procHandle;
 
-
 static jboolean initIDs(JNIEnv *env)
 {
     if (handleID == 0) {
-        jclass this =
+        jclass nlClz =
             (*env)->FindClass(env, "jdk/internal/loader/NativeLibraries$NativeLibraryImpl");
-        if (this == 0)
+        if (nlClz == 0)
             return JNI_FALSE;
-        handleID = (*env)->GetFieldID(env, this, "handle", "J");
+        handleID = (*env)->GetFieldID(env, nlClz, "handle", "J");
         if (handleID == 0)
             return JNI_FALSE;
-        jniVersionID = (*env)->GetFieldID(env, this, "jniVersion", "I");
+        jniVersionID = (*env)->GetFieldID(env, nlClz, "jniVersion", "I");
         if (jniVersionID == 0)
             return JNI_FALSE;
         procHandle = getProcessHandle();
@@ -109,12 +108,12 @@ static void *findJniFunction(JNIEnv *env, void *handle,
 /*
  * Class:     jdk_internal_loader_NativeLibraries
  * Method:    load
- * Signature: (Ljava/lang/String;ZZ)Z
+ * Signature: (Ljdk/internal/loader/NativeLibraries/NativeLibraryImpl;Ljava/lang/String;ZZ)Z
  */
 JNIEXPORT jboolean JNICALL
 Java_jdk_internal_loader_NativeLibraries_load
-  (JNIEnv *env, jobject this, jobject lib, jstring name,
+  (JNIEnv *env, jclass cls, jobject lib, jstring name,
-   jboolean isBuiltin, jboolean isJNI, jboolean throwExceptionIfFail)
+   jboolean isBuiltin, jboolean throwExceptionIfFail)
 {
     const char *cname;
     jint jniVersion;
@@ -129,53 +128,52 @@ Java_jdk_internal_loader_NativeLibraries_load
     if (cname == 0)
         return JNI_FALSE;
     handle = isBuiltin ? procHandle : JVM_LoadLibrary(cname, throwExceptionIfFail);
-    if (isJNI) {
+    if (handle) {
-        if (handle) {
+        JNI_OnLoad_t JNI_OnLoad;
-            JNI_OnLoad_t JNI_OnLoad;
+        JNI_OnLoad = (JNI_OnLoad_t)findJniFunction(env, handle,
-            JNI_OnLoad = (JNI_OnLoad_t)findJniFunction(env, handle,
+                                                   isBuiltin ? cname : NULL,
-                                                       isBuiltin ? cname : NULL,
+                                                   JNI_TRUE);
-                                                       JNI_TRUE);
+        if (JNI_OnLoad) {
-            if (JNI_OnLoad) {
+            JavaVM *jvm;
-                JavaVM *jvm;
+            (*env)->GetJavaVM(env, &jvm);
-                (*env)->GetJavaVM(env, &jvm);
+            jniVersion = (*JNI_OnLoad)(jvm, NULL);
-                jniVersion = (*JNI_OnLoad)(jvm, NULL);
-            } else {
-                jniVersion = 0x00010001;
-            }
-
-            cause = (*env)->ExceptionOccurred(env);
-            if (cause) {
-                (*env)->ExceptionClear(env);
-                (*env)->Throw(env, cause);
-                if (!isBuiltin) {
-                    JVM_UnloadLibrary(handle);
-                }
-                goto done;
-            }
-
-            if (!JVM_IsSupportedJNIVersion(jniVersion) ||
-                (isBuiltin && jniVersion < JNI_VERSION_1_8)) {
-                char msg[256];
-                jio_snprintf(msg, sizeof(msg),
-                             "unsupported JNI version 0x%08X required by %s",
-                             jniVersion, cname);
-                JNU_ThrowByName(env, "java/lang/UnsatisfiedLinkError", msg);
-                if (!isBuiltin) {
-                    JVM_UnloadLibrary(handle);
-                }
-                goto done;
-            }
-            (*env)->SetIntField(env, lib, jniVersionID, jniVersion);
         } else {
-            cause = (*env)->ExceptionOccurred(env);
+            jniVersion = 0x00010001;
-            if (cause) {
+        }
-                (*env)->ExceptionClear(env);
+
-                (*env)->SetLongField(env, lib, handleID, (jlong)0);
+        cause = (*env)->ExceptionOccurred(env);
-                (*env)->Throw(env, cause);
+        if (cause) {
+            (*env)->ExceptionClear(env);
+            (*env)->Throw(env, cause);
+            if (!isBuiltin) {
+                JVM_UnloadLibrary(handle);
             }
             goto done;
         }
+
+        if (!JVM_IsSupportedJNIVersion(jniVersion) ||
+            (isBuiltin && jniVersion < JNI_VERSION_1_8)) {
+            char msg[256];
+            jio_snprintf(msg, sizeof(msg),
+                         "unsupported JNI version 0x%08X required by %s",
+                         jniVersion, cname);
+            JNU_ThrowByName(env, "java/lang/UnsatisfiedLinkError", msg);
+            if (!isBuiltin) {
+                JVM_UnloadLibrary(handle);
+            }
+            goto done;
+        }
+        (*env)->SetIntField(env, lib, jniVersionID, jniVersion);
+    } else {
+        cause = (*env)->ExceptionOccurred(env);
+        if (cause) {
+            (*env)->ExceptionClear(env);
+            (*env)->SetLongField(env, lib, handleID, (jlong)0);
+            (*env)->Throw(env, cause);
+        }
+        goto done;
     }
+
     (*env)->SetLongField(env, lib, handleID, ptr_to_jlong(handle));
     loaded = JNI_TRUE;
 
@@ -187,11 +185,11 @@ Java_jdk_internal_loader_NativeLibraries_load
 /*
  * Class:     jdk_internal_loader_NativeLibraries
  * Method:    unload
- * Signature: (Ljava/lang/String;ZZJ)V
+ * Signature: (Ljava/lang/String;ZJ)V
  */
 JNIEXPORT void JNICALL
 Java_jdk_internal_loader_NativeLibraries_unload
-(JNIEnv *env, jclass cls, jstring name, jboolean isBuiltin, jboolean isJNI, jlong address)
+(JNIEnv *env, jclass cls, jstring name, jboolean isBuiltin, jlong address)
 {
     const char *onUnloadSymbols[] = JNI_ONUNLOAD_SYMBOLS;
     void *handle;
@@ -205,15 +203,14 @@ Java_jdk_internal_loader_NativeLibraries_unload
         return;
     }
     handle = jlong_to_ptr(address);
-    if (isJNI) {
+
-        JNI_OnUnload = (JNI_OnUnload_t )findJniFunction(env, handle,
+    JNI_OnUnload = (JNI_OnUnload_t )findJniFunction(env, handle,
-                                                        isBuiltin ? cname : NULL,
+                                                    isBuiltin ? cname : NULL,
-                                                        JNI_FALSE);
+                                                    JNI_FALSE);
-        if (JNI_OnUnload) {
+    if (JNI_OnUnload) {
-            JavaVM *jvm;
+        JavaVM *jvm;
-            (*env)->GetJavaVM(env, &jvm);
+        (*env)->GetJavaVM(env, &jvm);
-            (*JNI_OnUnload)(jvm, NULL);
+        (*JNI_OnUnload)(jvm, NULL);
-        }
     }
     if (!isBuiltin) {
         JVM_UnloadLibrary(handle);
@@ -225,11 +222,11 @@ Java_jdk_internal_loader_NativeLibraries_unload
 /*
  * Class:     jdk_internal_loader_NativeLibraries
  * Method:    findEntry0
- * Signature: (Ljava/lang/String;)J
+ * Signature: (Ljdk/internal/loader/NativeLibrary;Ljava/lang/String;)J
  */
 JNIEXPORT jlong JNICALL
 Java_jdk_internal_loader_NativeLibraries_findEntry0
-  (JNIEnv *env, jobject this, jobject lib, jstring name)
+  (JNIEnv *env, jclass cls, jobject lib, jstring name)
 {
     jlong handle;
     const char *cname;

src/java.base/share/native/libjava/RawNativeLibraries.c

@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+#include <stdlib.h>
+#include <assert.h>
+#include <string.h>
+
+#include "jni.h"
+#include "jni_util.h"
+#include "jlong.h"
+#include "jvm.h"
+#include "jdk_internal_loader_RawNativeLibraries.h"
+
+static jfieldID handleID;
+
+static jboolean initIDs(JNIEnv *env)
+{
+    if (handleID == 0) {
+        jclass rnlClz =
+            (*env)->FindClass(env, "jdk/internal/loader/RawNativeLibraries$RawNativeLibraryImpl");
+        if (rnlClz == 0)
+            return JNI_FALSE;
+        handleID = (*env)->GetFieldID(env, rnlClz, "handle", "J");
+        if (handleID == 0)
+            return JNI_FALSE;
+    }
+    return JNI_TRUE;
+}
+
+/*
+ * Class:     jdk_internal_loader_RawNativeLibraries
+ * Method:    Java_jdk_internal_loader_RawNativeLibraries_load0
+ * Signature: (Ljdk/internal/loader/RawNativeLibraries/RawNativeLibraryImpl;Ljava/lang/String;)Z
+ */
+JNIEXPORT jboolean JNICALL
+Java_jdk_internal_loader_RawNativeLibraries_load0
+  (JNIEnv *env, jclass cls, jobject lib, jstring name)
+{
+    const char *cname;
+    void * handle;
+
+    if (!initIDs(env))
+        return JNI_FALSE;
+
+    cname = JNU_GetStringPlatformChars(env, name, 0);
+    if (cname == 0)
+        return JNI_FALSE;
+    handle = JVM_LoadLibrary(cname, JNI_FALSE);
+    (*env)->SetLongField(env, lib, handleID, ptr_to_jlong(handle));
+
+    JNU_ReleaseStringPlatformChars(env, name, cname);
+    return handle != 0L;
+}
+
+/*
+ * Class:     jdk_internal_loader_RawNativeLibraries
+ * Method:    unload0
+ * Signature: (Ljava/lang/String;J)V
+ */
+JNIEXPORT void JNICALL Java_jdk_internal_loader_RawNativeLibraries_unload0
+  (JNIEnv *env, jclass cls, jstring name, jlong address)
+{
+    void *handle;
+    const char *cname;
+
+    if (!initIDs(env))
+        return;
+    cname = JNU_GetStringPlatformChars(env, name, 0);
+    if (cname == NULL) {
+        return;
+    }
+    handle = jlong_to_ptr(address);
+
+    JVM_UnloadLibrary(handle);
+    JNU_ReleaseStringPlatformChars(env, name, cname);
+}
+

test/jdk/jdk/internal/loader/NativeLibraries/Main.java

@@ -54,10 +54,13 @@ public class Main {
         System.loadLibrary(NativeLibrariesTest.LIB_NAME);
 
         // expect NativeLibraries to succeed even the library has been loaded by System::loadLibrary
-        test.load(true);
+        test.loadTestLibrary();
 
         // load zip library from JDK
-        test.load(System.mapLibraryName("zip"));
+        test.load(System.mapLibraryName("zip"), true /* succeed */);
+
+        // load non-existent library
+        test.load(System.mapLibraryName("NotExist"), false /* fail to load */);
     }
     /*
      * move p/Test.class out from classpath to the scratch directory

test/jdk/jdk/internal/loader/NativeLibraries/java.base/jdk/internal/loader/NativeLibrariesTest.java

@@ -61,7 +61,7 @@ public class NativeLibrariesTest implements Runnable {
      * Invoke by p.Test to load the same native library from different class loader
      */
     public void run() {
-        load(true); // expect loading of native library succeed
+        loadTestLibrary(); // expect loading of native library succeed
     }
 
     static Path libraryPath() {
@@ -101,18 +101,18 @@ public class NativeLibrariesTest implements Runnable {
         assertTrue(unloadedCount == 0, "Native library unloaded.  Expected: JNI_OnUnload not invoked");
     }
 
-    public void load(boolean succeed) {
+    public void loadTestLibrary() {
         NativeLibrary nl = nativeLibraries.load(libraryPath());
-        if (succeed) {
+        assertTrue(nl != null, "fail to load " + libraryPath());
-            assertTrue(nl != null, "fail to load library");
-        } else {
-            assertTrue(nl == null, "load library should fail");
-        }
     }
 
-    public void load(String pathname) {
+    public void load(String pathname, boolean succeed) {
         NativeLibrary nl = nativeLibraries.load(pathname);
-        assertTrue(nl != null, "fail to load zip library");
+        if (succeed) {
+            assertTrue(nl != null, "fail to load " + pathname);
+        } else {
+            assertTrue(nl == null, "expect to return null for " + pathname);
+        }
     }
 
     /*