8207924: serviceability/sa/TestUniverse.java#id0 intermittently fails with assert(get_instanceKlass()->is_loaded()) failed: must be at least loaded
Summary: InstanceKlass::implementors() needs the Compile_lock
Reviewed-by: thartmann, eosterlund
--- a/src/hotspot/share/ci/ciInstanceKlass.cpp Fri Aug 10 22:38:18 2018 -0400
+++ b/src/hotspot/share/ci/ciInstanceKlass.cpp Sat Aug 11 12:49:33 2018 -0400
@@ -596,6 +596,7 @@
// Go into the VM to fetch the implementor.
{
VM_ENTRY_MARK;
+ MutexLocker ml(Compile_lock);
Klass* k = get_instanceKlass()->implementor();
if (k != NULL) {
if (k == get_instanceKlass()) {
--- a/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp Fri Aug 10 22:38:18 2018 -0400
+++ b/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp Sat Aug 11 12:49:33 2018 -0400
@@ -382,7 +382,12 @@
err_msg("Expected interface type, got %s", klass->external_name()));
}
InstanceKlass* iklass = InstanceKlass::cast(klass);
- JVMCIKlassHandle handle(THREAD, iklass->implementor());
+ JVMCIKlassHandle handle(THREAD);
+ {
+ // Need Compile_lock around implementor()
+ MutexLocker locker(Compile_lock);
+ handle = iklass->implementor();
+ }
oop implementor = CompilerToVM::get_jvmci_type(handle, CHECK_NULL);
return JNIHandles::make_local(THREAD, implementor);
C2V_END
--- a/src/hotspot/share/oops/instanceKlass.cpp Fri Aug 10 22:38:18 2018 -0400
+++ b/src/hotspot/share/oops/instanceKlass.cpp Sat Aug 11 12:49:33 2018 -0400
@@ -1051,6 +1051,38 @@
}
}
+Klass* InstanceKlass::implementor() const {
+ assert_locked_or_safepoint(Compile_lock);
+ Klass** k = adr_implementor();
+ if (k == NULL) {
+ return NULL;
+ } else {
+ return *k;
+ }
+}
+
+void InstanceKlass::set_implementor(Klass* k) {
+ assert_lock_strong(Compile_lock);
+ assert(is_interface(), "not interface");
+ Klass** addr = adr_implementor();
+ assert(addr != NULL, "null addr");
+ if (addr != NULL) {
+ *addr = k;
+ }
+}
+
+int InstanceKlass::nof_implementors() const {
+ assert_lock_strong(Compile_lock);
+ Klass* k = implementor();
+ if (k == NULL) {
+ return 0;
+ } else if (k != this) {
+ return 1;
+ } else {
+ return 2;
+ }
+}
+
// The embedded _implementor field can only record one implementor.
// When there are more than one implementors, the _implementor field
// is set to the interface Klass* itself. Following are the possible
@@ -1061,7 +1093,7 @@
//
// The _implementor field only exists for interfaces.
void InstanceKlass::add_implementor(Klass* k) {
- assert(Compile_lock->owned_by_self(), "");
+ assert_lock_strong(Compile_lock);
assert(is_interface(), "not interface");
// Filter out my subinterfaces.
// (Note: Interfaces are never on the subklass list.)
@@ -2270,7 +2302,10 @@
if (is_linked()) {
unlink_class();
}
- init_implementor();
+ {
+ MutexLocker ml(Compile_lock);
+ init_implementor();
+ }
constants()->remove_unshareable_info();
@@ -3089,6 +3124,7 @@
st->cr();
if (is_interface()) {
+ MutexLocker ml(Compile_lock);
st->print_cr(BULLET"nof implementors: %d", nof_implementors());
if (nof_implementors() == 1) {
st->print_cr(BULLET"implementor: ");
@@ -3496,14 +3532,8 @@
guarantee(sib->super() == super, "siblings should have same superklass");
}
- // Verify implementor fields
- Klass* im = implementor();
- if (im != NULL) {
- guarantee(is_interface(), "only interfaces should have implementor set");
- guarantee(im->is_klass(), "should be klass");
- guarantee(!im->is_interface() || im == this,
- "implementors cannot be interfaces");
- }
+ // Verify implementor fields requires the Compile_lock, but this is sometimes
+ // called inside a safepoint, so don't verify.
// Verify local interfaces
if (local_interfaces()) {
--- a/src/hotspot/share/oops/instanceKlass.hpp Fri Aug 10 22:38:18 2018 -0400
+++ b/src/hotspot/share/oops/instanceKlass.hpp Sat Aug 11 12:49:33 2018 -0400
@@ -1014,36 +1014,9 @@
#endif
// Access to the implementor of an interface.
- Klass* implementor() const
- {
- Klass** k = adr_implementor();
- if (k == NULL) {
- return NULL;
- } else {
- return *k;
- }
- }
-
- void set_implementor(Klass* k) {
- assert(is_interface(), "not interface");
- Klass** addr = adr_implementor();
- assert(addr != NULL, "null addr");
- if (addr != NULL) {
- *addr = k;
- }
- }
-
- int nof_implementors() const {
- Klass* k = implementor();
- if (k == NULL) {
- return 0;
- } else if (k != this) {
- return 1;
- } else {
- return 2;
- }
- }
-
+ Klass* implementor() const;
+ void set_implementor(Klass* k);
+ int nof_implementors() const;
void add_implementor(Klass* k); // k is a new class that implements this interface
void init_implementor(); // initialize