Merge pull request #64 from babsingh/fix_LoadLibraryUnload

Fix LoadLibraryUnloadTest

test/jdk/java/io/ObjectStreamClass/TestOSCClassLoaderLeak.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.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -30,8 +30,8 @@
 import java.io.Serializable;
 import java.util.Arrays;
 import org.testng.annotations.Test;
-import static org.testng.Assert.assertNotNull;
 import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.assertFalse;
 
 import jdk.test.lib.util.ForceGC;
 
@@ -54,15 +54,14 @@ public void testClassLoaderLeak() throws Exception {
         objectStreamClass_MemoryLeakExample.toString();
 
         WeakReference<Object> myOwnClassLoaderWeakReference = new WeakReference<>(myOwnClassLoader);
-        assertNotNull(myOwnClassLoaderWeakReference.get());
+        assertFalse(myOwnClassLoaderWeakReference.refersTo(null));
         objectStreamClass_MemoryLeakExample = null;
         myOwnClassLoader = null;
         loadClass = null;
         con = null;
-        assertNotNull(myOwnClassLoaderWeakReference.get());
+        assertFalse(myOwnClassLoaderWeakReference.refersTo(null));
 
-        ForceGC gc = new ForceGC();
-        assertTrue(gc.await(() -> myOwnClassLoaderWeakReference.get() == null));
+        assertTrue(ForceGC.wait(() -> myOwnClassLoaderWeakReference.refersTo(null)));
     }
 }
 

test/jdk/java/lang/ClassLoader/loadLibraryUnload/LoadLibraryUnload.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.
  *
@@ -26,7 +26,8 @@
  * LoadLibraryUnload class calls ClassLoader.loadedLibrary from multiple threads
  */
 /*
- * @test
+ * The driver for this test is LoadLibraryUnloadTest.java.
+ *
  * @bug 8266310
  * @summary Loads a native library from multiple class loaders and multiple
  *          threads. This creates a race for loading the library. The winner
@@ -35,13 +36,13 @@
  *          loaded in a different class loader that won the race. The test
  *          checks that the loaded class is GC'ed, that means the class loader
  *          is GC'ed and the native library is unloaded.
- * @library /test/lib
- * @build LoadLibraryUnload p.Class1
- * @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.Reference;
+import java.lang.ref.ReferenceQueue;
 import java.lang.reflect.*;
 import java.lang.ref.WeakReference;
 import java.net.URL;
@@ -52,6 +53,8 @@
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 
+import jdk.test.lib.util.ForceGC;
+
 import p.Class1;
 
 public class LoadLibraryUnload {
@@ -82,11 +85,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 +100,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 +109,20 @@ 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];
+        final WeakReference<Object> wCanary[] = new WeakReference[LOADER_COUNT];
 
-        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], null);
+
+            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,21 +150,30 @@ 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();
-        ForceGC gc = new ForceGC();
-        if (!gc.await(() -> 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 (cleared)
+        boolean allClear = ForceGC.wait(() -> {
+            for (int i = 0; i < wCanary.length; i++) {
+                if (!wCanary[i].refersTo(null)) {
+                    return false;
+                }
+            }
+            return true;
+        });
+        Asserts.assertTrue(allClear, "Not all WeakReferences cleared");
+
+        // Ensure the WeakReferences are strongly referenced until they can be dequeued
+        Reference.reachabilityFence(wCanary);
     }
 }

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,18 +28,18 @@
  */
 /*
  * @test
- * @bug 8266310
+ * @bug 8266310 8289919 8293282
  * @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.
  * @library /test/lib
  * @build LoadLibraryUnload p.Class1
- * @run main/othervm/native -Xcheck:jni LoadLibraryUnloadTest
+ * @run main/othervm/native LoadLibraryUnloadTest
  */
 
 import jdk.test.lib.Asserts;
 import jdk.test.lib.JDKToolFinder;
-import jdk.test.lib.process.*;
+import jdk.test.lib.process.OutputAnalyzer;
 
 import java.lang.ProcessBuilder;
 import java.lang.Process;
@@ -50,7 +50,6 @@ public class LoadLibraryUnloadTest {
 
     private static String testClassPath = System.getProperty("test.classes");
     private static String testLibraryPath = System.getProperty("test.nativepath");
-    private static String classPathSeparator = System.getProperty("path.separator");
 
     private static Process runJavaCommand(String... command) throws Throwable {
         String java = JDKToolFinder.getJDKTool("java");
@@ -102,5 +101,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);
 }

test/jdk/java/lang/ClassLoader/nativeLibrary/NativeLibraryTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 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
@@ -66,9 +66,8 @@ public static void main(String... args) throws Exception {
             // Unload the class loader and native library, and give the Cleaner
             // thread a chance to unload the native library.
             // unloadedCount is incremented when the native library is unloaded.
-            ForceGC gc = new ForceGC();
             final int finalCount = count;
-            if (!gc.await(() -> finalCount == unloadedCount)) {
+            if (!ForceGC.wait(() -> finalCount == unloadedCount)) {
                 throw new RuntimeException("Expected unloaded=" + count +
                     " but got=" + unloadedCount);
             }

test/jdk/java/lang/invoke/defineHiddenClass/UnloadingTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 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
@@ -214,18 +214,15 @@ private HiddenClassUnloader(Class<?> hc) {
         }
 
         void unload() {
-            // Force garbage collection to trigger unloading of class loader and native library
-            ForceGC gc = new ForceGC();
-            assertTrue(gc.await(() -> weakRef.get() == null));
-
-            if (weakRef.get() != null) {
+            // Force garbage collection to trigger unloading of class loader
+            // and native library.
+            if (!ForceGC.wait(() -> weakRef.refersTo(null))) {
                 throw new RuntimeException("loader " + " not unloaded!");
             }
         }
 
         boolean tryUnload() {
-            ForceGC gc = new ForceGC();
-            return gc.await(() -> weakRef.get() == null);
+            return ForceGC.wait(() -> weakRef.refersTo(null));
         }
     }
 

test/jdk/java/lang/reflect/callerCache/ReflectionCallerCacheTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 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
@@ -107,9 +107,7 @@ private void load(String classname) throws Exception {
         WeakReference<?> weakLoader = loadAndRunClass(classname);
 
         // Force garbage collection to trigger unloading of class loader
-        new ForceGC().await(() -> weakLoader.get() == null);
-
-        if (weakLoader.get() != null) {
+        if (!ForceGC.wait(() -> weakLoader.refersTo(null))) {
             throw new RuntimeException("Class " + classname + " not unloaded!");
         }
     }

test/jdk/javax/security/auth/callback/PasswordCallback/CheckCleanerBound.java

@@ -49,8 +49,7 @@ public static void main(String[] args) throws Exception {
         // Check if the object has been collected.  The collection will not
         // happen if the cleaner implementation in PasswordCallback is bound
         // to the PasswordCallback object.
-        ForceGC gc = new ForceGC();
-        if (!gc.await(() -> weakRef.get() == null)) {
+        if (!ForceGC.wait(() -> weakRef.refersTo(null))) {
             throw new RuntimeException(
                 "PasswordCallback object is not released");
         }

test/jdk/sun/security/jgss/GssContextCleanup.java

@@ -54,8 +54,7 @@ public static void main(String[] args) throws Exception {
         context = null;
 
         // Check if the object has been collected.
-        ForceGC gc = new ForceGC();
-        if (!gc.await(() -> weakRef.get() == null)) {
+        if (!ForceGC.wait(() -> weakRef.refersTo(null))) {
             throw new RuntimeException("GSSContext object is not released");
         }
     }

test/jdk/sun/security/jgss/GssNameCleanup.java

@@ -55,8 +55,7 @@ public static void main(String[] args) throws Exception {
             name = null;
 
             // Check if the object has been collected.
-            ForceGC gc = new ForceGC();
-            if (!gc.await(() -> weakRef.get() == null)) {
+            if (!ForceGC.wait(() -> weakRef.refersTo(null))) {
                 throw new RuntimeException("GSSName object is not released");
             }
         } catch (GSSException gsse) {

test/jdk/sun/security/pkcs11/Provider/MultipleLogins.java

@@ -89,10 +89,8 @@ public static void main(String[] args) throws Exception {
             Security.removeProvider(providers[i].getName());
             providers[i] = null;
 
-            ForceGC gc = new ForceGC();
             int finalI = i;
-            gc.await(() -> weakRef[finalI].get() == null);
-            if (!weakRef[i].refersTo(null)) {
+            if (!ForceGC.wait(() -> weakRef[finalI].refersTo(null))) {
                 throw new RuntimeException("Expected SunPKCS11 Provider to be GC'ed..");
             }
         }