stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8218266: G1 crash in AccessInternal::PostRuntimeDispatch

Browse Source 
Protection_domains can be unloaded in the dictionary pd_set.

Reviewed-by: zgu, hseigel
This commit is contained in:
Coleen Phillimore 2019-02-28 16:30:31 -05:00
parent 29d842b5a0
commit 84a6e34fb0
8 changed files with 174 additions and 108 deletions

90
src/hotspot/share/classfile/dictionary.cpp

@ -24,7 +24,7 @@
#include "precompiled.hpp"
#include "classfile/classLoaderData.inline.hpp"
#include "classfile/dictionary.inline.hpp"
#include "classfile/dictionary.hpp"
#include "classfile/protectionDomainCache.hpp"
#include "classfile/systemDictionary.hpp"
#include "logging/log.hpp"
@ -35,6 +35,7 @@
#include "oops/oop.inline.hpp"
#include "runtime/atomic.hpp"
#include "runtime/orderAccess.hpp"
#include "runtime/mutexLocker.hpp"
#include "runtime/safepointVerifiers.hpp"
#include "utilities/hashtable.inline.hpp"
@ -80,6 +81,8 @@ DictionaryEntry* Dictionary::new_entry(unsigned int hash, InstanceKlass* klass)
void Dictionary::free_entry(DictionaryEntry* entry) {
// avoid recursion when deleting linked list
// pd_set is accessed during a safepoint.
// This doesn't require a lock because nothing is reading this
// entry anymore. The ClassLoader is dead.
while (entry->pd_set() != NULL) {
ProtectionDomainEntry* to_delete = entry->pd_set();
entry->set_pd_set(to_delete->next());
@ -146,11 +149,14 @@ bool Dictionary::resize_if_needed() {
}
bool DictionaryEntry::contains_protection_domain(oop protection_domain) const {
// Lock the pd_set list. This lock cannot safepoint since the caller holds
// a Dictionary entry, which can be moved if the Dictionary is resized.
MutexLockerEx ml(ProtectionDomainSet_lock, Mutex::_no_safepoint_check_flag);
#ifdef ASSERT
if (oopDesc::equals(protection_domain, instance_klass()->protection_domain())) {
// Ensure this doesn't show up in the pd_set (invariant)
bool in_pd_set = false;
for (ProtectionDomainEntry* current = pd_set_acquire();
for (ProtectionDomainEntry* current = pd_set();
current != NULL;
current = current->next()) {
if (oopDesc::equals(current->object_no_keepalive(), protection_domain)) {
@ -170,7 +176,7 @@ bool DictionaryEntry::contains_protection_domain(oop protection_domain) const {
return true;
}
for (ProtectionDomainEntry* current = pd_set_acquire();
for (ProtectionDomainEntry* current = pd_set();
current != NULL;
current = current->next()) {
if (oopDesc::equals(current->object_no_keepalive(), protection_domain)) return true;
@ -183,13 +189,12 @@ void DictionaryEntry::add_protection_domain(Dictionary* dict, Handle protection_
assert_locked_or_safepoint(SystemDictionary_lock);
if (!contains_protection_domain(protection_domain())) {
ProtectionDomainCacheEntry* entry = SystemDictionary::cache_get(protection_domain);
// The pd_set in the dictionary entry is protected by a low level lock.
// With concurrent PD table cleanup, these links could be broken.
MutexLockerEx ml(ProtectionDomainSet_lock, Mutex::_no_safepoint_check_flag);
ProtectionDomainEntry* new_head =
new ProtectionDomainEntry(entry, pd_set());
// Warning: Preserve store ordering. The SystemDictionary is read
// without locks. The new ProtectionDomainEntry must be
// complete before other threads can be allowed to see it
// via a store to _pd_set.
release_set_pd_set(new_head);
set_pd_set(new_head);
}
LogTarget(Trace, protectiondomain) lt;
if (lt.is_enabled()) {
@ -348,6 +353,56 @@ bool Dictionary::is_valid_protection_domain(unsigned int hash,
return entry->is_valid_protection_domain(protection_domain);
}
// 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() {
assert_locked_or_safepoint(SystemDictionary_lock);
if (loader_data()->is_the_null_class_loader_data()) {
// Classes in the boot loader are not loaded with protection domains
return;
}
for (int index = 0; index < table_size(); index++) {
for (DictionaryEntry* probe = bucket(index);
probe != NULL;
probe = probe->next()) {
Klass* e = probe->instance_klass();
MutexLockerEx ml(ProtectionDomainSet_lock, Mutex::_no_safepoint_check_flag);
ProtectionDomainEntry* current = probe->pd_set();
ProtectionDomainEntry* prev = NULL;
while (current != NULL) {
if (current->object_no_keepalive() == NULL) {
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(" 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();
}
}
}
}
}
SymbolPropertyTable::SymbolPropertyTable(int table_size)
: Hashtable<Symbol*, mtSymbol>(table_size, sizeof(SymbolPropertyEntry))
{
@ -404,6 +459,25 @@ void SymbolPropertyTable::methods_do(void f(Method*)) {
}
}
void DictionaryEntry::verify_protection_domain_set() {
MutexLockerEx ml(ProtectionDomainSet_lock, Mutex::_no_safepoint_check_flag);
for (ProtectionDomainEntry* current = pd_set(); // accessed at a safepoint
current != NULL;
current = current->_next) {
guarantee(oopDesc::is_oop_or_null(current->_pd_cache->object_no_keepalive()), "Invalid oop");
}
}
void DictionaryEntry::print_count(outputStream *st) {
MutexLockerEx ml(ProtectionDomainSet_lock, Mutex::_no_safepoint_check_flag);
int count = 0;
for (ProtectionDomainEntry* current = pd_set(); // accessed inside SD lock
current != NULL;
current = current->_next) {
count++;
}
st->print_cr("pd set count = #%d", count);
}
// ----------------------------------------------------------------------------

30
src/hotspot/share/classfile/dictionary.hpp

@ -59,8 +59,6 @@ public:
static bool does_any_dictionary_needs_resizing();
bool resize_if_needed();
DictionaryEntry* new_entry(unsigned int hash, InstanceKlass* klass);
void add_klass(unsigned int hash, Symbol* class_name, InstanceKlass* obj);
InstanceKlass* find_class(int index, unsigned int hash, Symbol* name);
@ -70,7 +68,7 @@ public:
void all_entries_do(KlassClosure* closure);
void classes_do(MetaspaceClosure* it);
void unlink();
void clean_cached_protection_domains();
// Protection domains
InstanceKlass* find(unsigned int hash, Symbol* name, Handle protection_domain);
@ -83,6 +81,10 @@ public:
void print_on(outputStream* st) const;
void verify();
private:
DictionaryEntry* new_entry(unsigned int hash, InstanceKlass* klass);
DictionaryEntry* bucket(int i) const {
return (DictionaryEntry*)Hashtable<InstanceKlass*, mtClass>::bucket(i);
}
@ -151,9 +153,6 @@ class DictionaryEntry : public HashtableEntry<InstanceKlass*, mtClass> {
ProtectionDomainEntry* pd_set() const { return _pd_set; }
void set_pd_set(ProtectionDomainEntry* new_head) { _pd_set = new_head; }
ProtectionDomainEntry* pd_set_acquire() const;
void release_set_pd_set(ProtectionDomainEntry* new_head);
// Tells whether the initiating class' protection domain can access the klass in this entry
bool is_valid_protection_domain(Handle protection_domain) {
if (!ProtectionDomainVerification) return true;
@ -164,29 +163,14 @@ class DictionaryEntry : public HashtableEntry<InstanceKlass*, mtClass> {
: contains_protection_domain(protection_domain());
}
void verify_protection_domain_set() {
for (ProtectionDomainEntry* current = pd_set(); // accessed at a safepoint
current != NULL;
current = current->_next) {
guarantee(oopDesc::is_oop_or_null(current->_pd_cache->object_no_keepalive()), "Invalid oop");
}
}
void verify_protection_domain_set();
bool equals(const Symbol* class_name) const {
InstanceKlass* klass = (InstanceKlass*)literal();
return (klass->name() == class_name);
}
void print_count(outputStream *st) {
int count = 0;
for (ProtectionDomainEntry* current = pd_set(); // accessed inside SD lock
current != NULL;
current = current->_next) {
count++;
}
st->print_cr("pd set count = #%d", count);
}
void print_count(outputStream *st);
void verify();
};

39
src/hotspot/share/classfile/dictionary.inline.hpp

@ -1,39 +0,0 @@
/*
* Copyright (c) 2018, 2019, 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.
*
*/
#ifndef SHARE_CLASSFILE_DICTIONARY_INLINE_HPP
#define SHARE_CLASSFILE_DICTIONARY_INLINE_HPP
#include "classfile/dictionary.hpp"
#include "runtime/orderAccess.hpp"
inline ProtectionDomainEntry* DictionaryEntry::pd_set_acquire() const {
return OrderAccess::load_acquire(&_pd_set);
}
inline void DictionaryEntry::release_set_pd_set(ProtectionDomainEntry* new_head) {
OrderAccess::release_store(&_pd_set, new_head);
}
#endif // SHARE_CLASSFILE_DICTIONARY_INLINE_HPP

24
src/hotspot/share/classfile/protectionDomainCache.cpp

@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2019, 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
@ -23,6 +23,8 @@
*/
#include "precompiled.hpp"
#include "classfile/classLoaderDataGraph.hpp"
#include "classfile/dictionary.hpp"
#include "classfile/protectionDomainCache.hpp"
#include "classfile/systemDictionary.hpp"
#include "logging/log.hpp"
@ -54,7 +56,27 @@ void ProtectionDomainCacheTable::trigger_cleanup() {
Service_lock->notify_all();
}
class CleanProtectionDomainEntries : public CLDClosure {
void do_cld(ClassLoaderData* data) {
Dictionary* dictionary = data->dictionary();
if (dictionary != NULL) {
dictionary->clean_cached_protection_domains();
}
}
};
void ProtectionDomainCacheTable::unlink() {
{
// First clean cached pd lists in loaded CLDs
// It's unlikely, but some loaded classes in a dictionary might
// point to a protection_domain that has been unloaded.
// The dictionary pd_set points at entries in the ProtectionDomainCacheTable.
MutexLocker ml(ClassLoaderDataGraph_lock);
MutexLocker mldict(SystemDictionary_lock); // need both.
CleanProtectionDomainEntries clean;
ClassLoaderDataGraph::loaded_cld_do(&clean);
}
MutexLocker ml(SystemDictionary_lock);
int oops_removed = 0;
for (int i = 0; i < table_size(); ++i) {

2
src/hotspot/share/runtime/mutexLocker.cpp

@ -39,6 +39,7 @@
Mutex* Patching_lock = NULL;
Monitor* SystemDictionary_lock = NULL;
Mutex* ProtectionDomainSet_lock = NULL;
Mutex* SharedDictionary_lock = NULL;
Mutex* Module_lock = NULL;
Mutex* CompiledIC_lock = NULL;
@ -254,6 +255,7 @@ void mutex_init() {
def(JmethodIdCreation_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_always); // used for creating jmethodIDs.
def(SystemDictionary_lock , PaddedMonitor, leaf, true, Monitor::_safepoint_check_always); // lookups done by VM thread
def(ProtectionDomainSet_lock , PaddedMutex , leaf-1, true, Monitor::_safepoint_check_never);
def(SharedDictionary_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_always); // lookups done by VM thread
def(Module_lock , PaddedMutex , leaf+2, true, Monitor::_safepoint_check_always);
def(InlineCacheBuffer_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_never);

1
src/hotspot/share/runtime/mutexLocker.hpp

@ -33,6 +33,7 @@
extern Mutex* Patching_lock; // a lock used to guard code patching of compiled code
extern Monitor* SystemDictionary_lock; // a lock on the system dictionary
extern Mutex* ProtectionDomainSet_lock; // a lock on the pd_set list in the system dictionary
extern Mutex* SharedDictionary_lock; // a lock on the CDS shared dictionary
extern Mutex* Module_lock; // a lock on module and package related data structures
extern Mutex* CompiledIC_lock; // a lock used to guard compiled IC patching and access

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

@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2019, 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
@ -23,14 +23,15 @@
/*
* @test
* @bug 8151486
* @bug 8151486 8218266
* @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/lib
* @modules java.base/jdk.internal.misc
* @build jdk.test.lib.Utils
* jdk.test.lib.util.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
* @run main/othervm/policy=test.policy -Djava.security.manager ProtectionDomainCacheTest
*/
import java.net.URL;
@ -42,48 +43,69 @@ import java.nio.file.Paths;
import java.util.List;
import jdk.test.lib.Utils;
import jdk.test.lib.util.JarUtils;
import java.io.File;
import jdk.test.lib.process.OutputAnalyzer;
import jdk.test.lib.process.ProcessTools;
/*
* 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";
static class Test {
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"; }
};
// 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();
Class<?> loadClass = Class.forName("ClassForName", true, classLoader);
loadClass.newInstance();
System.out.println("returning : " + classLoader);
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");
}
}
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");
public static void main(String args[]) throws Exception {
ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
"-Djava.security.policy==" + System.getProperty("test.src") + File.separator + "test.policy",
"-Dtest.classes=" + System.getProperty("test.classes", "."),
"-XX:+UnlockDiagnosticVMOptions",
"-XX:VerifySubSet=dictionary",
"-XX:+VerifyAfterGC",
"-Xlog:gc+verify,protectiondomain=debug",
"-Djava.security.manager",
Test.class.getName());
OutputAnalyzer output = new OutputAnalyzer(pb.start());
output.shouldContain("PD in set is not alive");
output.shouldHaveExitValue(0);
}
}

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

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