Skip to content

Commit

Permalink
8268364: jmethod clearing should be done during unloading
Browse files Browse the repository at this point in the history
Reviewed-by: coleenp
Backport-of: 3d84398
  • Loading branch information
krk authored and shipilev committed Dec 4, 2024
1 parent 1c80800 commit 50da3f6
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 11 deletions.
24 changes: 15 additions & 9 deletions src/hotspot/share/classfile/classLoaderData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,21 @@ void ClassLoaderData::unload() {
// after erroneous classes are released.
classes_do(InstanceKlass::notify_unload_class);

// Method::clear_jmethod_ids only sets the jmethod_ids to NULL without
// releasing the memory for related JNIMethodBlocks and JNIMethodBlockNodes.
// This is done intentionally because native code (e.g. JVMTI agent) holding
// jmethod_ids may access them after the associated classes and class loader
// are unloaded. The Java Native Interface Specification says "method ID
// does not prevent the VM from unloading the class from which the ID has
// been derived. After the class is unloaded, the method or field ID becomes
// invalid". In real world usages, the native code may rely on jmethod_ids
// being NULL after class unloading. Hence, it is unsafe to free the memory
// from the VM side without knowing when native code is going to stop using
// them.
if (_jmethod_ids != NULL) {
Method::clear_jmethod_ids(this);
}

// Clean up global class iterator for compiler
static_klass_iterator.adjust_saved_class(this);
}
Expand Down Expand Up @@ -749,15 +764,6 @@ ClassLoaderData::~ClassLoaderData() {
_metaspace = NULL;
delete m;
}
// Clear all the JNI handles for methods
// These aren't deallocated and are going to look like a leak, but that's
// needed because we can't really get rid of jmethodIDs because we don't
// know when native code is going to stop using them. The spec says that
// they're "invalid" but existing programs likely rely on their being
// NULL after class unloading.
if (_jmethod_ids != NULL) {
Method::clear_jmethod_ids(this);
}
// Delete lock
delete _metaspace_lock;

Expand Down
9 changes: 7 additions & 2 deletions src/hotspot/share/oops/method.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2191,10 +2191,15 @@ bool Method::is_method_id(jmethodID mid) {
Method* Method::checked_resolve_jmethod_id(jmethodID mid) {
if (mid == NULL) return NULL;
Method* o = resolve_jmethod_id(mid);
if (o == NULL || o == JNIMethodBlock::_free_method || !((Metadata*)o)->is_method()) {
if (o == NULL || o == JNIMethodBlock::_free_method) {
return NULL;
}
return o;
// Method should otherwise be valid. Assert for testing.
assert(is_valid_method(o), "should be valid jmethodid");
// If the method's class holder object is unreferenced, but not yet marked as
// unloaded, we need to return NULL here too because after a safepoint, its memory
// will be reclaimed.
return o->method_holder()->is_loader_alive() ? o : NULL;
};

void Method::set_on_stack(const bool value) {
Expand Down

1 comment on commit 50da3f6

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.