Merge "Avoid holding locks when calling ObjectFree callback"
diff --git a/openjdkjvmti/object_tagging.cc b/openjdkjvmti/object_tagging.cc
index 1562fb6..0a51bf2 100644
--- a/openjdkjvmti/object_tagging.cc
+++ b/openjdkjvmti/object_tagging.cc
@@ -43,6 +43,34 @@
 // Instantiate for jlong = JVMTI tags.
 template class JvmtiWeakTable<jlong>;
 
+void ObjectTagTable::Allow() {
+  JvmtiWeakTable<jlong>::Allow();
+  SendDelayedFreeEvents();
+}
+
+void ObjectTagTable::Broadcast(bool broadcast_for_checkpoint) {
+  JvmtiWeakTable<jlong>::Broadcast(broadcast_for_checkpoint);
+  if (!broadcast_for_checkpoint) {
+    SendDelayedFreeEvents();
+  }
+}
+
+void ObjectTagTable::SendDelayedFreeEvents() {
+  std::vector<jlong> to_send;
+  {
+    art::MutexLock mu(art::Thread::Current(), lock_);
+    to_send.swap(null_tags_);
+  }
+  for (jlong t : to_send) {
+    SendSingleFreeEvent(t);
+  }
+}
+
+void ObjectTagTable::SendSingleFreeEvent(jlong tag) {
+  event_handler_->DispatchEventOnEnv<ArtJvmtiEvent::kObjectFree>(
+      jvmti_env_, art::Thread::Current(), tag);
+}
+
 bool ObjectTagTable::Set(art::mirror::Object* obj, jlong new_tag) {
   if (new_tag == 0) {
     jlong tmp;
@@ -50,6 +78,7 @@
   }
   return JvmtiWeakTable<jlong>::Set(obj, new_tag);
 }
+
 bool ObjectTagTable::SetLocked(art::mirror::Object* obj, jlong new_tag) {
   if (new_tag == 0) {
     jlong tmp;
@@ -61,9 +90,10 @@
 bool ObjectTagTable::DoesHandleNullOnSweep() {
   return event_handler_->IsEventEnabledAnywhere(ArtJvmtiEvent::kObjectFree);
 }
+
 void ObjectTagTable::HandleNullSweep(jlong tag) {
-  event_handler_->DispatchEventOnEnv<ArtJvmtiEvent::kObjectFree>(
-      jvmti_env_, art::Thread::Current(), tag);
+  art::MutexLock mu(art::Thread::Current(), lock_);
+  null_tags_.push_back(tag);
 }
 
 }  // namespace openjdkjvmti
diff --git a/openjdkjvmti/object_tagging.h b/openjdkjvmti/object_tagging.h
index 4181302..ca05a05 100644
--- a/openjdkjvmti/object_tagging.h
+++ b/openjdkjvmti/object_tagging.h
@@ -48,7 +48,18 @@
 class ObjectTagTable final : public JvmtiWeakTable<jlong> {
  public:
   ObjectTagTable(EventHandler* event_handler, ArtJvmTiEnv* env)
-      : event_handler_(event_handler), jvmti_env_(env) {}
+      : lock_("Object tag table lock", art::LockLevel::kGenericBottomLock),
+        event_handler_(event_handler),
+        jvmti_env_(env) {}
+
+  // Denotes that weak-refs are visible on all threads. Used by semi-space.
+  void Allow() override
+      REQUIRES_SHARED(art::Locks::mutator_lock_)
+      REQUIRES(!allow_disallow_lock_);
+  // Used by cms and the checkpoint system.
+  void Broadcast(bool broadcast_for_checkpoint) override
+      REQUIRES_SHARED(art::Locks::mutator_lock_)
+      REQUIRES(!allow_disallow_lock_);
 
   bool Set(art::mirror::Object* obj, jlong tag) override
       REQUIRES_SHARED(art::Locks::mutator_lock_)
@@ -77,6 +88,16 @@
   void HandleNullSweep(jlong tag) override;
 
  private:
+  void SendDelayedFreeEvents()
+      REQUIRES_SHARED(art::Locks::mutator_lock_)
+      REQUIRES(!allow_disallow_lock_);
+
+  void SendSingleFreeEvent(jlong tag)
+      REQUIRES_SHARED(art::Locks::mutator_lock_)
+      REQUIRES(!allow_disallow_lock_, !lock_);
+
+  art::Mutex lock_ BOTTOM_MUTEX_ACQUIRED_AFTER;
+  std::vector<jlong> null_tags_ GUARDED_BY(lock_);
   EventHandler* event_handler_;
   ArtJvmTiEnv* jvmti_env_;
 };
diff --git a/runtime/gc/collector/semi_space.cc b/runtime/gc/collector/semi_space.cc
index 8cd484f..c58b59d 100644
--- a/runtime/gc/collector/semi_space.cc
+++ b/runtime/gc/collector/semi_space.cc
@@ -251,6 +251,7 @@
     ReaderMutexLock mu(self_, *Locks::heap_bitmap_lock_);
     SweepSystemWeaks();
   }
+  Runtime::Current()->BroadcastForNewSystemWeaks();
   Runtime::Current()->GetClassLinker()->CleanupClassLoaders();
   // Revoke buffers before measuring how many objects were moved since the TLABs need to be revoked
   // before they are properly counted.
diff --git a/test/905-object-free/src/art/Test905.java b/test/905-object-free/src/art/Test905.java
index 62b6e62..dddd1aa 100644
--- a/test/905-object-free/src/art/Test905.java
+++ b/test/905-object-free/src/art/Test905.java
@@ -16,10 +16,53 @@
 
 package art;
 
+import java.lang.ref.PhantomReference;
+import java.lang.ref.ReferenceQueue;
 import java.util.ArrayList;
 import java.util.Arrays;
 
 public class Test905 {
+  // Taken from jdwp tests.
+  public static class MarkerObj {
+    public static int cnt = 0;
+    public void finalize() { cnt++; }
+  }
+  public static class GcMarker {
+    private final ReferenceQueue mQueue;
+    private final ArrayList<PhantomReference> mList;
+    public GcMarker() {
+      mQueue = new ReferenceQueue();
+      mList = new ArrayList<PhantomReference>(3);
+    }
+    public void add(Object referent) {
+      mList.add(new PhantomReference(referent, mQueue));
+    }
+    public void waitForGc() {
+      waitForGc(mList.size());
+    }
+    public void waitForGc(int numberOfExpectedFinalizations) {
+      if (numberOfExpectedFinalizations > mList.size()) {
+        throw new IllegalArgumentException("wait condition will never be met");
+      }
+      // Request finalization of objects, and subsequent reference enqueueing.
+      // Repeat until reference queue reaches expected size.
+      do {
+          System.runFinalization();
+          Runtime.getRuntime().gc();
+          try { Thread.sleep(10); } catch (Exception e) {}
+      } while (isLive(numberOfExpectedFinalizations));
+    }
+    private boolean isLive(int numberOfExpectedFinalizations) {
+      int numberFinalized = 0;
+      for (int i = 0, n = mList.size(); i < n; i++) {
+        if (mList.get(i).isEnqueued()) {
+          numberFinalized++;
+        }
+      }
+      return numberFinalized < numberOfExpectedFinalizations;
+    }
+  }
+
   public static void run() throws Exception {
     doTest();
   }
@@ -44,7 +87,7 @@
     allocate(l, 1);
     l.clear();
 
-    Runtime.getRuntime().gc();
+    gcAndWait();
 
     getAndPrintTags();
     System.out.println("---");
@@ -56,12 +99,12 @@
     }
     l.clear();
 
-    Runtime.getRuntime().gc();
+    gcAndWait();
 
     getAndPrintTags();
     System.out.println("---");
 
-    Runtime.getRuntime().gc();
+    gcAndWait();
 
     getAndPrintTags();
     System.out.println("---");
@@ -80,7 +123,7 @@
     for (int i = 1; i <= 100000; ++i) {
       stressAllocate(i);
     }
-    Runtime.getRuntime().gc();
+    gcAndWait();
     long[] freedTags1 = getCollectedTags(0);
     long[] freedTags2 = getCollectedTags(1);
     System.out.println("Free counts " + freedTags1.length + " " + freedTags2.length);
@@ -103,6 +146,17 @@
     System.out.println(Arrays.toString(freedTags));
   }
 
+  private static GcMarker getMarker() {
+    GcMarker m = new GcMarker();
+    m.add(new MarkerObj());
+    return m;
+  }
+
+  private static void gcAndWait() {
+    GcMarker marker = getMarker();
+    marker.waitForGc();
+  }
+
   private static native void setupObjectFreeCallback();
   private static native void enableFreeTracking(boolean enable);
   private static native long[] getCollectedTags(int index);