Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8275800: Redefinition leaks MethodData::_extra_data_lock #6105

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 13 additions & 18 deletions src/hotspot/share/oops/instanceKlass.cpp
Expand Up @@ -583,7 +583,10 @@ void InstanceKlass::deallocate_contents(ClassLoaderData* loader_data) {

// Release C heap allocated data that this points to, which includes
// reference counting symbol names.
release_C_heap_structures_internal();
// Can't release the constant pool here because the constant pool can be
// deallocated separately from the InstanceKlass for default methods and
// redefine classes.
release_C_heap_structures(/* release_constant_pool */ false);

deallocate_methods(loader_data, methods());
set_methods(NULL);
Expand Down Expand Up @@ -1638,7 +1641,8 @@ bool InstanceKlass::find_field_from_offset(int offset, bool is_static, fieldDesc
void InstanceKlass::methods_do(void f(Method* method)) {
// Methods aren't stable until they are loaded. This can be read outside
// a lock through the ClassLoaderData for profiling
if (!is_loaded()) {
// Redefined scratch classes are on the list and need to be cleaned
if (!is_loaded() && !is_scratch_class()) {
return;
}

Expand Down Expand Up @@ -2681,22 +2685,13 @@ static void method_release_C_heap_structures(Method* m) {
m->release_C_heap_structures();
}

void InstanceKlass::release_C_heap_structures() {

// Called also by InstanceKlass::deallocate_contents, with false for release_constant_pool.
void InstanceKlass::release_C_heap_structures(bool release_constant_pool) {
// Clean up C heap
release_C_heap_structures_internal();
constants()->release_C_heap_structures();
Klass::release_C_heap_structures();

// Deallocate and call destructors for MDO mutexes
methods_do(method_release_C_heap_structures);
}

void InstanceKlass::release_C_heap_structures_internal() {
Klass::release_C_heap_structures();

// Can't release the constant pool here because the constant pool can be
// deallocated separately from the InstanceKlass for default methods and
// redefine classes.

// Deallocate oop map cache
if (_oop_map_cache != NULL) {
Expand Down Expand Up @@ -2732,6 +2727,10 @@ void InstanceKlass::release_C_heap_structures_internal() {
#endif

FREE_C_HEAP_ARRAY(char, _source_debug_extension);

if (release_constant_pool) {
constants()->release_C_heap_structures();
}
}

void InstanceKlass::set_source_debug_extension(const char* array, int length) {
Expand Down Expand Up @@ -3988,8 +3987,6 @@ void InstanceKlass::purge_previous_version_list() {
// so will be deallocated during the next phase of class unloading.
log_trace(redefine, class, iklass, purge)
("previous version " INTPTR_FORMAT " is dead.", p2i(pv_node));
// For debugging purposes.
pv_node->set_is_scratch_class();
// Unlink from previous version list.
assert(pv_node->class_loader_data() == loader_data, "wrong loader_data");
InstanceKlass* next = pv_node->previous_versions();
Expand Down Expand Up @@ -4104,8 +4101,6 @@ void InstanceKlass::add_previous_version(InstanceKlass* scratch_class,
ConstantPool* cp_ref = scratch_class->constants();
if (!cp_ref->on_stack()) {
log_trace(redefine, class, iklass, add)("scratch class not added; no methods are running");
// For debugging purposes.
scratch_class->set_is_scratch_class();
scratch_class->class_loader_data()->add_to_deallocate_list(scratch_class);
return;
}
Expand Down
5 changes: 1 addition & 4 deletions src/hotspot/share/oops/instanceKlass.hpp
Expand Up @@ -1101,7 +1101,7 @@ class InstanceKlass: public Klass {
// callbacks for actions during class unloading
static void unload_class(InstanceKlass* ik);

virtual void release_C_heap_structures();
virtual void release_C_heap_structures(bool release_constant_pool = true);

// Naming
const char* signature_name() const;
Expand Down Expand Up @@ -1218,9 +1218,6 @@ class InstanceKlass: public Klass {
StaticLookupMode static_mode,
PrivateLookupMode private_mode);

// Free CHeap allocated fields.
void release_C_heap_structures_internal();

#if INCLUDE_JVMTI
// RedefineClasses support
void link_previous_versions(InstanceKlass* pv) { _previous_versions = pv; }
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/oops/klass.cpp
Expand Up @@ -105,7 +105,7 @@ bool Klass::is_subclass_of(const Klass* k) const {
return false;
}

void Klass::release_C_heap_structures() {
void Klass::release_C_heap_structures(bool release_constant_pool) {
if (_name != NULL) _name->decrement_refcount();
}

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/oops/klass.hpp
Expand Up @@ -661,7 +661,7 @@ class Klass : public Metadata {
Symbol* name() const { return _name; }
void set_name(Symbol* n);

virtual void release_C_heap_structures();
virtual void release_C_heap_structures(bool release_constant_pool = true);

public:
virtual jint compute_modifier_flags() const = 0;
Expand Down
3 changes: 3 additions & 0 deletions src/hotspot/share/prims/jvmtiRedefineClasses.cpp
Expand Up @@ -4404,6 +4404,9 @@ void VM_RedefineClasses::redefine_single_class(Thread* current, jclass the_jclas

the_class->set_has_been_redefined();

// Scratch class is unloaded but still needs cleaning, and skipping for CDS.
scratch_class->set_is_scratch_class();

// keep track of previous versions of this class
the_class->add_previous_version(scratch_class, emcp_method_count);

Expand Down