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

8296955: Kitchensink.java failed with "double free or corruption (!prev): <addr>"

Browse Source 
Reviewed-by: kbarrett, eosterlund, sspitsyn, dcubed
This commit is contained in:
Coleen Phillimore 2022-12-13 00:49:16 +00:00
parent 8962c723a8
commit d453190300
3 changed files with 39 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
src/hotspot/share/oops/instanceKlass.cpp
View File

@ -2148,15 +2148,8 @@ jmethodID InstanceKlass::get_jmethod_id(const methodHandle& method_h) {
// the cache can't grow so we can just get the current values
get_jmethod_id_length_value(jmeths, idnum, &length, &id);
} else {
// cache can grow so we have to be more careful
if (Threads::number_of_threads() == 0 ||
SafepointSynchronize::is_at_safepoint()) {
// we're single threaded or at a safepoint - no locking needed
get_jmethod_id_length_value(jmeths, idnum, &length, &id);
} else {
MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag);
get_jmethod_id_length_value(jmeths, idnum, &length, &id);
}
MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag);
get_jmethod_id_length_value(jmeths, idnum, &length, &id);
}
}
// implied else:
@ -2166,8 +2159,8 @@ jmethodID InstanceKlass::get_jmethod_id(const methodHandle& method_h) {
length <= idnum || // cache is too short
id == NULL) { // cache doesn't contain entry
// This function can be called by the VMThread so we have to do all
// things that might block on a safepoint before grabbing the lock.
// This function can be called by the VMThread or GC worker threads so we
// have to do all things that might block on a safepoint before grabbing the lock.
// Otherwise, we can deadlock with the VMThread or have a cache
// consistency issue. These vars keep track of what we might have
// to free after the lock is dropped.
@ -2186,25 +2179,20 @@ jmethodID InstanceKlass::get_jmethod_id(const methodHandle& method_h) {
}
// allocate a new jmethodID that might be used
jmethodID new_id = NULL;
if (method_h->is_old() && !method_h->is_obsolete()) {
// The method passed in is old (but not obsolete), we need to use the current version
Method* current_method = method_with_idnum((int)idnum);
assert(current_method != NULL, "old and but not obsolete, so should exist");
new_id = Method::make_jmethod_id(class_loader_data(), current_method);
} else {
// It is the current version of the method or an obsolete method,
// use the version passed in
new_id = Method::make_jmethod_id(class_loader_data(), method_h());
}
if (Threads::number_of_threads() == 0 ||
SafepointSynchronize::is_at_safepoint()) {
// we're single threaded or at a safepoint - no locking needed
id = get_jmethod_id_fetch_or_update(idnum, new_id, new_jmeths,
&to_dealloc_id, &to_dealloc_jmeths);
} else {
{
MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag);
jmethodID new_id = NULL;
if (method_h->is_old() && !method_h->is_obsolete()) {
// The method passed in is old (but not obsolete), we need to use the current version
Method* current_method = method_with_idnum((int)idnum);
assert(current_method != NULL, "old and but not obsolete, so should exist");
new_id = Method::make_jmethod_id(class_loader_data(), current_method);
} else {
// It is the current version of the method or an obsolete method,
// use the version passed in
new_id = Method::make_jmethod_id(class_loader_data(), method_h());
}
id = get_jmethod_id_fetch_or_update(idnum, new_id, new_jmeths,
&to_dealloc_id, &to_dealloc_jmeths);
}
@ -2254,9 +2242,7 @@ jmethodID InstanceKlass::get_jmethod_id_fetch_or_update(
assert(new_id != NULL, "sanity check");
assert(to_dealloc_id_p != NULL, "sanity check");
assert(to_dealloc_jmeths_p != NULL, "sanity check");
assert(Threads::number_of_threads() == 0 ||
SafepointSynchronize::is_at_safepoint() ||
JmethodIdCreation_lock->owned_by_self(), "sanity check");
assert(JmethodIdCreation_lock->owned_by_self(), "sanity check");
// reacquire the cache - we are locked, single threaded or at a safepoint
jmethodID* jmeths = methods_jmethod_ids_acquire();

58
src/hotspot/share/oops/method.cpp
View File

@ -2165,50 +2165,29 @@ JNIMethodBlockNode::JNIMethodBlockNode(int num_methods) : _top(0), _next(NULL) {
}
}
void Method::ensure_jmethod_ids(ClassLoaderData* loader_data, int capacity) {
ClassLoaderData* cld = loader_data;
if (!SafepointSynchronize::is_at_safepoint()) {
// Have to add jmethod_ids() to class loader data thread-safely.
// Also have to add the method to the list safely, which the lock
// protects as well.
MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag);
if (cld->jmethod_ids() == NULL) {
cld->set_jmethod_ids(new JNIMethodBlock(capacity));
} else {
cld->jmethod_ids()->ensure_methods(capacity);
}
void Method::ensure_jmethod_ids(ClassLoaderData* cld, int capacity) {
// Have to add jmethod_ids() to class loader data thread-safely.
// Also have to add the method to the list safely, which the lock
// protects as well.
MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag);
if (cld->jmethod_ids() == NULL) {
cld->set_jmethod_ids(new JNIMethodBlock(capacity));
} else {
// At safepoint, we are single threaded and can set this.
if (cld->jmethod_ids() == NULL) {
cld->set_jmethod_ids(new JNIMethodBlock(capacity));
} else {
cld->jmethod_ids()->ensure_methods(capacity);
}
cld->jmethod_ids()->ensure_methods(capacity);
}
}
// Add a method id to the jmethod_ids
jmethodID Method::make_jmethod_id(ClassLoaderData* loader_data, Method* m) {
ClassLoaderData* cld = loader_data;
if (!SafepointSynchronize::is_at_safepoint()) {
// Have to add jmethod_ids() to class loader data thread-safely.
// Also have to add the method to the list safely, which the lock
// protects as well.
MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag);
if (cld->jmethod_ids() == NULL) {
cld->set_jmethod_ids(new JNIMethodBlock());
}
// jmethodID is a pointer to Method*
return (jmethodID)cld->jmethod_ids()->add_method(m);
} else {
// At safepoint, we are single threaded and can set this.
if (cld->jmethod_ids() == NULL) {
cld->set_jmethod_ids(new JNIMethodBlock());
}
// jmethodID is a pointer to Method*
return (jmethodID)cld->jmethod_ids()->add_method(m);
jmethodID Method::make_jmethod_id(ClassLoaderData* cld, Method* m) {
// Have to add jmethod_ids() to class loader data thread-safely.
// Also have to add the method to the list safely, which the lock
// protects as well.
assert(JmethodIdCreation_lock->owned_by_self(), "sanity check");
if (cld->jmethod_ids() == NULL) {
cld->set_jmethod_ids(new JNIMethodBlock());
}
// jmethodID is a pointer to Method*
return (jmethodID)cld->jmethod_ids()->add_method(m);
}
jmethodID Method::jmethod_id() {
@ -2218,8 +2197,7 @@ jmethodID Method::jmethod_id() {
// Mark a jmethodID as free. This is called when there is a data race in
// InstanceKlass while creating the jmethodID cache.
void Method::destroy_jmethod_id(ClassLoaderData* loader_data, jmethodID m) {
ClassLoaderData* cld = loader_data;
void Method::destroy_jmethod_id(ClassLoaderData* cld, jmethodID m) {
Method** ptr = (Method**)m;
assert(cld->jmethod_ids() != NULL, "should have method handles");
cld->jmethod_ids()->destroy_method(ptr);

6
src/hotspot/share/oops/method.hpp
View File

@ -774,13 +774,13 @@ public:
// however, can be GC'ed away if the class is unloaded or if the method is
// made obsolete or deleted -- in these cases, the jmethodID
// refers to NULL (as is the case for any weak reference).
static jmethodID make_jmethod_id(ClassLoaderData* loader_data, Method* mh);
static void destroy_jmethod_id(ClassLoaderData* loader_data, jmethodID mid);
static jmethodID make_jmethod_id(ClassLoaderData* cld, Method* mh);
static void destroy_jmethod_id(ClassLoaderData* cld, jmethodID mid);
// Ensure there is enough capacity in the internal tracking data
// structures to hold the number of jmethodIDs you plan to generate.
// This saves substantial time doing allocations.
static void ensure_jmethod_ids(ClassLoaderData* loader_data, int capacity);
static void ensure_jmethod_ids(ClassLoaderData* cld, int capacity);
// Use resolve_jmethod_id() in situations where the caller is expected
// to provide a valid jmethodID; the only sanity checks are in asserts;