Skip to content
Permalink
Browse files
8275800: Redefinition leaks MethodData::_extra_data_lock
Reviewed-by: sspitsyn, dholmes
  • Loading branch information
coleenp committed Oct 27, 2021
1 parent 485d658 commit 40606021ee6b7d18674e36b3f6249f1ca8a7647e
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 24 deletions.
@@ -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);
@@ -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;
}

@@ -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) {
@@ -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) {
@@ -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();
@@ -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;
}
@@ -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;
@@ -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; }
@@ -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();
}

@@ -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;
@@ -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);

3 comments on commit 4060602

@openjdk-notifier
Copy link

@openjdk-notifier openjdk-notifier bot commented on 4060602 Oct 27, 2021

Choose a reason for hiding this comment

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

@GoeLin
Copy link
Member

@GoeLin GoeLin commented on 4060602 Dec 30, 2021

Choose a reason for hiding this comment

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

/backport jdk17u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 4060602 Dec 30, 2021

Choose a reason for hiding this comment

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

@GoeLin the backport was successfully created on the branch GoeLin-backport-40606021 in my personal fork of openjdk/jdk17u-dev. To create a pull request with this backport targeting openjdk/jdk17u-dev:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 40606021 from the openjdk/jdk repository.

The commit being backported was authored by Coleen Phillimore on 27 Oct 2021 and was reviewed by Serguei Spitsyn and David Holmes.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk17u-dev:

$ git fetch https://github.com/openjdk-bots/jdk17u-dev GoeLin-backport-40606021:GoeLin-backport-40606021
$ git checkout GoeLin-backport-40606021
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk17u-dev GoeLin-backport-40606021

Please sign in to comment.