Skip to content

Commit

Permalink
8309140: ResourceHashtable failed "assert(~(_allocation_t[0] | alloca…
Browse files Browse the repository at this point in the history
…tion_mask) == (uintptr_t)this) failed: lost resource object"

Reviewed-by: iklam
Backport-of: b6c789f
  • Loading branch information
coleenp committed Jul 6, 2023
1 parent 830279e commit 98ad856
Show file tree
Hide file tree
Showing 11 changed files with 98 additions and 58 deletions.
23 changes: 14 additions & 9 deletions src/hotspot/share/classfile/loaderConstraints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ class ConstraintSet { // copied into hashtable as
};


ResourceHashtable<SymbolHandle, ConstraintSet, 107, AnyObj::C_HEAP, mtClass, SymbolHandle::compute_hash> _loader_constraint_table;
using InternalLoaderConstraintTable = ResourceHashtable<SymbolHandle, ConstraintSet, 107, AnyObj::C_HEAP, mtClass, SymbolHandle::compute_hash>;
static InternalLoaderConstraintTable* _loader_constraint_table;

void LoaderConstraint::extend_loader_constraint(Symbol* class_name,
ClassLoaderData* loader,
Expand All @@ -131,11 +132,15 @@ void LoaderConstraint::extend_loader_constraint(Symbol* class_name,
// SystemDictionary lock held. This is true even for readers as
// entries in the table could be being dynamically resized.

void LoaderConstraintTable::initialize() {
_loader_constraint_table = new (mtClass) InternalLoaderConstraintTable();
}

LoaderConstraint* LoaderConstraintTable::find_loader_constraint(
Symbol* name, ClassLoaderData* loader_data) {

assert_lock_strong(SystemDictionary_lock);
ConstraintSet* set = _loader_constraint_table.get(name);
ConstraintSet* set = _loader_constraint_table->get(name);
if (set == nullptr) {
return nullptr;
}
Expand Down Expand Up @@ -164,7 +169,7 @@ void LoaderConstraintTable::add_loader_constraint(Symbol* name, InstanceKlass* k
// a parameter name to a method call. We impose this constraint that the
// class that is eventually loaded must match between these two loaders.
bool created;
ConstraintSet* set = _loader_constraint_table.put_if_absent(name, &created);
ConstraintSet* set = _loader_constraint_table->put_if_absent(name, &created);
if (created) {
set->initialize(constraint);
} else {
Expand Down Expand Up @@ -246,7 +251,7 @@ void LoaderConstraintTable::purge_loader_constraints() {
assert_locked_or_safepoint(SystemDictionary_lock);
// Remove unloaded entries from constraint table
PurgeUnloadedConstraints purge;
_loader_constraint_table.unlink(&purge);
_loader_constraint_table->unlink(&purge);
}

void log_ldr_constraint_msg(Symbol* class_name, const char* reason,
Expand Down Expand Up @@ -438,7 +443,7 @@ void LoaderConstraintTable::merge_loader_constraints(Symbol* class_name,
}

// Remove src from set
ConstraintSet* set = _loader_constraint_table.get(class_name);
ConstraintSet* set = _loader_constraint_table->get(class_name);
set->remove_constraint(src);
}

Expand Down Expand Up @@ -477,7 +482,7 @@ void LoaderConstraintTable::verify() {
}
};
assert_locked_or_safepoint(SystemDictionary_lock);
_loader_constraint_table.iterate_all(check);
_loader_constraint_table->iterate_all(check);
}

void LoaderConstraintTable::print_table_statistics(outputStream* st) {
Expand All @@ -491,7 +496,7 @@ void LoaderConstraintTable::print_table_statistics(outputStream* st) {
}
return sum;
};
TableStatistics ts = _loader_constraint_table.statistics_calculate(size);
TableStatistics ts = _loader_constraint_table->statistics_calculate(size);
ts.print(st, "LoaderConstraintTable");
}

Expand All @@ -513,8 +518,8 @@ void LoaderConstraintTable::print_on(outputStream* st) {
assert_locked_or_safepoint(SystemDictionary_lock);
ResourceMark rm;
st->print_cr("Java loader constraints (table_size=%d, constraints=%d)",
_loader_constraint_table.table_size(), _loader_constraint_table.number_of_entries());
_loader_constraint_table.iterate_all(printer);
_loader_constraint_table->table_size(), _loader_constraint_table->number_of_entries());
_loader_constraint_table->iterate_all(printer);
}

void LoaderConstraintTable::print() { print_on(tty); }
2 changes: 1 addition & 1 deletion src/hotspot/share/classfile/loaderConstraints.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class LoaderConstraintTable : public AllStatic {
static void merge_loader_constraints(Symbol* class_name, LoaderConstraint* pp1,
LoaderConstraint* pp2, InstanceKlass* klass);
public:

static void initialize();
// Check class loader constraints
static bool add_entry(Symbol* name, InstanceKlass* klass1, ClassLoaderData* loader1,
InstanceKlass* klass2, ClassLoaderData* loader2);
Expand Down
19 changes: 12 additions & 7 deletions src/hotspot/share/classfile/placeholders.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ class PlaceholderKey {
};

const int _placeholder_table_size = 503; // Does this really have to be prime?
ResourceHashtable<PlaceholderKey, PlaceholderEntry, _placeholder_table_size, AnyObj::C_HEAP, mtClass,
PlaceholderKey::hash, PlaceholderKey::equals> _placeholders;
using InternalPlaceholderTable = ResourceHashtable<PlaceholderKey, PlaceholderEntry, _placeholder_table_size, AnyObj::C_HEAP, mtClass,
PlaceholderKey::hash, PlaceholderKey::equals>;
static InternalPlaceholderTable* _placeholders;

// SeenThread objects represent list of threads that are
// currently performing a load action on a class.
Expand Down Expand Up @@ -207,7 +208,7 @@ PlaceholderEntry* add_entry(Symbol* class_name, ClassLoaderData* loader_data,
entry.set_supername(supername);
PlaceholderKey key(class_name, loader_data);
bool created;
PlaceholderEntry* table_copy = _placeholders.put_if_absent(key, entry, &created);
PlaceholderEntry* table_copy = _placeholders->put_if_absent(key, entry, &created);
assert(created, "better be absent");
return table_copy;
}
Expand All @@ -217,14 +218,14 @@ void remove_entry(Symbol* class_name, ClassLoaderData* loader_data) {
assert_locked_or_safepoint(SystemDictionary_lock);

PlaceholderKey key(class_name, loader_data);
_placeholders.remove(key);
_placeholders->remove(key);
}


PlaceholderEntry* PlaceholderTable::get_entry(Symbol* class_name, ClassLoaderData* loader_data) {
assert_locked_or_safepoint(SystemDictionary_lock);
PlaceholderKey key(class_name, loader_data);
return _placeholders.get(key);
return _placeholders->get(key);
}

static const char* action_to_string(PlaceholderTable::classloadAction action) {
Expand Down Expand Up @@ -271,6 +272,10 @@ PlaceholderEntry* PlaceholderTable::find_and_add(Symbol* name,
return probe;
}

void PlaceholderTable::initialize(){
_placeholders = new (mtClass) InternalPlaceholderTable();
}


// placeholder is used to track class loading internal states
// superthreadQ tracks class circularity, while loading superclass/superinterface
Expand Down Expand Up @@ -340,8 +345,8 @@ void PlaceholderTable::print_on(outputStream* st) {
return true;
};
st->print_cr("Placeholder table (table_size=%d, placeholders=%d)",
_placeholders.table_size(), _placeholders.number_of_entries());
_placeholders.iterate(printer);
_placeholders->table_size(), _placeholders->number_of_entries());
_placeholders->iterate(printer);
}

void PlaceholderTable::print() { return print_on(tty); }
2 changes: 1 addition & 1 deletion src/hotspot/share/classfile/placeholders.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class PlaceholderTable : public AllStatic {
LOAD_SUPER = 2, // loading superclass for this class
DEFINE_CLASS = 3 // find_or_define class
};

static void initialize();
static PlaceholderEntry* get_entry(Symbol* name, ClassLoaderData* loader_data);

// find_and_add returns probe pointer - old or new
Expand Down
22 changes: 13 additions & 9 deletions src/hotspot/share/classfile/protectionDomainCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,17 @@ bool ProtectionDomainCacheTable::equals(const WeakHandle& protection_domain1, co

// WeakHandle is both the key and the value. We need it as the key to compare the oops that each point to
// for equality. We need it as the value to return the one that already exists to link in the DictionaryEntry.
ResourceHashtable<WeakHandle, WeakHandle, 1009, AnyObj::C_HEAP, mtClass,
using InternalProtectionDomainCacheTable = ResourceHashtable<WeakHandle, WeakHandle, 1009, AnyObj::C_HEAP, mtClass,
ProtectionDomainCacheTable::compute_hash,
ProtectionDomainCacheTable::equals> _pd_cache_table;
ProtectionDomainCacheTable::equals>;
static InternalProtectionDomainCacheTable* _pd_cache_table;

bool ProtectionDomainCacheTable::_dead_entries = false;
int ProtectionDomainCacheTable::_total_oops_removed = 0;

void ProtectionDomainCacheTable::initialize(){
_pd_cache_table = new (mtClass) InternalProtectionDomainCacheTable();
}
void ProtectionDomainCacheTable::trigger_cleanup() {
MutexLocker ml(Service_lock, Mutex::_no_safepoint_check_flag);
_dead_entries = true;
Expand Down Expand Up @@ -159,7 +163,7 @@ void ProtectionDomainCacheTable::unlink() {
};

Deleter deleter;
_pd_cache_table.unlink(&deleter);
_pd_cache_table->unlink(&deleter);

_total_oops_removed += deleter._oops_removed;
_dead_entries = false;
Expand All @@ -171,15 +175,15 @@ void ProtectionDomainCacheTable::print_on(outputStream* st) {
st->print_cr(" protection_domain: " PTR_FORMAT, p2i(value.peek()));
};
st->print_cr("Protection domain cache table (table_size=%d, protection domains=%d)",
_pd_cache_table.table_size(), _pd_cache_table.number_of_entries());
_pd_cache_table.iterate_all(printer);
_pd_cache_table->table_size(), _pd_cache_table->number_of_entries());
_pd_cache_table->iterate_all(printer);
}

void ProtectionDomainCacheTable::verify() {
auto verifier = [&] (WeakHandle& key, WeakHandle& value) {
guarantee(value.peek() == nullptr || oopDesc::is_oop(value.peek()), "must be an oop");
};
_pd_cache_table.iterate_all(verifier);
_pd_cache_table->iterate_all(verifier);
}

// The object_no_keepalive() call peeks at the phantomly reachable oop without
Expand All @@ -192,7 +196,7 @@ WeakHandle ProtectionDomainCacheTable::add_if_absent(Handle protection_domain) {
assert_locked_or_safepoint(SystemDictionary_lock);
WeakHandle w(Universe::vm_weak(), protection_domain);
bool created;
WeakHandle* wk = _pd_cache_table.put_if_absent(w, w, &created);
WeakHandle* wk = _pd_cache_table->put_if_absent(w, w, &created);
if (!created) {
// delete the one created since we already had it in the table
w.release(Universe::vm_weak());
Expand All @@ -215,10 +219,10 @@ void ProtectionDomainCacheTable::print_table_statistics(outputStream* st) {
// The only storage is in OopStorage for an oop
return sizeof(oop);
};
TableStatistics ts = _pd_cache_table.statistics_calculate(size);
TableStatistics ts = _pd_cache_table->statistics_calculate(size);
ts.print(st, "ProtectionDomainCacheTable");
}

int ProtectionDomainCacheTable::number_of_entries() {
return _pd_cache_table.number_of_entries();
return _pd_cache_table->number_of_entries();
}
1 change: 1 addition & 0 deletions src/hotspot/share/classfile/protectionDomainCache.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class ProtectionDomainCacheTable : public AllStatic {
static int _total_oops_removed;

public:
static void initialize();
static unsigned int compute_hash(const WeakHandle& protection_domain);
static bool equals(const WeakHandle& protection_domain1, const WeakHandle& protection_domain2);

Expand Down
20 changes: 13 additions & 7 deletions src/hotspot/share/classfile/resolutionErrors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,15 @@ class ResolutionErrorKey {
}
};

ResourceHashtable<ResolutionErrorKey, ResolutionErrorEntry*, 107, AnyObj::C_HEAP, mtClass,
using InternalResolutionErrorTable = ResourceHashtable<ResolutionErrorKey, ResolutionErrorEntry*, 107, AnyObj::C_HEAP, mtClass,
ResolutionErrorKey::hash,
ResolutionErrorKey::equals> _resolution_error_table;
ResolutionErrorKey::equals>;

static InternalResolutionErrorTable* _resolution_error_table;

void ResolutionErrorTable::initialize() {
_resolution_error_table = new (mtClass) InternalResolutionErrorTable();
}

// create new error entry
void ResolutionErrorTable::add_entry(const constantPoolHandle& pool, int cp_index,
Expand All @@ -68,7 +74,7 @@ void ResolutionErrorTable::add_entry(const constantPoolHandle& pool, int cp_inde

ResolutionErrorKey key(pool(), cp_index);
ResolutionErrorEntry *entry = new ResolutionErrorEntry(error, message, cause, cause_msg);
_resolution_error_table.put(key, entry);
_resolution_error_table->put(key, entry);
}

// create new nest host error entry
Expand All @@ -80,14 +86,14 @@ void ResolutionErrorTable::add_entry(const constantPoolHandle& pool, int cp_inde

ResolutionErrorKey key(pool(), cp_index);
ResolutionErrorEntry *entry = new ResolutionErrorEntry(message);
_resolution_error_table.put(key, entry);
_resolution_error_table->put(key, entry);
}

// find entry in the table
ResolutionErrorEntry* ResolutionErrorTable::find_entry(const constantPoolHandle& pool, int cp_index) {
assert_locked_or_safepoint(SystemDictionary_lock);
ResolutionErrorKey key(pool(), cp_index);
ResolutionErrorEntry** entry = _resolution_error_table.get(key);
ResolutionErrorEntry** entry = _resolution_error_table->get(key);
return entry == nullptr ? nullptr : *entry;
}

Expand Down Expand Up @@ -139,7 +145,7 @@ void ResolutionErrorTable::delete_entry(ConstantPool* c) {
assert_locked_or_safepoint(SystemDictionary_lock);

ResolutionErrorDeleteIterate deleteIterator(c);
_resolution_error_table.unlink(&deleteIterator);
_resolution_error_table->unlink(&deleteIterator);
}

class ResolutionIteratePurgeErrors : StackObj {
Expand All @@ -160,5 +166,5 @@ void ResolutionErrorTable::purge_resolution_errors() {
assert_locked_or_safepoint(SystemDictionary_lock);

ResolutionIteratePurgeErrors purgeErrorsIterator;
_resolution_error_table.unlink(&purgeErrorsIterator);
_resolution_error_table->unlink(&purgeErrorsIterator);
}
1 change: 1 addition & 0 deletions src/hotspot/share/classfile/resolutionErrors.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class ResolutionErrorEntry;
class ResolutionErrorTable : AllStatic {

public:
static void initialize();
static void add_entry(const constantPoolHandle& pool, int which, Symbol* error, Symbol* message,
Symbol* cause, Symbol* cause_msg);

Expand Down
27 changes: 17 additions & 10 deletions src/hotspot/share/classfile/systemDictionary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,11 @@ class InvokeMethodKey : public StackObj {

};

ResourceHashtable<InvokeMethodKey, Method*, 139, AnyObj::C_HEAP, mtClass,
InvokeMethodKey::compute_hash, InvokeMethodKey::key_comparison> _invoke_method_intrinsic_table;
ResourceHashtable<SymbolHandle, OopHandle, 139, AnyObj::C_HEAP, mtClass, SymbolHandle::compute_hash> _invoke_method_type_table;
using InvokeMethodIntrinsicTable = ResourceHashtable<InvokeMethodKey, Method*, 139, AnyObj::C_HEAP, mtClass,
InvokeMethodKey::compute_hash, InvokeMethodKey::key_comparison>;
static InvokeMethodIntrinsicTable* _invoke_method_intrinsic_table;
using InvokeMethodTypeTable = ResourceHashtable<SymbolHandle, OopHandle, 139, AnyObj::C_HEAP, mtClass, SymbolHandle::compute_hash>;
static InvokeMethodTypeTable* _invoke_method_type_table;

OopHandle SystemDictionary::_java_system_loader;
OopHandle SystemDictionary::_java_platform_loader;
Expand Down Expand Up @@ -1592,7 +1594,7 @@ void SystemDictionary::methods_do(void f(Method*)) {

{
MutexLocker ml(InvokeMethodIntrinsicTable_lock);
_invoke_method_intrinsic_table.iterate_all(doit);
_invoke_method_intrinsic_table->iterate_all(doit);
}

}
Expand All @@ -1601,10 +1603,15 @@ void SystemDictionary::methods_do(void f(Method*)) {
// Initialization

void SystemDictionary::initialize(TRAPS) {
_invoke_method_intrinsic_table = new (mtClass) InvokeMethodIntrinsicTable();
_invoke_method_type_table = new (mtClass) InvokeMethodTypeTable();
ResolutionErrorTable::initialize();
LoaderConstraintTable::initialize();
PlaceholderTable::initialize();
ProtectionDomainCacheTable::initialize();
#if INCLUDE_CDS
SystemDictionaryShared::initialize();
#endif

// Resolve basic classes
vmClasses::resolve_all(CHECK);
// Resolve classes used by archived heap objects
Expand Down Expand Up @@ -1954,7 +1961,7 @@ Method* SystemDictionary::find_method_handle_intrinsic(vmIntrinsicID iid,
MonitorLocker ml(THREAD, InvokeMethodIntrinsicTable_lock);
while (true) {
bool created;
met = _invoke_method_intrinsic_table.put_if_absent(key, &created);
met = _invoke_method_intrinsic_table->put_if_absent(key, &created);
assert(met != nullptr, "either created or found");
if (*met != nullptr) {
return *met;
Expand Down Expand Up @@ -1985,7 +1992,7 @@ Method* SystemDictionary::find_method_handle_intrinsic(vmIntrinsicID iid,
MonitorLocker ml(THREAD, InvokeMethodIntrinsicTable_lock);
if (throw_error) {
// Remove the entry and let another thread try, or get the same exception.
bool removed = _invoke_method_intrinsic_table.remove(key);
bool removed = _invoke_method_intrinsic_table->remove(key);
assert(removed, "must be the owner");
ml.notify_all();
} else {
Expand Down Expand Up @@ -2148,7 +2155,7 @@ Handle SystemDictionary::find_method_handle_type(Symbol* signature,
OopHandle* o;
{
MutexLocker ml(THREAD, InvokeMethodTypeTable_lock);
o = _invoke_method_type_table.get(signature);
o = _invoke_method_type_table->get(signature);
}

if (o != nullptr) {
Expand Down Expand Up @@ -2219,11 +2226,11 @@ Handle SystemDictionary::find_method_handle_type(Symbol* signature,
MutexLocker ml(THREAD, InvokeMethodTypeTable_lock);
bool created = false;
assert(method_type != nullptr, "unexpected null");
OopHandle* h = _invoke_method_type_table.get(signature);
OopHandle* h = _invoke_method_type_table->get(signature);
if (h == nullptr) {
signature->make_permanent(); // The signature is never unloaded.
OopHandle elem = OopHandle(Universe::vm_global(), method_type());
bool created = _invoke_method_type_table.put(signature, elem);
bool created = _invoke_method_type_table->put(signature, elem);
assert(created, "better be created");
}
}
Expand Down
Loading

0 comments on commit 98ad856

Please sign in to comment.