8289919: [test] LoadLibraryUnloadTest.java failed with "Failed to unl…

…oad native library"

Reviewed-by: mchung

test/jdk/java/lang/ClassLoader/loadLibraryUnload/LoadLibraryUnload.java

@@ -40,8 +40,10 @@
  * @run main/othervm/native -Xcheck:jni LoadLibraryUnload
  */
 import jdk.test.lib.Asserts;
-import jdk.test.lib.util.ForceGC;
+import jdk.test.lib.Utils;
+
 import java.lang.*;
+import java.lang.ref.ReferenceQueue;
 import java.lang.reflect.*;
 import java.lang.ref.WeakReference;
 import java.net.URL;
@@ -82,11 +84,13 @@ public Class<?> loadClass(String name) throws ClassNotFoundException {
     private static class LoadLibraryFromClass implements Runnable {
         Object object;
         Method method;
+        Object canary;
 
-        public LoadLibraryFromClass(Class<?> fromClass) {
+        public LoadLibraryFromClass(Class<?> fromClass, Object canary) {
             try {
                 this.object = fromClass.newInstance();
-                this.method = fromClass.getDeclaredMethod("loadLibrary");
+                this.method = fromClass.getDeclaredMethod("loadLibrary", Object.class);
+                this.canary = canary;
             } catch (ReflectiveOperationException roe) {
                 throw new RuntimeException(roe);
             }
@@ -95,7 +99,7 @@ public LoadLibraryFromClass(Class<?> fromClass) {
         @Override
         public void run() {
             try {
-                method.invoke(object);
+                method.invoke(object, canary);
             } catch (ReflectiveOperationException roe) {
                 throw new RuntimeException(roe);
             }
@@ -104,15 +108,21 @@ public void run() {
 
     public static void main(String[] args) throws Exception {
 
-        Class<?> clazz = null;
+        int LOADER_COUNT = 5;
         List<Thread> threads = new ArrayList<>();
+        Object[] canary = new Object[LOADER_COUNT];
+        WeakReference<Object> wCanary[] = new WeakReference[LOADER_COUNT];
+        ReferenceQueue<Object> refQueue = new ReferenceQueue<>();
 
-        for (int i = 0 ; i < 5 ; i++) {
-            // 5 loaders and 10 threads in total.
+        for (int i = 0 ; i < LOADER_COUNT ; i++) {
+            // LOADER_COUNT loaders and 2X threads in total.
             // winner loads the library in 2 threads
-            clazz = new TestLoader().loadClass("p.Class1");
-            threads.add(new Thread(new LoadLibraryFromClass(clazz)));
-            threads.add(new Thread(new LoadLibraryFromClass(clazz)));
+            canary[i] = new Object();
+            wCanary[i] = new WeakReference<>(canary[i], refQueue);
+
+            Class<?> clazz = new TestLoader().loadClass("p.Class1");
+            threads.add(new Thread(new LoadLibraryFromClass(clazz, canary[i])));
+            threads.add(new Thread(new LoadLibraryFromClass(clazz, canary[i])));
         }
 
         final Set<Throwable> exceptions = ConcurrentHashMap.newKeySet();
@@ -140,20 +150,27 @@ public static void main(String[] args) throws Exception {
                 .reduce(true, (i, a) -> i && a);
 
         // expect exactly 8 errors
-        Asserts.assertTrue(exceptions.size() == 8,
-                "Expected to see 8 failing threads");
+        int expectedErrorCount = (LOADER_COUNT - 1) * 2;
+        Asserts.assertTrue(exceptions.size() == expectedErrorCount,
+                "Expected to see " + expectedErrorCount + " failing threads");
 
         Asserts.assertTrue(allAreUnsatisfiedLinkError,
                 "All errors have to be UnsatisfiedLinkError");
 
-        WeakReference<Class<?>> wClass = new WeakReference<>(clazz);
-
         // release strong refs
-        clazz = null;
         threads = null;
+        canary = null;
         exceptions.clear();
-        if (!ForceGC.wait(() -> wClass.refersTo(null))) {
-            throw new RuntimeException("Class1 hasn't been GC'ed");
+
+        // Wait for the canary for each of the libraries to be GC'd
+        // before exiting the test.
+        for (int i = 0; i < LOADER_COUNT; i++) {
+            System.gc();
+            var res = refQueue.remove(Utils.adjustTimeout(5 * 1000L));
+            System.out.println(i + " dequeued: " + res);
+            if (res == null) {
+                Asserts.fail("Too few cleared WeakReferences");
+            }
         }
     }
 }

test/jdk/java/lang/ClassLoader/loadLibraryUnload/LoadLibraryUnloadTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2022, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2021, BELLSOFT. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -28,7 +28,7 @@
  */
 /*
  * @test
- * @bug 8266310
+ * @bug 8266310 8289919
  * @summary Checks that JNI_OnLoad is invoked only once when multiple threads
  *          call System.loadLibrary concurrently, and JNI_OnUnload is invoked
  *          when the native library is loaded from a custom class loader.
@@ -102,5 +102,8 @@ public static void main(String[] args) throws Throwable {
         Asserts.assertTrue(
                 countLines(outputAnalyzer, "Native library unloaded.") == refCount,
                 "Failed to unload native library");
+
+        Asserts.assertEquals(0, outputAnalyzer.getExitValue(),
+                "LoadLibraryUnload exit value not zero");
     }
 }

test/jdk/java/lang/ClassLoader/loadLibraryUnload/libloadLibraryUnload.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2022, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2021, BELLSOFT. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -25,6 +25,8 @@
 #include <stdio.h>
 #include "jni.h"
 
+static volatile jobject ref = NULL;
+
 JNIEXPORT jint JNICALL
 JNI_OnLoad(JavaVM *vm, void *reserved)
 {
@@ -40,9 +42,25 @@ JNI_OnLoad(JavaVM *vm, void *reserved)
     return JNI_VERSION_1_2;
 }
 
+JNIEXPORT void JNICALL Java_p_Class1_setRef(JNIEnv *env, jobject this, jobject obj) {
+    if (ref == NULL) {
+        // Only create a single GlobalRef
+        ref = (*env)->NewGlobalRef(env, obj);
+        printf("GlobalRef created\n");
+    }
+}
+
 JNIEXPORT void JNICALL
 JNI_OnUnload(JavaVM *vm, void *reserved) {
 
+    if (ref != NULL) {
+        JNIEnv *env = NULL;
+        if ((*vm)->GetEnv(vm, (void**) &env, JNI_VERSION_1_2) != JNI_OK) {
+            return; /* JNI version not supported */
+        }
+        (*env)->DeleteGlobalRef(env, ref);
+        printf("GlobalRef deleted\n");
+    }
     printf("Native library unloaded.\n");
     fflush(stdout);
 }

test/jdk/java/lang/ClassLoader/loadLibraryUnload/p/Class1.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2022, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2021, BELLSOFT. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -33,8 +33,18 @@ public Class1() {
     }
 
     // method called from java threads
-    public void loadLibrary() throws Exception {
+    public void loadLibrary(Object obj) throws Exception {
         System.loadLibrary("loadLibraryUnload");
         System.out.println("Native library loaded from Class1.");
+        synchronized (Class1.class) {
+            setRef(obj);
+        }
     }
+
+    /**
+     * Native method to store an object ref in a native Global reference
+     * to be cleared when the library is unloaded.
+     * @param obj an object
+     */
+    private static native void setRef(Object obj);
 }