8175249: VMThread::run fails in VerifyBeforeExit : Universe::verify

Protection domain package access cache needs to be walked in unloading

Reviewed-by: dholmes, jiangli, zgu, kbarrett

src/hotspot/share/classfile/classLoaderData.cpp

@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 2012, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2018, 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
@@ -1292,7 +1292,7 @@ bool ClassLoaderDataGraph::do_unloading(BoolObjectClosure* is_alive_closure,
       // Remove entries in the dictionary of live class loader that have
       // initiated loading classes in a dead class loader.
       if (data->dictionary() != NULL) {
-        data->dictionary()->do_unloading();
+        data->dictionary()->do_unloading(is_alive_closure);
       }
       // Walk a ModuleEntry's reads, and a PackageEntry's exports
       // lists to determine if there are modules on those lists that are now

src/hotspot/share/classfile/dictionary.cpp

@@ -212,8 +212,44 @@ void DictionaryEntry::add_protection_domain(Dictionary* dict, Handle protection_
   }
 }
 
+// During class loading we may have cached a protection domain that has
+// since been unreferenced, so this entry should be cleared.
+void Dictionary::clean_cached_protection_domains(BoolObjectClosure* is_alive, DictionaryEntry* probe) {
+  assert_locked_or_safepoint(SystemDictionary_lock);
 
-void Dictionary::do_unloading() {
+  ProtectionDomainEntry* current = probe->pd_set();
+  ProtectionDomainEntry* prev = NULL;
+  while (current != NULL) {
+    if (!is_alive->do_object_b(current->object_no_keepalive())) {
+      LogTarget(Debug, protectiondomain) lt;
+      if (lt.is_enabled()) {
+        ResourceMark rm;
+        // Print out trace information
+        LogStream ls(lt);
+        ls.print_cr("PD in set is not alive:");
+        ls.print("class loader: "); loader_data()->class_loader()->print_value_on(&ls);
+        ls.print(" protection domain: "); current->object_no_keepalive()->print_value_on(&ls);
+        ls.print(" loading: "); probe->instance_klass()->print_value_on(&ls);
+        ls.cr();
+      }
+      if (probe->pd_set() == current) {
+        probe->set_pd_set(current->next());
+      } else {
+        assert(prev != NULL, "should be set by alive entry");
+        prev->set_next(current->next());
+      }
+      ProtectionDomainEntry* to_delete = current;
+      current = current->next();
+      delete to_delete;
+    } else {
+      prev = current;
+      current = current->next();
+    }
+  }
+}
+
+
+void Dictionary::do_unloading(BoolObjectClosure* is_alive) {
   assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint");
 
   // The NULL class loader doesn't initiate loading classes from other class loaders
@@ -239,6 +275,8 @@ void Dictionary::do_unloading() {
         free_entry(probe);
         continue;
       }
+      // Clean pd_set
+      clean_cached_protection_domains(is_alive, probe);
       p = probe->next_addr();
     }
   }
@@ -412,6 +450,10 @@ void Dictionary::add_protection_domain(int index, unsigned int hash,
 
   entry->add_protection_domain(this, protection_domain);
 
+#ifdef ASSERT
+  assert(loader_data() != ClassLoaderData::the_null_class_loader_data(), "doesn't make sense");
+#endif
+
   assert(entry->contains_protection_domain(protection_domain()),
          "now protection domain should be present");
 }

src/hotspot/share/classfile/dictionary.hpp

@@ -53,6 +53,8 @@ class Dictionary : public Hashtable<InstanceKlass*, mtClass> {
 
   DictionaryEntry* get_entry(int index, unsigned int hash, Symbol* name);
 
+  void clean_cached_protection_domains(BoolObjectClosure* is_alive, DictionaryEntry* probe);
+
 protected:
   static size_t entry_size();
 public:
@@ -84,7 +86,7 @@ public:
   void remove_classes_in_error_state();
 
   // Unload classes whose defining loaders are unloaded
-  void do_unloading();
+  void do_unloading(BoolObjectClosure* is_alive);
 
   // Protection domains
   InstanceKlass* find(unsigned int hash, Symbol* name, Handle protection_domain);

src/hotspot/share/classfile/protectionDomainCache.hpp

@@ -113,6 +113,7 @@ class ProtectionDomainEntry :public CHeapObj<mtClass> {
   }
 
   ProtectionDomainEntry* next() { return _next; }
+  void set_next(ProtectionDomainEntry* entry) { _next = entry; }
   oop object();
   oop object_no_keepalive();
 };

test/hotspot/jtreg/runtime/Dictionary/ClassForName.java

@@ -0,0 +1,47 @@
+/*
+ * Copyright (c) 2018, 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.
+ *
+ * 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.
+ */
+
+import java.net.URLClassLoader;
+
+/*
+ * This class is loaded by the custom URLClassLoader, and then calls
+ * Class.forName() with the protection domain for the checkPackageAccess
+ * call created from the code source jar file.
+ */
+public class ClassForName {
+    static {
+        if (!(ClassForName.class.getClassLoader() instanceof URLClassLoader)) {
+            throw new RuntimeException("Supposed to be loaded by URLClassLoader");
+        }
+    }
+
+    public ClassForName() {
+        try {
+            // class_loader = URLClassLoader, protection_domain = ClassForName.getProtectionDomain()
+            Class.forName(java.util.List.class.getName(), false,
+                          ClassLoader.getSystemClassLoader());
+        } catch (Throwable e) {
+            e.printStackTrace();
+        }
+    }
+}

test/hotspot/jtreg/runtime/Dictionary/ProtectionDomainCacheTest.java

@@ -0,0 +1,87 @@
+/*
+ * Copyright (c) 2018, 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.
+ *
+ * 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.
+ */
+
+/*
+ * @test
+ * @bug 8151486
+ * @summary Call Class.forName() on the system classloader from a class loaded
+ *          from a custom classloader, using the current class's protection domain.
+ * @library /test/jdk/lib/testlibrary
+ * @build jdk.testlibrary.Utils JarUtils
+ * @build ClassForName ProtectionDomainCacheTest
+ * @run main/othervm/policy=test.policy -XX:+UnlockDiagnosticVMOptions -XX:VerifySubSet=dictionary -XX:+VerifyAfterGC -Xlog:gc+verify=debug,protectiondomain=trace,class+unload:gc.log -Djava.security.manager ProtectionDomainCacheTest
+ */
+
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import jdk.testlibrary.Utils;
+
+/*
+ * Create .jar, load ClassForName from .jar using a URLClassLoader
+ */
+public class ProtectionDomainCacheTest {
+    private static final long TIMEOUT = (long)(5000.0 * Utils.TIMEOUT_FACTOR);
+    private static final String TESTCLASSES = System.getProperty("test.classes", ".");
+    private static final String CLASSFILENAME = "ClassForName.class";
+
+    // Use a new classloader to load the ClassForName class.
+    public static void loadAndRun(Path jarFilePath)
+            throws Exception {
+        ClassLoader classLoader = new URLClassLoader(
+                new URL[]{jarFilePath.toUri().toURL()}) {
+            @Override public String toString() { return "LeakedClassLoader"; }
+        };
+
+        Class<?> loadClass = Class.forName("ClassForName", true, classLoader);
+        loadClass.newInstance();
+
+        System.out.println("returning : " + classLoader);
+    }
+
+    public static void main(final String[] args) throws Exception {
+        // Create a temporary .jar file containing ClassForName.class
+        Path testClassesDir = Paths.get(TESTCLASSES);
+        Path jarFilePath = Files.createTempFile("cfn", ".jar");
+        JarUtils.createJarFile(jarFilePath, testClassesDir, CLASSFILENAME);
+        jarFilePath.toFile().deleteOnExit();
+
+        // Remove the ClassForName.class file that jtreg built, to make sure
+        // we're loading from the tmp .jar
+        Path classFile = FileSystems.getDefault().getPath(TESTCLASSES,
+                                                          CLASSFILENAME);
+        Files.delete(classFile);
+
+        loadAndRun(jarFilePath);
+
+        // Give the GC a chance to unload protection domains
+        for (int i = 0; i < 100; i++) {
+            System.gc();
+        }
+        System.out.println("All Classloaders and protection domain cache entries successfully unloaded");
+    }
+}

test/hotspot/jtreg/runtime/Dictionary/test.policy

@@ -0,0 +1,6 @@
+grant {
+  permission java.io.FilePermission "<<ALL FILES>>", "read, write, delete";
+  permission java.lang.RuntimePermission "createClassLoader";
+  permission java.lang.RuntimePermission "getClassLoader";
+  permission java.util.PropertyPermission "*", "read"; /* for Utils */
+};