8259242: Remove ProtectionDomainSet_lock

Reviewed-by: dholmes, pchilanomate
This commit is contained in:
Coleen Phillimore 2021-04-09 14:59:43 +00:00
parent 9bb1863ed7
commit 06e6b1f7ae
10 changed files with 129 additions and 82 deletions

View File

@ -44,6 +44,7 @@
#include "runtime/javaCalls.hpp"
#include "runtime/mutexLocker.hpp"
#include "runtime/safepointVerifiers.hpp"
#include "utilities/growableArray.hpp"
#include "utilities/hashtable.inline.hpp"
// Optimization: if any dictionary needs resizing, we set this flag,
@ -78,20 +79,19 @@ Dictionary::~Dictionary() {
DictionaryEntry* Dictionary::new_entry(unsigned int hash, InstanceKlass* klass) {
DictionaryEntry* entry = (DictionaryEntry*)Hashtable<InstanceKlass*, mtClass>::new_entry(hash, klass);
entry->set_pd_set(NULL);
entry->release_set_pd_set(NULL);
assert(klass->is_instance_klass(), "Must be");
return entry;
}
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());
while (entry->pd_set_acquire() != NULL) {
ProtectionDomainEntry* to_delete = entry->pd_set_acquire();
entry->release_set_pd_set(to_delete->next_acquire());
delete to_delete;
}
BasicHashtable<mtClass>::free_entry(entry);
@ -141,15 +141,26 @@ bool DictionaryEntry::is_valid_protection_domain(Handle protection_domain) {
: contains_protection_domain(protection_domain());
}
// Reading the pd_set on each DictionaryEntry is lock free and cannot safepoint.
// Adding and deleting entries is under the SystemDictionary_lock
// Deleting unloaded entries on ClassLoaderData for dictionaries that are not unloaded
// is a three step process:
// moving the entries to a separate list, handshake to wait for
// readers to complete (see NSV here), and then actually deleting the entries.
// Deleting entries is done by the ServiceThread when triggered by class unloading.
bool DictionaryEntry::contains_protection_domain(oop protection_domain) const {
assert(Thread::current()->is_Java_thread() || SafepointSynchronize::is_at_safepoint(),
"can only be called by a JavaThread or at safepoint");
// This cannot safepoint while reading the protection domain set.
NoSafepointVerifier nsv;
#ifdef ASSERT
if (protection_domain == instance_klass()->protection_domain()) {
MutexLocker ml(ProtectionDomainSet_lock, Mutex::_no_safepoint_check_flag);
// Ensure this doesn't show up in the pd_set (invariant)
bool in_pd_set = false;
for (ProtectionDomainEntry* current = pd_set();
for (ProtectionDomainEntry* current = pd_set_acquire();
current != NULL;
current = current->next()) {
current = current->next_acquire()) {
if (current->object_no_keepalive() == protection_domain) {
in_pd_set = true;
break;
@ -167,12 +178,9 @@ bool DictionaryEntry::contains_protection_domain(oop protection_domain) const {
return true;
}
// 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.
MutexLocker ml(ProtectionDomainSet_lock, Mutex::_no_safepoint_check_flag);
for (ProtectionDomainEntry* current = pd_set();
for (ProtectionDomainEntry* current = pd_set_acquire();
current != NULL;
current = current->next()) {
current = current->next_acquire()) {
if (current->object_no_keepalive() == protection_domain) {
return true;
}
@ -180,17 +188,13 @@ bool DictionaryEntry::contains_protection_domain(oop protection_domain) const {
return false;
}
void DictionaryEntry::add_protection_domain(Dictionary* dict, Handle protection_domain) {
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.
MutexLocker ml(ProtectionDomainSet_lock, Mutex::_no_safepoint_check_flag);
ProtectionDomainEntry* new_head =
new ProtectionDomainEntry(entry, pd_set());
set_pd_set(new_head);
// Additions and deletions hold the SystemDictionary_lock, readers are lock-free
ProtectionDomainEntry* new_head = new ProtectionDomainEntry(entry, _pd_set);
release_set_pd_set(new_head);
}
LogTarget(Trace, protectiondomain) lt;
if (lt.is_enabled()) {
@ -420,8 +424,9 @@ void Dictionary::validate_protection_domain(unsigned int name_hash,
// 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);
void Dictionary::clean_cached_protection_domains(GrowableArray<ProtectionDomainEntry*>* delete_list) {
assert(Thread::current()->is_Java_thread(), "only called by JavaThread");
assert_lock_strong(SystemDictionary_lock);
assert(!loader_data()->has_class_mirror_holder(), "cld should have a ClassLoader holder not a Class holder");
if (loader_data()->is_the_null_class_loader_data()) {
@ -435,8 +440,7 @@ void Dictionary::clean_cached_protection_domains() {
probe = probe->next()) {
Klass* e = probe->instance_klass();
MutexLocker ml(ProtectionDomainSet_lock, Mutex::_no_safepoint_check_flag);
ProtectionDomainEntry* current = probe->pd_set();
ProtectionDomainEntry* current = probe->pd_set_acquire();
ProtectionDomainEntry* prev = NULL;
while (current != NULL) {
if (current->object_no_keepalive() == NULL) {
@ -450,18 +454,19 @@ void Dictionary::clean_cached_protection_domains() {
ls.print(" loading: "); probe->instance_klass()->print_value_on(&ls);
ls.cr();
}
if (probe->pd_set() == current) {
probe->set_pd_set(current->next());
if (probe->pd_set_acquire() == current) {
probe->release_set_pd_set(current->next_acquire());
} else {
assert(prev != NULL, "should be set by alive entry");
prev->set_next(current->next());
prev->release_set_next(current->next_acquire());
}
ProtectionDomainEntry* to_delete = current;
current = current->next();
delete to_delete;
// Mark current for deletion but in the meantime it can still be
// traversed.
delete_list->push(current);
current = current->next_acquire();
} else {
prev = current;
current = current->next();
current = current->next_acquire();
}
}
}
@ -552,20 +557,20 @@ void SymbolPropertyTable::free_entry(SymbolPropertyEntry* entry) {
}
void DictionaryEntry::verify_protection_domain_set() {
MutexLocker ml(ProtectionDomainSet_lock, Mutex::_no_safepoint_check_flag);
for (ProtectionDomainEntry* current = pd_set(); // accessed at a safepoint
assert(SafepointSynchronize::is_at_safepoint(), "must only be called as safepoint");
for (ProtectionDomainEntry* current = pd_set_acquire(); // accessed at a safepoint
current != NULL;
current = current->_next) {
guarantee(oopDesc::is_oop_or_null(current->_pd_cache->object_no_keepalive()), "Invalid oop");
current = current->next_acquire()) {
guarantee(oopDesc::is_oop_or_null(current->object_no_keepalive()), "Invalid oop");
}
}
void DictionaryEntry::print_count(outputStream *st) {
MutexLocker ml(ProtectionDomainSet_lock, Mutex::_no_safepoint_check_flag);
assert_locked_or_safepoint(SystemDictionary_lock);
int count = 0;
for (ProtectionDomainEntry* current = pd_set(); // accessed inside SD lock
for (ProtectionDomainEntry* current = pd_set_acquire();
current != NULL;
current = current->_next) {
current = current->next_acquire()) {
count++;
}
st->print_cr("pd set count = #%d", count);
@ -596,6 +601,8 @@ void Dictionary::print_on(outputStream* st) const {
// redundant and obvious.
st->print(", ");
cld->print_value_on(st);
st->print(", ");
probe->print_count(st);
}
st->cr();
}

View File

@ -33,6 +33,7 @@
class DictionaryEntry;
class ProtectionDomainEntry;
template <typename T> class GrowableArray;
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// The data structure for the class loader data dictionaries.
@ -67,7 +68,7 @@ public:
void all_entries_do(KlassClosure* closure);
void classes_do(MetaspaceClosure* it);
void clean_cached_protection_domains();
void clean_cached_protection_domains(GrowableArray<ProtectionDomainEntry*>* delete_list);
// Protection domains
InstanceKlass* find(unsigned int hash, Symbol* name, Handle protection_domain);
@ -111,18 +112,11 @@ class DictionaryEntry : public HashtableEntry<InstanceKlass*, mtClass> {
// Contains the set of approved protection domains that can access
// this dictionary entry.
//
// This protection domain set is a set of tuples:
//
// (InstanceKlass C, initiating class loader ICL, Protection Domain PD)
//
// [Note that C.protection_domain(), which is stored in the java.lang.Class
// mirror of C, is NOT the same as PD]
//
// If such an entry (C, ICL, PD) exists in the table, it means that
// it is okay for a class Foo to reference C, where
//
// Foo.protection_domain() == PD, and
// Foo's defining class loader == ICL
// If an entry for PD exists in the list, it means that
// it is okay for a caller class to reference the class in this dictionary entry.
//
// The usage of the PD set can be seen in SystemDictionary::validate_protection_domain()
// It is essentially a cache to avoid repeated Java up-calls to
@ -147,8 +141,8 @@ class DictionaryEntry : public HashtableEntry<InstanceKlass*, mtClass> {
return (DictionaryEntry**)HashtableEntry<InstanceKlass*, mtClass>::next_addr();
}
ProtectionDomainEntry* pd_set() const { return _pd_set; }
void set_pd_set(ProtectionDomainEntry* new_head) { _pd_set = new_head; }
ProtectionDomainEntry* pd_set_acquire() const { return Atomic::load_acquire(&_pd_set); }
void release_set_pd_set(ProtectionDomainEntry* entry) { Atomic::release_store(&_pd_set, entry); }
// Tells whether the initiating class' protection domain can access the klass in this entry
inline bool is_valid_protection_domain(Handle protection_domain);

View File

@ -34,6 +34,8 @@
#include "memory/universe.hpp"
#include "oops/oop.inline.hpp"
#include "oops/weakHandle.inline.hpp"
#include "runtime/atomic.hpp"
#include "utilities/growableArray.hpp"
#include "utilities/hashtable.inline.hpp"
unsigned int ProtectionDomainCacheTable::compute_hash(Handle protection_domain) {
@ -58,18 +60,61 @@ void ProtectionDomainCacheTable::trigger_cleanup() {
}
class CleanProtectionDomainEntries : public CLDClosure {
GrowableArray<ProtectionDomainEntry*>* _delete_list;
public:
CleanProtectionDomainEntries(GrowableArray<ProtectionDomainEntry*>* delete_list) :
_delete_list(delete_list) {}
void do_cld(ClassLoaderData* data) {
Dictionary* dictionary = data->dictionary();
if (dictionary != NULL) {
dictionary->clean_cached_protection_domains();
dictionary->clean_cached_protection_domains(_delete_list);
}
}
};
static GrowableArray<ProtectionDomainEntry*>* _delete_list = NULL;
class HandshakeForPD : public HandshakeClosure {
public:
HandshakeForPD() : HandshakeClosure("HandshakeForPD") {}
void do_thread(Thread* thread) {
log_trace(protectiondomain)("HandshakeForPD::do_thread: thread="
INTPTR_FORMAT, p2i(thread));
}
};
static void purge_deleted_entries() {
// If there are any deleted entries, Handshake-all then they'll be
// safe to remove since traversing the pd_set list does not stop for
// safepoints and only JavaThreads will read the pd_set.
// This is actually quite rare because the protection domain is generally associated
// with the caller class and class loader, which if still alive will keep this
// protection domain entry alive.
if (_delete_list->length() >= 10) {
HandshakeForPD hs_pd;
Handshake::execute(&hs_pd);
for (int i = _delete_list->length() - 1; i >= 0; i--) {
ProtectionDomainEntry* entry = _delete_list->at(i);
_delete_list->remove_at(i);
delete entry;
}
assert(_delete_list->length() == 0, "should be cleared");
}
}
void ProtectionDomainCacheTable::unlink() {
// The dictionary entries _pd_set field should be null also, so nothing to do.
assert(java_lang_System::allow_security_manager(), "should not be called otherwise");
// Create a list for holding deleted entries
if (_delete_list == NULL) {
_delete_list = new (ResourceObj::C_HEAP, mtClass)
GrowableArray<ProtectionDomainEntry*>(20, mtClass);
}
{
// First clean cached pd lists in loaded CLDs
// It's unlikely, but some loaded classes in a dictionary might
@ -77,10 +122,13 @@ void ProtectionDomainCacheTable::unlink() {
// The dictionary pd_set points at entries in the ProtectionDomainCacheTable.
MutexLocker ml(ClassLoaderDataGraph_lock);
MutexLocker mldict(SystemDictionary_lock); // need both.
CleanProtectionDomainEntries clean;
CleanProtectionDomainEntries clean(_delete_list);
ClassLoaderDataGraph::loaded_cld_do(&clean);
}
// Purge any deleted entries outside of the SystemDictionary_lock.
purge_deleted_entries();
MutexLocker ml(SystemDictionary_lock);
int oops_removed = 0;
for (int i = 0; i < table_size(); ++i) {
@ -129,10 +177,6 @@ oop ProtectionDomainCacheEntry::object() {
return literal().resolve();
}
oop ProtectionDomainEntry::object() {
return _pd_cache->object();
}
// The object_no_keepalive() call peeks at the phantomly reachable oop without
// keeping it alive. This is okay to do in the VM thread state if it is not
// leaked out to become strongly reachable.

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2021, 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
@ -27,7 +27,7 @@
#include "oops/oop.hpp"
#include "oops/weakHandle.hpp"
#include "memory/iterator.hpp"
#include "runtime/atomic.hpp"
#include "utilities/hashtable.hpp"
// This class caches the approved protection domains that can access loaded classes.
@ -62,8 +62,6 @@ class ProtectionDomainCacheEntry : public HashtableEntry<WeakHandle, mtClass> {
// The amount of different protection domains used is typically magnitudes smaller
// than the number of system dictionary entries (loaded classes).
class ProtectionDomainCacheTable : public Hashtable<WeakHandle, mtClass> {
friend class VMStructs;
private:
ProtectionDomainCacheEntry* bucket(int i) const {
return (ProtectionDomainCacheEntry*) Hashtable<WeakHandle, mtClass>::bucket(i);
}
@ -104,20 +102,17 @@ public:
};
// This describes the linked list protection domain for each DictionaryEntry in pd_set.
class ProtectionDomainEntry :public CHeapObj<mtClass> {
friend class VMStructs;
public:
ProtectionDomainEntry* _next;
ProtectionDomainCacheEntry* _pd_cache;
ProtectionDomainEntry* volatile _next;
public:
ProtectionDomainEntry(ProtectionDomainCacheEntry* pd_cache, ProtectionDomainEntry* next) {
_pd_cache = pd_cache;
_next = next;
}
ProtectionDomainEntry(ProtectionDomainCacheEntry* pd_cache,
ProtectionDomainEntry* head) : _pd_cache(pd_cache), _next(head) {}
ProtectionDomainEntry* next() { return _next; }
void set_next(ProtectionDomainEntry* entry) { _next = entry; }
oop object();
ProtectionDomainEntry* next_acquire() { return Atomic::load_acquire(&_next); }
void release_set_next(ProtectionDomainEntry* entry) { Atomic::release_store(&_next, entry); }
oop object_no_keepalive();
};
#endif // SHARE_CLASSFILE_PROTECTIONDOMAINCACHE_HPP

View File

@ -42,7 +42,6 @@
Mutex* Patching_lock = NULL;
Mutex* CompiledMethod_lock = NULL;
Monitor* SystemDictionary_lock = NULL;
Mutex* ProtectionDomainSet_lock = NULL;
Mutex* SharedDictionary_lock = NULL;
Mutex* Module_lock = NULL;
Mutex* CompiledIC_lock = NULL;
@ -261,7 +260,6 @@ void mutex_init() {
def(JmethodIdCreation_lock , PaddedMutex , special-2, true, _safepoint_check_never); // used for creating jmethodIDs.
def(SystemDictionary_lock , PaddedMonitor, leaf, true, _safepoint_check_always);
def(ProtectionDomainSet_lock , PaddedMutex , leaf-1, true, _safepoint_check_never);
def(SharedDictionary_lock , PaddedMutex , leaf, true, _safepoint_check_always);
def(Module_lock , PaddedMutex , leaf+2, false, _safepoint_check_always);
def(InlineCacheBuffer_lock , PaddedMutex , leaf, true, _safepoint_check_never);

View File

@ -34,7 +34,6 @@
extern Mutex* Patching_lock; // a lock used to guard code patching of compiled code
extern Mutex* CompiledMethod_lock; // a lock used to guard a compiled method and OSR queues
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

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2021, 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
@ -37,7 +37,7 @@ public class ClassForName {
public ClassForName() {
try {
// class_loader = URLClassLoader, protection_domain = ClassForName.getProtectionDomain()
// class_loader = App$ClassLoader, protection_domain = ClassForName.getProtectionDomain()
Class.forName(java.util.List.class.getName(), false,
ClassLoader.getSystemClassLoader());
} catch (Throwable e) {

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2021, 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
@ -84,7 +84,9 @@ public class ProtectionDomainCacheTest {
CLASSFILENAME);
Files.delete(classFile);
loadAndRun(jarFilePath);
for (int i = 0; i < 20; i++) {
loadAndRun(jarFilePath);
}
// Give the GC a chance to unload protection domains
for (int i = 0; i < 100; i++) {
@ -101,11 +103,12 @@ public class ProtectionDomainCacheTest {
"-XX:+UnlockDiagnosticVMOptions",
"-XX:VerifySubSet=dictionary",
"-XX:+VerifyAfterGC",
"-Xlog:gc+verify,protectiondomain=debug",
"-Xlog:gc+verify,protectiondomain=trace",
"-Djava.security.manager",
Test.class.getName());
OutputAnalyzer output = new OutputAnalyzer(pb.start());
output.shouldContain("PD in set is not alive");
output.shouldContain("HandshakeForPD::do_thread");
output.shouldHaveExitValue(0);
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2021, 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
@ -43,7 +43,14 @@
* @comment build test class and indify classes
* @build vm.mlvm.mixed.stress.java.findDeadlock.INDIFY_Test
* @run driver vm.mlvm.share.IndifiedClassesBuilder
*
* @run main/othervm -Xlog:gc,safepoint vm.mlvm.mixed.stress.java.findDeadlock.INDIFY_Test
*
* To see code that takes more time to safepoint run with:
* main/othervm -XX:+SafepointTimeout -XX:+UnlockDiagnosticVMOptions
* -XX:+AbortVMOnSafepointTimeout
* -XX:SafepointTimeoutDelay=500
* -XX:+PrintSystemDictionaryAtExit
* -Xlog:gc,safepoint
* vm.mlvm.mixed.stress.java.findDeadlock.INDIFY_Test
*/

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2021, 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
@ -109,7 +109,7 @@ public class Indify {
public boolean keepgoing = false;
public boolean expandProperties = false;
public boolean overwrite = false;
public boolean quiet = false;
public boolean quiet = true;
public boolean verbose = false;
public boolean transitionalJSR292 = true; // default to false later
public boolean all = false;