ART: Correctly handle temporary classes in class-load events (2/3)
When a temporary class is given out in a ClassLoad event, all stored
references need to be fixed up before publishing a ClassPrepare event.
This CL handles objects stored as local references.
Bug: 31684920
Test: m test-art-host-run-test-912-classes
Change-Id: I3c6bbfdaca31d0c0d8ae9bf1facef404e949b6aa
diff --git a/runtime/openjdkjvmti/ti_class.cc b/runtime/openjdkjvmti/ti_class.cc
index a1efb97..66381c2 100644
--- a/runtime/openjdkjvmti/ti_class.cc
+++ b/runtime/openjdkjvmti/ti_class.cc
@@ -336,50 +336,52 @@
art::mirror::Class* output = klass.Get();
FixupGlobalReferenceTables(input, output);
+ FixupLocalReferenceTables(self, input, output);
}
if (heap->IsGcConcurrentAndMoving()) {
heap->DecrementDisableMovingGC(self);
}
}
+ class RootUpdater : public art::RootVisitor {
+ public:
+ RootUpdater(const art::mirror::Class* input, art::mirror::Class* output)
+ : input_(input), output_(output) {}
+
+ void VisitRoots(art::mirror::Object*** roots,
+ size_t count,
+ const art::RootInfo& info ATTRIBUTE_UNUSED)
+ OVERRIDE {
+ for (size_t i = 0; i != count; ++i) {
+ if (*roots[i] == input_) {
+ *roots[i] = output_;
+ }
+ }
+ }
+
+ void VisitRoots(art::mirror::CompressedReference<art::mirror::Object>** roots,
+ size_t count,
+ const art::RootInfo& info ATTRIBUTE_UNUSED)
+ OVERRIDE REQUIRES_SHARED(art::Locks::mutator_lock_) {
+ for (size_t i = 0; i != count; ++i) {
+ if (roots[i]->AsMirrorPtr() == input_) {
+ roots[i]->Assign(output_);
+ }
+ }
+ }
+
+ private:
+ const art::mirror::Class* input_;
+ art::mirror::Class* output_;
+ };
+
void FixupGlobalReferenceTables(art::mirror::Class* input,
art::mirror::Class* output)
REQUIRES(art::Locks::mutator_lock_) {
art::JavaVMExt* java_vm = art::Runtime::Current()->GetJavaVM();
// Fix up the global table with a root visitor.
- class GlobalUpdate : public art::RootVisitor {
- public:
- GlobalUpdate(art::mirror::Class* root_input, art::mirror::Class* root_output)
- : input_(root_input), output_(root_output) {}
-
- void VisitRoots(art::mirror::Object*** roots,
- size_t count,
- const art::RootInfo& info ATTRIBUTE_UNUSED)
- OVERRIDE {
- for (size_t i = 0; i != count; ++i) {
- if (*roots[i] == input_) {
- *roots[i] = output_;
- }
- }
- }
-
- void VisitRoots(art::mirror::CompressedReference<art::mirror::Object>** roots,
- size_t count,
- const art::RootInfo& info ATTRIBUTE_UNUSED)
- OVERRIDE REQUIRES_SHARED(art::Locks::mutator_lock_) {
- for (size_t i = 0; i != count; ++i) {
- if (roots[i]->AsMirrorPtr() == input_) {
- roots[i]->Assign(output_);
- }
- }
- }
-
- private:
- const art::mirror::Class* input_;
- art::mirror::Class* output_;
- };
- GlobalUpdate global_update(input, output);
+ RootUpdater global_update(input, output);
java_vm->VisitRoots(&global_update);
class WeakGlobalUpdate : public art::IsMarkedVisitor {
@@ -402,6 +404,33 @@
java_vm->SweepJniWeakGlobals(&weak_global_update);
}
+ void FixupLocalReferenceTables(art::Thread* self,
+ art::mirror::Class* input,
+ art::mirror::Class* output)
+ REQUIRES(art::Locks::mutator_lock_) {
+ class LocalUpdate {
+ public:
+ LocalUpdate(const art::mirror::Class* root_input, art::mirror::Class* root_output)
+ : input_(root_input), output_(root_output) {}
+
+ static void Callback(art::Thread* t, void* arg) REQUIRES(art::Locks::mutator_lock_) {
+ LocalUpdate* local = reinterpret_cast<LocalUpdate*>(arg);
+
+ // Fix up the local table with a root visitor.
+ RootUpdater local_update(local->input_, local->output_);
+ t->GetJniEnv()->locals.VisitRoots(
+ &local_update, art::RootInfo(art::kRootJNILocal, t->GetThreadId()));
+ }
+
+ private:
+ const art::mirror::Class* input_;
+ art::mirror::Class* output_;
+ };
+ LocalUpdate local_upd(input, output);
+ art::MutexLock mu(self, *art::Locks::thread_list_lock_);
+ art::Runtime::Current()->GetThreadList()->ForEach(LocalUpdate::Callback, &local_upd);
+ }
+
// A set of all the temp classes we have handed out. We have to fix up references to these.
// For simplicity, we store the temp classes as JNI global references in a vector. Normally a
// Prepare event will closely follow, so the vector should be small.
diff --git a/test/912-classes/classes.cc b/test/912-classes/classes.cc
index 6c12522..b727453 100644
--- a/test/912-classes/classes.cc
+++ b/test/912-classes/classes.cc
@@ -443,6 +443,9 @@
found_ = true;
stored_class_ = jni_env->NewGlobalRef(klass);
weakly_stored_class_ = jni_env->NewWeakGlobalRef(klass);
+ // The following is bad and relies on implementation details. But otherwise a test would be
+ // a lot more complicated.
+ local_stored_class_ = jni_env->NewLocalRef(klass);
}
}
@@ -455,6 +458,7 @@
CHECK(stored_class_ != nullptr);
CHECK(jni_env->IsSameObject(stored_class_, klass));
CHECK(jni_env->IsSameObject(weakly_stored_class_, klass));
+ CHECK(jni_env->IsSameObject(local_stored_class_, klass));
compared_ = true;
}
}
@@ -469,17 +473,20 @@
env->DeleteGlobalRef(stored_class_);
DCHECK(weakly_stored_class_ != nullptr);
env->DeleteWeakGlobalRef(weakly_stored_class_);
+ // Do not attempt to delete the local ref. It will be out of date by now.
}
}
private:
static jobject stored_class_;
static jweak weakly_stored_class_;
+ static jobject local_stored_class_;
static bool found_;
static bool compared_;
};
jobject ClassLoadPrepareEquality::stored_class_ = nullptr;
jweak ClassLoadPrepareEquality::weakly_stored_class_ = nullptr;
+jobject ClassLoadPrepareEquality::local_stored_class_ = nullptr;
bool ClassLoadPrepareEquality::found_ = false;
bool ClassLoadPrepareEquality::compared_ = false;