Skip to content

Commit

Permalink
8333542: Breakpoint in parallel code does not work
Browse files Browse the repository at this point in the history
Co-authored-by: Chris Plummer <cjplummer@openjdk.org>
Reviewed-by: dholmes, vlivanov
  • Loading branch information
coleenp and plummercj committed Jun 25, 2024
1 parent 86b0cf2 commit b3bf31a
Show file tree
Hide file tree
Showing 16 changed files with 387 additions and 177 deletions.
26 changes: 26 additions & 0 deletions src/hotspot/share/classfile/javaClasses.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -788,6 +788,7 @@ int java_lang_Class::_class_loader_offset;
int java_lang_Class::_module_offset;
int java_lang_Class::_protection_domain_offset;
int java_lang_Class::_component_mirror_offset;
int java_lang_Class::_init_lock_offset;
int java_lang_Class::_signers_offset;
int java_lang_Class::_name_offset;
int java_lang_Class::_source_file_offset;
Expand Down Expand Up @@ -911,6 +912,12 @@ void java_lang_Class::initialize_mirror_fields(Klass* k,
Handle protection_domain,
Handle classData,
TRAPS) {
// Allocate a simple java object for a lock.
// This needs to be a java object because during class initialization
// it can be held across a java call.
typeArrayOop r = oopFactory::new_typeArray(T_INT, 0, CHECK);
set_init_lock(mirror(), r);

// Set protection domain also
set_protection_domain(mirror(), protection_domain());

Expand Down Expand Up @@ -1132,6 +1139,10 @@ bool java_lang_Class::restore_archived_mirror(Klass *k,
if (!k->is_array_klass()) {
// - local static final fields with initial values were initialized at dump time

// create the init_lock
typeArrayOop r = oopFactory::new_typeArray(T_INT, 0, CHECK_(false));
set_init_lock(mirror(), r);

if (protection_domain.not_null()) {
set_protection_domain(mirror(), protection_domain());
}
Expand Down Expand Up @@ -1196,6 +1207,15 @@ oop java_lang_Class::component_mirror(oop java_class) {
return java_class->obj_field(_component_mirror_offset);
}

oop java_lang_Class::init_lock(oop java_class) {
assert(_init_lock_offset != 0, "must be set");
return java_class->obj_field(_init_lock_offset);
}
void java_lang_Class::set_init_lock(oop java_class, oop init_lock) {
assert(_init_lock_offset != 0, "must be set");
java_class->obj_field_put(_init_lock_offset, init_lock);
}

objArrayOop java_lang_Class::signers(oop java_class) {
assert(_signers_offset != 0, "must be set");
return (objArrayOop)java_class->obj_field(_signers_offset);
Expand Down Expand Up @@ -1415,12 +1435,18 @@ void java_lang_Class::compute_offsets() {
InstanceKlass* k = vmClasses::Class_klass();
CLASS_FIELDS_DO(FIELD_COMPUTE_OFFSET);

// Init lock is a C union with component_mirror. Only instanceKlass mirrors have
// init_lock and only ArrayKlass mirrors have component_mirror. Since both are oops
// GC treats them the same.
_init_lock_offset = _component_mirror_offset;

CLASS_INJECTED_FIELDS(INJECTED_FIELD_COMPUTE_OFFSET);
}

#if INCLUDE_CDS
void java_lang_Class::serialize_offsets(SerializeClosure* f) {
f->do_bool(&_offsets_computed);
f->do_u4((u4*)&_init_lock_offset);

CLASS_FIELDS_DO(FIELD_SERIALIZE_OFFSET);

Expand Down
8 changes: 7 additions & 1 deletion src/hotspot/share/classfile/javaClasses.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -226,6 +226,7 @@ class java_lang_Class : AllStatic {
static int _static_oop_field_count_offset;

static int _protection_domain_offset;
static int _init_lock_offset;
static int _signers_offset;
static int _class_loader_offset;
static int _module_offset;
Expand All @@ -240,6 +241,7 @@ class java_lang_Class : AllStatic {
static GrowableArray<Klass*>* _fixup_mirror_list;
static GrowableArray<Klass*>* _fixup_module_field_list;

static void set_init_lock(oop java_class, oop init_lock);
static void set_protection_domain(oop java_class, oop protection_domain);
static void set_class_loader(oop java_class, oop class_loader);
static void set_component_mirror(oop java_class, oop comp_mirror);
Expand Down Expand Up @@ -292,6 +294,10 @@ class java_lang_Class : AllStatic {

// Support for embedded per-class oops
static oop protection_domain(oop java_class);
static oop init_lock(oop java_class);
static void clear_init_lock(oop java_class) {
set_init_lock(java_class, nullptr);
}
static oop component_mirror(oop java_class);
static objArrayOop signers(oop java_class);
static void set_signers(oop java_class, objArrayOop signers);
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/classfile/vmSymbols.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,7 @@ class SerializeClosure;
template(bool_array_signature, "[Z") \
template(byte_array_signature, "[B") \
template(char_array_signature, "[C") \
template(int_array_signature, "[I") \
template(runnable_signature, "Ljava/lang/Runnable;") \
template(continuation_signature, "Ljdk/internal/vm/Continuation;") \
template(continuationscope_signature, "Ljdk/internal/vm/ContinuationScope;") \
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/interpreter/linkResolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1824,7 +1824,7 @@ void LinkResolver::resolve_invokedynamic(CallInfo& result, const constantPoolHan
// the interpreter or runtime performs a serialized check of
// the relevant ResolvedIndyEntry::method field. This is done by the caller
// of this method, via CPC::set_dynamic_call, which uses
// a lock to do the final serialization of updates
// an ObjectLocker to do the final serialization of updates
// to ResolvedIndyEntry state, including method.

// Log dynamic info to CDS classlist.
Expand Down
45 changes: 34 additions & 11 deletions src/hotspot/share/oops/cpCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
#include "runtime/atomic.hpp"
#include "runtime/handles.inline.hpp"
#include "runtime/mutexLocker.hpp"
#include "runtime/synchronizer.hpp"
#include "runtime/vm_version.hpp"
#include "utilities/macros.hpp"

Expand Down Expand Up @@ -174,7 +175,7 @@ void ConstantPoolCache::set_direct_or_vtable_call(Bytecodes::Code invoke_code,
}
if (invoke_code == Bytecodes::_invokestatic) {
assert(method->method_holder()->is_initialized() ||
method->method_holder()->is_init_thread(JavaThread::current()),
method->method_holder()->is_reentrant_initialization(JavaThread::current()),
"invalid class initialization state for invoke_static");

if (!VM_Version::supports_fast_class_init_checks() && method->needs_clinit_barrier()) {
Expand Down Expand Up @@ -269,11 +270,20 @@ ResolvedMethodEntry* ConstantPoolCache::set_method_handle(int method_index, cons
// A losing writer waits on the lock until the winner writes the method and leaves
// the lock, so that when the losing writer returns, he can use the linked
// cache entry.

// Lock fields to write
Bytecodes::Code invoke_code = Bytecodes::_invokehandle;
MutexLocker ml(constant_pool()->pool_holder()->init_monitor());
ResolvedMethodEntry* method_entry = resolved_method_entry_at(method_index);

JavaThread* current = JavaThread::current();
objArrayHandle resolved_references(current, constant_pool()->resolved_references());
// Use the resolved_references() lock for this cpCache entry.
// resolved_references are created for all classes with Invokedynamic, MethodHandle
// or MethodType constant pool cache entries.
assert(resolved_references() != nullptr,
"a resolved_references array should have been created for this class");
ObjectLocker ol(resolved_references, current);

ResolvedMethodEntry* method_entry = resolved_method_entry_at(method_index);
if (method_entry->is_resolved(invoke_code)) {
return method_entry;
}
Expand Down Expand Up @@ -311,7 +321,6 @@ ResolvedMethodEntry* ConstantPoolCache::set_method_handle(int method_index, cons
// Store appendix, if any.
if (has_appendix) {
const int appendix_index = method_entry->resolved_references_index();
objArrayOop resolved_references = constant_pool()->resolved_references();
assert(appendix_index >= 0 && appendix_index < resolved_references->length(), "oob");
assert(resolved_references->obj_at(appendix_index) == nullptr, "init just once");
resolved_references->obj_at_put(appendix_index, appendix());
Expand Down Expand Up @@ -587,7 +596,14 @@ bool ConstantPoolCache::save_and_throw_indy_exc(
assert(PENDING_EXCEPTION->is_a(vmClasses::LinkageError_klass()),
"No LinkageError exception");

MutexLocker ml(THREAD, cpool->pool_holder()->init_monitor());
// Use the resolved_references() lock for this cpCache entry.
// resolved_references are created for all classes with Invokedynamic, MethodHandle
// or MethodType constant pool cache entries.
JavaThread* current = THREAD;
objArrayHandle resolved_references(current, cpool->resolved_references());
assert(resolved_references() != nullptr,
"a resolved_references array should have been created for this class");
ObjectLocker ol(resolved_references, current);

// if the indy_info is resolved or the indy_resolution_failed flag is set then another
// thread either succeeded in resolving the method or got a LinkageError
Expand All @@ -610,21 +626,29 @@ bool ConstantPoolCache::save_and_throw_indy_exc(

oop ConstantPoolCache::set_dynamic_call(const CallInfo &call_info, int index) {
ResourceMark rm;
MutexLocker ml(constant_pool()->pool_holder()->init_monitor());

// Use the resolved_references() lock for this cpCache entry.
// resolved_references are created for all classes with Invokedynamic, MethodHandle
// or MethodType constant pool cache entries.
JavaThread* current = JavaThread::current();
constantPoolHandle cp(current, constant_pool());

objArrayHandle resolved_references(current, cp->resolved_references());
assert(resolved_references() != nullptr,
"a resolved_references array should have been created for this class");
ObjectLocker ol(resolved_references, current);
assert(index >= 0, "Indy index must be positive at this point");

if (resolved_indy_entry_at(index)->method() != nullptr) {
return constant_pool()->resolved_reference_from_indy(index);
return cp->resolved_reference_from_indy(index);
}

if (resolved_indy_entry_at(index)->resolution_failed()) {
// Before we got here, another thread got a LinkageError exception during
// resolution. Ignore our success and throw their exception.
guarantee(index >= 0, "Invalid indy index");
int encoded_index = ResolutionErrorTable::encode_indy_index(index);
JavaThread* THREAD = JavaThread::current(); // For exception macros.
constantPoolHandle cp(THREAD, constant_pool());
ConstantPool::throw_resolution_error(cp, encoded_index, THREAD);
ConstantPool::throw_resolution_error(cp, encoded_index, current);
return nullptr;
}

Expand All @@ -648,7 +672,6 @@ oop ConstantPoolCache::set_dynamic_call(const CallInfo &call_info, int index) {

if (has_appendix) {
const int appendix_index = resolved_indy_entry_at(index)->resolved_references_index();
objArrayOop resolved_references = constant_pool()->resolved_references();
assert(appendix_index >= 0 && appendix_index < resolved_references->length(), "oob");
assert(resolved_references->obj_at(appendix_index) == nullptr, "init just once");
resolved_references->obj_at_put(appendix_index, appendix());
Expand Down
Loading

3 comments on commit b3bf31a

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

@MBaesken
Copy link
Member

Choose a reason for hiding this comment

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

/backport jdk21u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on b3bf31a Jul 4, 2024

Choose a reason for hiding this comment

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

@MBaesken Could not automatically backport b3bf31a0 to openjdk/jdk21u-dev due to conflicts in the following files:

  • src/hotspot/share/classfile/vmSymbols.hpp
  • src/hotspot/share/oops/cpCache.cpp
  • src/hotspot/share/oops/instanceKlass.cpp

Please fetch the appropriate branch/commit and manually resolve these conflicts by using the following commands in your personal fork of openjdk/jdk21u-dev. Note: these commands are just some suggestions and you can use other equivalent commands you know.

# Fetch the up-to-date version of the target branch
$ git fetch --no-tags https://git.openjdk.org/jdk21u-dev.git master:master

# Check out the target branch and create your own branch to backport
$ git checkout master
$ git checkout -b backport-MBaesken-b3bf31a0-master

# Fetch the commit you want to backport
$ git fetch --no-tags https://git.openjdk.org/jdk.git b3bf31a0a08da679ec2fd21613243fb17b1135a9

# Backport the commit
$ git cherry-pick --no-commit b3bf31a0a08da679ec2fd21613243fb17b1135a9
# Resolve conflicts now

# Commit the files you have modified
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport b3bf31a0a08da679ec2fd21613243fb17b1135a9'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk21u-dev with the title Backport b3bf31a0a08da679ec2fd21613243fb17b1135a9.

Below you can find a suggestion for the pull request body:

Hi all,

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

The commit being backported was authored by Coleen Phillimore on 25 Jun 2024 and was reviewed by David Holmes and Vladimir Ivanov.

Thanks!

Please sign in to comment.