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);