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 (file)
--- a/runtime/openjdkjvmti/ti_class.cc
+++ b/runtime/openjdkjvmti/ti_class.cc
@@ -336,50 +336,52 @@ struct ClassCallback : public art::ClassLoadCallback {
       art::mirror::Class* output = klass.Get();
 
       FixupGlobalReferenceTables(input, output);
+      FixupLocalReferenceTables(self, input, output);
     }
     if (heap->IsGcConcurrentAndMoving()) {
       heap->DecrementDisableMovingGC(self);
     }
   }
 
-  void FixupGlobalReferenceTables(art::mirror::Class* input,
-                                  art::mirror::Class* output)
-      REQUIRES(art::Locks::mutator_lock_) {
-    art::JavaVMExt* java_vm = art::Runtime::Current()->GetJavaVM();
+  class RootUpdater : public art::RootVisitor {
+   public:
+    RootUpdater(const art::mirror::Class* input, art::mirror::Class* output)
+        : input_(input), output_(output) {}
 
-    // 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::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_);
-          }
+    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);
+   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.
+    RootUpdater global_update(input, output);
     java_vm->VisitRoots(&global_update);
 
     class WeakGlobalUpdate : public art::IsMarkedVisitor {
@@ -402,6 +404,33 @@ struct ClassCallback : public art::ClassLoadCallback {
     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 (file)
--- a/test/912-classes/classes.cc
+++ b/test/912-classes/classes.cc
@@ -443,6 +443,9 @@ class ClassLoadPrepareEquality {
       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 @@ class ClassLoadPrepareEquality {
       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 @@ class ClassLoadPrepareEquality {
       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;